Re: [XEN PATCH][for-4.19 v2] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
On 2023-11-13 15:44, Jan Beulich wrote:
On 07.11.2023 11:30, Nicola Vetrini wrote:
--- a/xen/arch/x86/boot/cmdline.c
+++ b/xen/arch/x86/boot/cmdline.c
@@ -31,6 +31,7 @@ asm (
);
#include
+#include
#include "defs.h"
#include "video.h"
Please respect the goal of alphabetically sorted groups of #include-s.
Will do
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -33,6 +33,8 @@ asm (
#include "../../../include/xen/kconfig.h"
#include
+#include
Same here, put on top of the tidying patch I just sent.
This one, right?
https://lore.kernel.org/xen-devel/c027b9cd-37f3-8223-141f-a1dceda82...@suse.com/
--- 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)
{
static EFI_GUID __initdata loaded_image_guid =
LOADED_IMAGE_PROTOCOL;
static EFI_GUID __initdata shim_lock_guid =
SHIM_LOCK_PROTOCOL_GUID;
Besides this patch not working on its own (as already said by Julien,
you want to explicitly state dependencies), the change above makes me
wonder about efi_multiboot2(): Neither the earlier series nor the
change here are touching either of the two instances of the function.
Was that merely an oversight, or is there another reason?
Looks like an oversight, but I'll have to investigate; if it needs to be
modified I'll do a separate patch.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH][for-4.19 v2] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
On 07.11.2023 11:30, Nicola Vetrini wrote:
> --- a/xen/arch/x86/boot/cmdline.c
> +++ b/xen/arch/x86/boot/cmdline.c
> @@ -31,6 +31,7 @@ asm (
> );
>
> #include
> +#include
> #include "defs.h"
> #include "video.h"
Please respect the goal of alphabetically sorted groups of #include-s.
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -33,6 +33,8 @@ asm (
> #include "../../../include/xen/kconfig.h"
> #include
>
> +#include
Same here, put on top of the tidying patch I just sent.
> --- 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)
> {
> static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL;
> static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
Besides this patch not working on its own (as already said by Julien,
you want to explicitly state dependencies), the change above makes me
wonder about efi_multiboot2(): Neither the earlier series nor the
change here are touching either of the two instances of the function.
Was that merely an oversight, or is there another reason?
Jan
Re: [XEN PATCH][for-4.19 v2] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
Hi Nicola, On 07/11/2023 10:30, 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 With one note below: Acked-by: Julien Grall --- Changes in v2: - Edit safe.json. - Remove mention of SAF-1-safe in deviations.rst. --- docs/misra/deviations.rst | 5 ++--- docs/misra/safe.json| 2 +- xen/arch/arm/cpuerrata.c| 7 +++ xen/arch/arm/setup.c| 5 ++--- xen/arch/arm/smpboot.c | 3 +-- xen/arch/arm/traps.c| 21 +++-- xen/arch/x86/boot/cmdline.c | 5 +++-- xen/arch/x86/boot/reloc.c | 7 --- xen/arch/x86/extable.c | 3 +-- xen/arch/x86/setup.c| 3 +-- xen/arch/x86/traps.c| 27 +-- xen/common/efi/boot.c | 5 ++--- 12 files changed, 36 insertions(+), 57 deletions(-) diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst index d468da2f5ce9..0fa0475db0eb 100644 --- a/docs/misra/deviations.rst +++ b/docs/misra/deviations.rst @@ -134,9 +134,8 @@ Deviations related to MISRA C:2012 Rules: - Tagged as `safe` for ECLAIR. * - R8.4 - - Functions and variables used only by asm modules are either marked with - the `asmlinkage` macro or with a SAF-1-safe textual deviation - (see safe.json). This will require a rebase on top https://lore.kernel.org/all/b914ac93-2499-4bfd-a60a-51a9f1c17...@xen.org/ with the updated wording. Also, it is a good idea to mention any dependency with any patches that are not yet in 'staging' (The for-next branch from Stefano doesn't count). This helps the committers to know which order the patches should be committed and also the reviewers to apply the patches for review. Cheers, -- Julien Grall
[XEN PATCH][for-4.19 v2] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
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
---
Changes in v2:
- Edit safe.json.
- Remove mention of SAF-1-safe in deviations.rst.
---
docs/misra/deviations.rst | 5 ++---
docs/misra/safe.json| 2 +-
xen/arch/arm/cpuerrata.c| 7 +++
xen/arch/arm/setup.c| 5 ++---
xen/arch/arm/smpboot.c | 3 +--
xen/arch/arm/traps.c| 21 +++--
xen/arch/x86/boot/cmdline.c | 5 +++--
xen/arch/x86/boot/reloc.c | 7 ---
xen/arch/x86/extable.c | 3 +--
xen/arch/x86/setup.c| 3 +--
xen/arch/x86/traps.c| 27 +--
xen/common/efi/boot.c | 5 ++---
12 files changed, 36 insertions(+), 57 deletions(-)
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index d468da2f5ce9..0fa0475db0eb 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -134,9 +134,8 @@ Deviations related to MISRA C:2012 Rules:
- Tagged as `safe` for ECLAIR.
* - R8.4
- - Functions and variables used only by asm modules are either marked with
- the `asmlinkage` macro or with a SAF-1-safe textual deviation
- (see safe.json).
+ - Functions and variables used only to interface with asm modules should
+ be marked with the `asmlinkage` macro.
- Tagged as `safe` for ECLAIR.
* - R8.6
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 39c5c056c7d4..eaff0312e20c 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -16,7 +16,7 @@
"eclair": "MC3R1.R8.4"
},
"name": "Rule 8.4: asm-only definition",
-"text": "Functions and variables used only by asm modules do not
need to have a visible declaration prior to their definition."
+"text": "Not used anymore."
},
{
"id": "SAF-2-safe",
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 9137958fb682..a28fa6ac78cc 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -370,10 +370,9 @@ custom_param("spec-ctrl", parse_spec_ctrl);
/* Arm64 only for now as for Arm32 the workaround is currently handled in C. */
#ifdef CONFIG_ARM_64
-/* SAF-1-safe */
-void __init arm_enable_wa2_handling(const struct alt_instr *alt,
-const uint32_t *origptr,
-uint32_t *updptr, int nr_inst)
+void asmlinkage __init arm_enable_wa2_handling(const struct alt_instr *alt,
+ const uint32_t *origptr,
+ uint32_t *updptr, int nr_inst)
{
BUG_ON(nr_inst != 1);
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 3f3a45719ccb..bedbce3c0d37 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1077,9 +1077,8 @@ static bool __init is_dom0less_mode(void)
size_t __read_mostly dcache_line_bytes;
/* C entry point for boot CPU */
-/* SAF-1-safe */
-void __init start_xen(unsigned long boot_phys_offset,
- unsigned long fdt_paddr)
+void asmlinkage __init start_xen(unsigned long boot_phys_offset,
+ unsigned long fdt_paddr)
{
size_t fdt_size;
const char *cmdline;
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index ec76de3cac12..a32f610b47d9 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -303,8 +303,7 @@ smp_prepare_cpus(void)
}
/* Boot the current CPU */
-/* SAF-1-safe */
-void start_secondary(void)
+void asmlinkage start_secondary(void)
{
unsigned int cpuid = init_data.cpuid;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 5aa14d470791..63d3bc7bd67b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -161,8 +161,7 @@ void init_traps(void)
isb();
}
-/* SAF-1-safe */
-void __div0(void)
+void asmlinkage __div0(void)
{
printk("Division by zero in hypervisor.\n");
BUG();
@@ -1955,8 +1954,7 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
* Actions that needs to be done after entering the hypervisor from the
* guest and before the interrupts are unmasked.
*/
-/* SAF-1-safe */
-void enter_hypervisor_from_guest_preirq(void)
+void asmlinkage enter_hypervisor_from_guest_preirq(void)
{
struct vcpu *v = current;
@@ -1970,8 +1968,7 @@ void enter_hypervisor_from_guest_preirq(void)
* guest and before we handle any request. Depending on the exception trap,
* this may be called with interrupts unmasked.
*/
-/* SAF-1-safe */
-void enter_hypervisor_from_guest(void)
+void asmlinkage enter_hypervisor_from_guest(void)
{
struct vcpu *v = current;
@@ -1999,8 +1996,7 @@ void
