Re: [PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION

[oleksii.kurochko@xxxxxxxxx]

On Fri, 2024-09-27 at 11:41 +0200, Roger Pau Monné wrote:
> On Fri, Sep 27, 2024 at 11:07:58AM +0200,
> oleksii.kurochko@xxxxxxxxx wrote:
> > On Fri, 2024-09-27 at 09:58 +0200, Roger Pau Monné wrote:
> > > On Thu, Sep 26, 2024 at 06:54:20PM +0200, Oleksii Kurochko wrote:
> > > > Introduce SIMPLE_DECL_SECTION to cover the case when
> > > > an architecture wants to declare a section without specifying
> > > > of load address for the section.
> > > > 
> > > > Update x86/xen.lds.S to use SIMPLE_DECL_SECTION.
> > > 
> > > No strong opinion, but I feel SIMPLE is not very descriptive.  It
> > > might be better to do it the other way around: introduce a define
> > > for
> > > when the DECL_SECTION macro should specify a load address:
> > > DECL_SECTION_WITH_LADDR for example.
> > In the next patch, two sections are introduced: dt_dev_info and
> > acpi_dev_info. The definition of these sections has been made
> > common
> > and moved to xen.lds.h, and it looks like this:
> >    +#define DT_DEV_INFO(secname)    \
> >    +  . = ALIGN(POINTER_ALIGN);     \
> >    +  DECL_SECTION(secname) {       \
> >    +       _sdevice = .;           \
> >    +       *(secname)              \
> >    +       _edevice = .;           \
> >    +  } :text
> > (A similar approach is used for ACPI, please refer to the next
> > patch in
> > this series.)
> > 
> > For PPC, DECL_SECTION should specify a load address, whereas for
> > Arm
> > and RISC-V, it should not.
> > 
> > With this generalization, the name of DECL_SECTION should have the
> > same
> > name in both cases, whether a load address needs to be specified or
> > not
> 
> Oh, sorry, I think you misunderstood my suggestion.
> 
> I'm not suggesting to introduce a new macro named
> DECL_SECTION_WITH_LADDR(), but rather to use DECL_SECTION_WITH_LADDR
> instead of SIMPLE_DECL_SECTION in order to signal whether
> DECL_SECTION() should specify a load address or not, iow:
> 
> #ifdef DECL_SECTION_WITH_LADDR
> # define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START)
> #else
> # define DECL_SECTION(x) x :
> #endif
Thanks for the clarification, I really misunderstood your initial
suggestion.

I'm okay with the renaming; perhaps it will indeed make things a bit
clearer.

If Jan doesn’t mind (since he gave the Ack), I'll rename the define in
the next patch version.
Jan, do you mind if I proceed with the renaming?

~ Oleksii