Introduce conditional macros to define device
information sections based on the configuration of ACPI
or device tree support. These sections are required for
common code of device initialization and getting an information
about a device.
These macros are expected to be used across different
architectures (Arm, PPC, RISC-V), so they are moved to
the common xen/xen.lds.h, based on their original definition
in Arm.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
- New patch.
---
xen/include/xen/xen.lds.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index a17810bb28..aa7301139d 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -114,6 +114,21 @@
/* List of constructs other than *_SECTIONS in alphabetical order. */
+#define USE_DECL_SECTION(x) DECL_SECTION(x)
+
+#define NOUSE_DECL_SECTION(x) x :
+
+#ifdef CONFIG_ACPI
+#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
+ DECL_SECTION_MACROS_NAME(secname) { \
+ _asdevice = .; \
+ *(secname) \
+ _aedevice = .; \
+ } :text
+#else
+#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
+#endif /* CONFIG_ACPI */
+
#define BUGFRAMES \
__start_bug_frames_0 = .; \
*(.bug_frames.0) \
@@ -131,6 +146,17 @@
*(.bug_frames.3) \
__stop_bug_frames_3 = .;
+#ifdef CONFIG_HAS_DEVICE_TREE
+#define DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
+ DECL_SECTION_MACROS_NAME(secname) { \
+ _sdevice = .; \
+ *(secname) \
+ _edevice = .; \
+ } :text
+#else
+#define DECL_DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
+#endif /* CONFIG_HAS_DEVICE_TREE */
+
#ifdef CONFIG_HYPFS
#define HYPFS_PARAM \
. = ALIGN(POINTER_ALIGN); \
--
2.46.0On 17.09.2024 18:15, Oleksii Kurochko wrote:
> --- a/xen/include/xen/xen.lds.h
> +++ b/xen/include/xen/xen.lds.h
> @@ -114,6 +114,21 @@
>
> /* List of constructs other than *_SECTIONS in alphabetical order. */
>
> +#define USE_DECL_SECTION(x) DECL_SECTION(x)
> +
> +#define NOUSE_DECL_SECTION(x) x :
> +
> +#ifdef CONFIG_ACPI
> +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> + DECL_SECTION_MACROS_NAME(secname) { \
> + _asdevice = .; \
> + *(secname) \
> + _aedevice = .; \
> + } :text
> +#else
> +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> +#endif /* CONFIG_ACPI */
> +
> #define BUGFRAMES \
> __start_bug_frames_0 = .; \
> *(.bug_frames.0) \
> @@ -131,6 +146,17 @@
> *(.bug_frames.3) \
> __stop_bug_frames_3 = .;
>
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +#define DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> + DECL_SECTION_MACROS_NAME(secname) { \
> + _sdevice = .; \
> + *(secname) \
> + _edevice = .; \
> + } :text
> +#else
> +#define DECL_DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> +#endif /* CONFIG_HAS_DEVICE_TREE */
Any specific reason for the _SEC suffixes in the names? We don't have
such elsewhere, as can be seen (by example) ...
> #ifdef CONFIG_HYPFS
> #define HYPFS_PARAM \
> . = ALIGN(POINTER_ALIGN); \
... even in context here.
Jan
On Mon, 2024-09-23 at 12:08 +0200, Jan Beulich wrote:
> On 17.09.2024 18:15, Oleksii Kurochko wrote:
> > --- a/xen/include/xen/xen.lds.h
> > +++ b/xen/include/xen/xen.lds.h
> > @@ -114,6 +114,21 @@
> >
> > /* List of constructs other than *_SECTIONS in alphabetical order.
> > */
> >
> > +#define USE_DECL_SECTION(x) DECL_SECTION(x)
> > +
> > +#define NOUSE_DECL_SECTION(x) x :
> > +
> > +#ifdef CONFIG_ACPI
> > +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> > + DECL_SECTION_MACROS_NAME(secname) { \
> > + _asdevice = .; \
> > + *(secname) \
> > + _aedevice = .; \
> > + } :text
> > +#else
> > +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> > +#endif /* CONFIG_ACPI */
> > +
> > #define BUGFRAMES \
> > __start_bug_frames_0 = .; \
> > *(.bug_frames.0) \
> > @@ -131,6 +146,17 @@
> > *(.bug_frames.3) \
> > __stop_bug_frames_3 = .;
> >
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +#define DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> > + DECL_SECTION_MACROS_NAME(secname) { \
> > + _sdevice = .; \
> > + *(secname) \
> > + _edevice = .; \
> > + } :text
> > +#else
> > +#define DECL_DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> > +#endif /* CONFIG_HAS_DEVICE_TREE */
>
> Any specific reason for the _SEC suffixes in the names? We don't have
> such elsewhere, as can be seen (by example) ...
>
> > #ifdef CONFIG_HYPFS
> > #define HYPFS_PARAM \
> > . = ALIGN(POINTER_ALIGN); \
>
> ... even in context here.
The _SEC suffixes can be removed; they were only used to highlight that
it was a section declaration. I'll drop it in the next patch version.
~ Oleksii
Hi Oleksii,
On 9/17/24 11:15 AM, Oleksii Kurochko wrote:
> Introduce conditional macros to define device
> information sections based on the configuration of ACPI
> or device tree support. These sections are required for
> common code of device initialization and getting an information
> about a device.
>
> These macros are expected to be used across different
> architectures (Arm, PPC, RISC-V), so they are moved to
> the common xen/xen.lds.h, based on their original definition
> in Arm.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in v2:
> - New patch.
> ---
> xen/include/xen/xen.lds.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
> index a17810bb28..aa7301139d 100644
> --- a/xen/include/xen/xen.lds.h
> +++ b/xen/include/xen/xen.lds.h
> @@ -114,6 +114,21 @@
>
> /* List of constructs other than *_SECTIONS in alphabetical order. */
>
> +#define USE_DECL_SECTION(x) DECL_SECTION(x)
> +
> +#define NOUSE_DECL_SECTION(x) x :
> +
> +#ifdef CONFIG_ACPI
> +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> + DECL_SECTION_MACROS_NAME(secname) { \
> + _asdevice = .; \
> + *(secname) \
> + _aedevice = .; \
> + } :text
> +#else
> +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> +#endif /* CONFIG_ACPI */
This works, but is there a reason you didn't just define the actual
entries themselves in the macro, like is done for most of the other
macros in this file (including BUFRAMES right below this)? Something
like:
#define ADEV_INFO \
_asdevice = .; \
*(secname) \
_aedevice = .; \
Parameterizing the section name seems pointless since there are other
parts of the code that rely on them, and parameterizing the macro for
declaring a section adds unnecessary boilerplate for non-ppc/x86
architectures (the NOUSE_DECL_SECTION no-op).
> +
> #define BUGFRAMES \
> __start_bug_frames_0 = .; \
> *(.bug_frames.0) \
> @@ -131,6 +146,17 @@
> *(.bug_frames.3) \
> __stop_bug_frames_3 = .;
>
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +#define DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> + DECL_SECTION_MACROS_NAME(secname) { \
> + _sdevice = .; \
> + *(secname) \
> + _edevice = .; \
> + } :text
> +#else
> +#define DECL_DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> +#endif /* CONFIG_HAS_DEVICE_TREE */
Ditto. Also, if you go with the approach I mentioned, then you wouldn't
need to guard these on CONFIG_HAS_DEVICE_TREE/CONFIG_ACPI since that
would be done in the linker scripts themselves.
Thanks,
Shawn
On 19.09.2024 23:05, Shawn Anastasio wrote:
> On 9/17/24 11:15 AM, Oleksii Kurochko wrote:
>> --- a/xen/include/xen/xen.lds.h
>> +++ b/xen/include/xen/xen.lds.h
>> @@ -114,6 +114,21 @@
>>
>> /* List of constructs other than *_SECTIONS in alphabetical order. */
>>
>> +#define USE_DECL_SECTION(x) DECL_SECTION(x)
>> +
>> +#define NOUSE_DECL_SECTION(x) x :
>> +
>> +#ifdef CONFIG_ACPI
>> +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
>> + DECL_SECTION_MACROS_NAME(secname) { \
>> + _asdevice = .; \
>> + *(secname) \
>> + _aedevice = .; \
>> + } :text
>> +#else
>> +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
>> +#endif /* CONFIG_ACPI */
>
> This works, but is there a reason you didn't just define the actual
> entries themselves in the macro, like is done for most of the other
> macros in this file (including BUFRAMES right below this)? Something
> like:
>
> #define ADEV_INFO \
> _asdevice = .; \
> *(secname) \
> _aedevice = .; \
>
> Parameterizing the section name seems pointless since there are other
> parts of the code that rely on them, and parameterizing the macro for
> declaring a section adds unnecessary boilerplate for non-ppc/x86
> architectures (the NOUSE_DECL_SECTION no-op).
>
>> +
>> #define BUGFRAMES \
>> __start_bug_frames_0 = .; \
>> *(.bug_frames.0) \
>> @@ -131,6 +146,17 @@
>> *(.bug_frames.3) \
>> __stop_bug_frames_3 = .;
>>
>> +#ifdef CONFIG_HAS_DEVICE_TREE
>> +#define DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
>> + DECL_SECTION_MACROS_NAME(secname) { \
>> + _sdevice = .; \
>> + *(secname) \
>> + _edevice = .; \
>> + } :text
>> +#else
>> +#define DECL_DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
>> +#endif /* CONFIG_HAS_DEVICE_TREE */
>
> Ditto. Also, if you go with the approach I mentioned, then you wouldn't
> need to guard these on CONFIG_HAS_DEVICE_TREE/CONFIG_ACPI since that
> would be done in the linker scripts themselves.
+1
Jan
Hi Shawn,
On Thu, 2024-09-19 at 16:05 -0500, Shawn Anastasio wrote:
> Hi Oleksii,
>
> On 9/17/24 11:15 AM, Oleksii Kurochko wrote:
> > Introduce conditional macros to define device
> > information sections based on the configuration of ACPI
> > or device tree support. These sections are required for
> > common code of device initialization and getting an information
> > about a device.
> >
> > These macros are expected to be used across different
> > architectures (Arm, PPC, RISC-V), so they are moved to
> > the common xen/xen.lds.h, based on their original definition
> > in Arm.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in v2:
> > - New patch.
> > ---
> > xen/include/xen/xen.lds.h | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
> > index a17810bb28..aa7301139d 100644
> > --- a/xen/include/xen/xen.lds.h
> > +++ b/xen/include/xen/xen.lds.h
> > @@ -114,6 +114,21 @@
> >
> > /* List of constructs other than *_SECTIONS in alphabetical order.
> > */
> >
> > +#define USE_DECL_SECTION(x) DECL_SECTION(x)
> > +
> > +#define NOUSE_DECL_SECTION(x) x :
> > +
> > +#ifdef CONFIG_ACPI
> > +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> > + DECL_SECTION_MACROS_NAME(secname) { \
> > + _asdevice = .; \
> > + *(secname) \
> > + _aedevice = .; \
> > + } :text
> > +#else
> > +#define ACPI_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> > +#endif /* CONFIG_ACPI */
>
> This works, but is there a reason you didn't just define the actual
> entries themselves in the macro, like is done for most of the other
> macros in this file (including BUFRAMES right below this)?
There is no specific reason, just decided it would be nice to abstract
the the full section declaration.
> Something
> like:
>
> #define ADEV_INFO \
> _asdevice = .; \
> *(secname) \
> _aedevice = .; \
>
> Parameterizing the section name seems pointless since there are other
> parts of the code that rely on them, and parameterizing the macro for
> declaring a section adds unnecessary boilerplate for non-ppc/x86
> architectures (the NOUSE_DECL_SECTION no-op).
I’m okay with the suggested approach. I’ll wait for a bit to see if
anyone has other comments. If there are no responses, I’ll resend the
patch series with the proposed changes.
Thanks!
~ Oleksii
>
> > +
> > #define BUGFRAMES \
> > __start_bug_frames_0 = .; \
> > *(.bug_frames.0) \
> > @@ -131,6 +146,17 @@
> > *(.bug_frames.3) \
> > __stop_bug_frames_3 = .;
> >
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +#define DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME) \
> > + DECL_SECTION_MACROS_NAME(secname) { \
> > + _sdevice = .; \
> > + *(secname) \
> > + _edevice = .; \
> > + } :text
> > +#else
> > +#define DECL_DT_DEV_INFO_SEC(secname, DECL_SECTION_MACROS_NAME)
> > +#endif /* CONFIG_HAS_DEVICE_TREE */
>
> Ditto. Also, if you go with the approach I mentioned, then you
> wouldn't
> need to guard these on CONFIG_HAS_DEVICE_TREE/CONFIG_ACPI since that
> would be done in the linker scripts themselves.
© 2016 - 2025 Red Hat, Inc.