Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2023-11-23 09:57, Jan Beulich wrote:
> On 16.11.2023 10:08, Nicola Vetrini wrote:
> > The comment-based justifications for MISRA C:2012 Rule 8.4 are 
> > replaced
> > by the asmlinkage pseudo-attribute, for the sake of uniformity.
> >
> > Add missing 'xen/compiler.h' #include-s where needed.
> >
> > The text in docs/misra/deviations.rst and docs/misra/safe.json
> > is modified to reflect this change.
> >
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > ---
> > This patch should be applied after patch 2 of this series.
> > The request made by Julien to update the wording is
> > contained in the present patch.
> Along with this request he supplied you with an ack. Did you drop that
> for a particular reason, or did you simply forget to record it here?
I forgot and added it in a reply.

> > --- a/xen/arch/x86/boot/reloc.c
> > +++ b/xen/arch/x86/boot/reloc.c
> > @@ -28,6 +28,7 @@ asm (
> >
> >  #include "defs.h"
> >
> > +#include <xen/compiler.h>
> >  #include <xen/kconfig.h>
> >  #include <xen/multiboot.h>
> >  #include <xen/multiboot2.h>
> > @@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t 
> > mbi_in, uint32_t video_out)
> >      return mbi_out;
> >  }
> >
> > -/* SAF-1-safe */
> > -void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t 
> > trampoline,
> > -                      uint32_t video_info)
> > +void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in,
> > +                                 uint32_t trampoline, uint32_t 
> > video_info)
> >  {
> One purpose of asmlinkage is to possibly alter the default C calling 
> convention
> to some more easy to use in assembly code. At least over a period of 
> time Linux
> for example used that on ix86. If we decided to do something like this, 
> there
> would be a collision with __stdcall. Hence I'm not convinced we can put
> asmlinkage here. At which point the complete removal of SAF-1-safe also 
> would
> need dropping.
Ok, so we can keep SAF-1 here and below and leave the wording in 
deviations.rst and safe.sjon as is.
> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -1254,9 +1254,8 @@ static void __init efi_exit_boot(EFI_HANDLE 
> > ImageHandle, EFI_SYSTEM_TABLE *Syste
> >      efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
> >  }
> >
> > -/* SAF-1-safe */
> > -void EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle,
> > -                                      EFI_SYSTEM_TABLE *SystemTable)
> > +void asmlinkage EFIAPI __init noreturn efi_start(EFI_HANDLE 
> > ImageHandle,
> > +                                                 EFI_SYSTEM_TABLE 
> > *SystemTable)
> Same here wrt EFIAPI, as that also alters the calling convention.
>
> Jan
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)