[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 1/5] xen: define ACPI and DT device info sections macros
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@xxxxxxxxx> > > --- > > 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.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |