Re: [PATCH v5 04/10] vpci: Refactor REGISTER_VPCI_INIT

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jun 06, 2025 at 09:05:48AM +0200, Jan Beulich wrote:
> On 06.06.2025 08:29, Chen, Jiqian wrote:
> > On 2025/6/5 20:50, Roger Pau Monné wrote:
> >> On Mon, May 26, 2025 at 05:45:53PM +0800, Jiqian Chen wrote: 
> >>> +  }; \
> >>> +  static vpci_capability_t *const finit##_entry  \
> >>> +               __used_section(".data.vpci") = &finit##_t
> >>
> >> IMO this should better use .rodata instead of .data. 
> > Is below change correct?
> > 
> > +    static const vpci_capability_t *const finit##_entry  \
> > +        __used_section(".rodata") = &finit##_t
> 
> No, specifically because ...
> 
> >> Not that it matters much in practice, as we place it in .rodata anyway.  
> >> Note
> >> however you will have to move the placement of the VPCI_ARRAY in the
> >> linker script ahead of *(.rodata.*), otherwise that section match will
> >> consume the vPCI data.
> > I am sorry, how to move it ahead of *(.rodata.*) ?
> > Is below change correct?
> > 
> > diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
> > index 793d0e11450c..3817642135aa 100644
> > --- a/xen/include/xen/xen.lds.h
> > +++ b/xen/include/xen/xen.lds.h
> > @@ -188,7 +188,7 @@
> >  #define VPCI_ARRAY               \
> >         . = ALIGN(POINTER_ALIGN); \
> >         __start_vpci_array = .;   \
> > -       *(SORT(.data.vpci.*))     \
> > +       *(.rodata)             \
> 
> ... this isn't - you'd move _all_ of .rodata into here, which definitely
> isn't what you want. What I understand Roger meant was a .rodata-like
> section, e.g. .rodata.vpci.* (much like it was .data.vpci.* before).

Indeed, my suggestion was merely to use .rodata instead of .data, as
that's more accurate IMO.  I think it should be *(.rodata.vpci) (and
same section change for the __used_section() attribute.

Thanks, Roger.