| Message ID | 4815279026ca4e2f1d93c98bfe6ea51ee139280d.1701270983.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | address some violations of MISRA C Rule 8.4 | expand |
On 29.11.2023 16:24, Nicola Vetrini wrote: > --- a/xen/arch/x86/desc.c > +++ b/xen/arch/x86/desc.c > @@ -91,7 +91,7 @@ seg_desc_t boot_compat_gdt[PAGE_SIZE / sizeof(seg_desc_t)] = > * References boot_cpu_gdt_table for a short period, until the CPUs switch > * onto their per-CPU GDTs. > */ > -const struct desc_ptr boot_gdtr = { > +const struct desc_ptr asmlinkage boot_gdtr = { > .limit = LAST_RESERVED_GDT_BYTE, > .base = (unsigned long)(boot_gdt - FIRST_RESERVED_GDT_ENTRY), > }; I'm not convinced asmlinkage is okay to use on data. Recall that in principle it may expand to an attribute specifying a non-default calling convention. Such attributes cannot be assumed to continue to be possible to apply to non-functions, even if such may happen to work with a particular compiler version. Jan
On 2023-11-30 17:44, Jan Beulich wrote: > On 29.11.2023 16:24, Nicola Vetrini wrote: >> --- a/xen/arch/x86/desc.c >> +++ b/xen/arch/x86/desc.c >> @@ -91,7 +91,7 @@ seg_desc_t boot_compat_gdt[PAGE_SIZE / >> sizeof(seg_desc_t)] = >> * References boot_cpu_gdt_table for a short period, until the CPUs >> switch >> * onto their per-CPU GDTs. >> */ >> -const struct desc_ptr boot_gdtr = { >> +const struct desc_ptr asmlinkage boot_gdtr = { >> .limit = LAST_RESERVED_GDT_BYTE, >> .base = (unsigned long)(boot_gdt - FIRST_RESERVED_GDT_ENTRY), >> }; > > I'm not convinced asmlinkage is okay to use on data. Recall that in > principle > it may expand to an attribute specifying a non-default calling > convention. > Such attributes cannot be assumed to continue to be possible to apply > to > non-functions, even if such may happen to work with a particular > compiler > version. > It's already being used on variables, I believe. xen/arch/x86/mm.c:l1_pgentry_t asmlinkage __section(".bss.page_aligned") __aligned(PAGE_SIZE) xen/arch/x86/setup.c:unsigned long asmlinkage __read_mostly cr4_pv32_mask; xen/arch/x86/setup.c:char asmlinkage __section(".init.bss.stack_aligned") __aligned(STACK_SIZE) xen/arch/x86/setup.c:unsigned int asmlinkage __initdata multiboot_ptr; If you have concern about this particular variable, then we can fall back on SAF or just put a declaration in the appropriate place.
On 01.12.2023 09:42, Nicola Vetrini wrote: > On 2023-11-30 17:44, Jan Beulich wrote: >> On 29.11.2023 16:24, Nicola Vetrini wrote: >>> --- a/xen/arch/x86/desc.c >>> +++ b/xen/arch/x86/desc.c >>> @@ -91,7 +91,7 @@ seg_desc_t boot_compat_gdt[PAGE_SIZE / >>> sizeof(seg_desc_t)] = >>> * References boot_cpu_gdt_table for a short period, until the CPUs >>> switch >>> * onto their per-CPU GDTs. >>> */ >>> -const struct desc_ptr boot_gdtr = { >>> +const struct desc_ptr asmlinkage boot_gdtr = { >>> .limit = LAST_RESERVED_GDT_BYTE, >>> .base = (unsigned long)(boot_gdt - FIRST_RESERVED_GDT_ENTRY), >>> }; >> >> I'm not convinced asmlinkage is okay to use on data. Recall that in >> principle >> it may expand to an attribute specifying a non-default calling >> convention. >> Such attributes cannot be assumed to continue to be possible to apply >> to >> non-functions, even if such may happen to work with a particular >> compiler >> version. >> > > It's already being used on variables, I believe. > > xen/arch/x86/mm.c:l1_pgentry_t asmlinkage __section(".bss.page_aligned") > __aligned(PAGE_SIZE) > xen/arch/x86/setup.c:unsigned long asmlinkage __read_mostly > cr4_pv32_mask; > xen/arch/x86/setup.c:char asmlinkage > __section(".init.bss.stack_aligned") __aligned(STACK_SIZE) > xen/arch/x86/setup.c:unsigned int asmlinkage __initdata multiboot_ptr; Yeah, I was fearing that something was overlooked earlier on. Jan > If you have concern about this particular variable, then we can fall > back on SAF or just put a declaration in the appropriate place. >
diff --git a/xen/arch/x86/desc.c b/xen/arch/x86/desc.c index 39080ca67211..b332adedf28e 100644 --- a/xen/arch/x86/desc.c +++ b/xen/arch/x86/desc.c @@ -91,7 +91,7 @@ seg_desc_t boot_compat_gdt[PAGE_SIZE / sizeof(seg_desc_t)] = * References boot_cpu_gdt_table for a short period, until the CPUs switch * onto their per-CPU GDTs. */ -const struct desc_ptr boot_gdtr = { +const struct desc_ptr asmlinkage boot_gdtr = { .limit = LAST_RESERVED_GDT_BYTE, .base = (unsigned long)(boot_gdt - FIRST_RESERVED_GDT_ENTRY), }; diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 86467da301e5..8ea64e31cdc2 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -808,8 +808,9 @@ static const char *__init get_option(const char *cmd, const char *opt) return o; } -void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable, - const char *cmdline) +void asmlinkage __init efi_multiboot2(EFI_HANDLE ImageHandle, + EFI_SYSTEM_TABLE *SystemTable, + const char *cmdline) { EFI_GRAPHICS_OUTPUT_PROTOCOL *gop; EFI_HANDLE gop_handle; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 0a66db10b959..9ce21c443bea 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -143,7 +143,7 @@ /* Mapping of the fixmap space needed early. */ l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) l1_fixmap[L1_PAGETABLE_ENTRIES]; -l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) +l1_pgentry_t asmlinkage __section(".bss.page_aligned") __aligned(PAGE_SIZE) l1_fixmap_x[L1_PAGETABLE_ENTRIES]; bool __read_mostly machine_to_phys_mapping_valid; diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index 4c54ecbc91d7..8aa621533f3d 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -310,7 +310,7 @@ static void set_cpu_sibling_map(unsigned int cpu) } } -void start_secondary(void *unused) +void asmlinkage start_secondary(void *unused) { struct cpu_info *info = get_cpu_info();
No functional change. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- xen/arch/x86/desc.c | 2 +- xen/arch/x86/efi/efi-boot.h | 5 +++-- xen/arch/x86/mm.c | 2 +- xen/arch/x86/smpboot.c | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-)