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
