1. Patchew
  2. Xen
  3. [PATCH v4 0/6] Move {acpi_}device_init() and device_get_class() to common code
  4. View patch

[PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION

Oleksii Kurochko posted 6 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION
Posted by Oleksii Kurochko 2 months, 3 weeks ago 
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.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
 - new patch
---
 xen/arch/x86/xen.lds.S    | 6 ++++--
 xen/include/xen/xen.lds.h | 6 ++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index b60d2f0d82..9275a566e1 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -3,6 +3,10 @@
 
 #include <xen/cache.h>
 #include <xen/lib.h>
+
+#ifdef EFI
+#define SIMPLE_DECL_SECTION
+#endif
 #include <xen/xen.lds.h>
 #include <asm/page.h>
 #undef ENTRY
@@ -12,9 +16,7 @@
 
 #define FORMAT "pei-x86-64"
 #undef __XEN_VIRT_START
-#undef DECL_SECTION
 #define __XEN_VIRT_START __image_base__
-#define DECL_SECTION(x) x :
 
 ENTRY(efi_start)
 
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index 24b8900ffe..8135732756 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -5,6 +5,8 @@
  * Common macros to be used in architecture specific linker scripts.
  */
 
+#ifndef SIMPLE_DECL_SECTION
+
 /*
  * Declare a section whose load address is based at PA 0 rather than
  * Xen's virtual base address.
@@ -15,6 +17,10 @@
 # define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START)
 #endif
 
+#else /* SIMPLE_DECL_SECTION */
+#define DECL_SECTION(x) x :
+#endif
+
 /*
  * To avoid any confusion, please note that the EFI macro does not correspond
  * to EFI support and is used when linking a native EFI (i.e. PE/COFF) binary,
-- 
2.46.1
Re: [PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION
Posted by Roger Pau Monné 2 months, 3 weeks ago 
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.

> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V4:
>  - new patch
> ---
>  xen/arch/x86/xen.lds.S    | 6 ++++--
>  xen/include/xen/xen.lds.h | 6 ++++++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index b60d2f0d82..9275a566e1 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -3,6 +3,10 @@
>  
>  #include <xen/cache.h>
>  #include <xen/lib.h>
> +
> +#ifdef EFI
> +#define SIMPLE_DECL_SECTION
> +#endif

A nit, but we have been trying to add some indentation to make the
ifdef blocks easier to read, so this would become:

#ifdef EFI
# define SIMPLE_DECL_SECTION
#endif

If it's not too much fuzz to adjust here and below.

Thanks, Roger.
Re: [PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION
Posted by oleksii.kurochko@gmail.com 2 months, 3 weeks ago 
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

~ Oleksii
Re: [PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION
Posted by Roger Pau Monné 2 months, 3 weeks ago 
On Fri, Sep 27, 2024 at 11:07:58AM +0200, oleksii.kurochko@gmail.com 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, Roger.
Re: [PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION
Posted by oleksii.kurochko@gmail.com 2 months, 3 weeks ago 
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@gmail.com 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
Re: [PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION
Posted by Jan Beulich 2 months, 3 weeks ago 
On 27.09.2024 12:42, oleksii.kurochko@gmail.com wrote:
> 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@gmail.com 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?

I'm not overly fussed, so fee free to go ahead and retain my ack.

Jan
Re: [PATCH v4 1/6] xen: introduce SIMPLE_DECL_SECTION
Posted by Jan Beulich 2 months, 3 weeks ago 
On 26.09.2024 18:54, 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.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.