[PATCH] xen/arm: Remove stray semicolon at VREG_REG_HELPERS/TLB_HELPER* callers

Michal Orzel posted 1 patch 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230614094144.9533-1-michal.orzel@amd.com
xen/arch/arm/include/asm/arm32/flushtlb.h | 12 +++++++-----
xen/arch/arm/include/asm/arm64/flushtlb.h | 17 ++++++++++-------
xen/arch/arm/include/asm/vreg.h           |  4 ++--
3 files changed, 19 insertions(+), 14 deletions(-)
[PATCH] xen/arm: Remove stray semicolon at VREG_REG_HELPERS/TLB_HELPER* callers
Posted by Michal Orzel 11 months ago
This is inconsistent with the rest of the code where macros are used
to define functions, as it results in an empty declaration (i.e.
semicolon with nothing before it) after function definition. This is also
not allowed by C99.

Take the opportunity to undefine TLB_HELPER* macros after last use.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Discussion:
https://lore.kernel.org/xen-devel/17C59D5C-795E-4591-A7C9-A4C5179BF373@arm.com/

Other empty declarations appear at callers of TYPE_SAFE and Linux module
macros like EXPORT_SYMBOL for which we need some sort of agreement.
---
 xen/arch/arm/include/asm/arm32/flushtlb.h | 12 +++++++-----
 xen/arch/arm/include/asm/arm64/flushtlb.h | 17 ++++++++++-------
 xen/arch/arm/include/asm/vreg.h           |  4 ++--
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/include/asm/arm32/flushtlb.h b/xen/arch/arm/include/asm/arm32/flushtlb.h
index 7ae6a12f8155..22ee3b317b4d 100644
--- a/xen/arch/arm/include/asm/arm32/flushtlb.h
+++ b/xen/arch/arm/include/asm/arm32/flushtlb.h
@@ -29,19 +29,21 @@ static inline void name(void)       \
 }
 
 /* Flush local TLBs, current VMID only */
-TLB_HELPER(flush_guest_tlb_local, TLBIALL, nsh);
+TLB_HELPER(flush_guest_tlb_local, TLBIALL, nsh)
 
 /* Flush inner shareable TLBs, current VMID only */
-TLB_HELPER(flush_guest_tlb, TLBIALLIS, ish);
+TLB_HELPER(flush_guest_tlb, TLBIALLIS, ish)
 
 /* Flush local TLBs, all VMIDs, non-hypervisor mode */
-TLB_HELPER(flush_all_guests_tlb_local, TLBIALLNSNH, nsh);
+TLB_HELPER(flush_all_guests_tlb_local, TLBIALLNSNH, nsh)
 
 /* Flush innershareable TLBs, all VMIDs, non-hypervisor mode */
-TLB_HELPER(flush_all_guests_tlb, TLBIALLNSNHIS, ish);
+TLB_HELPER(flush_all_guests_tlb, TLBIALLNSNHIS, ish)
 
 /* Flush all hypervisor mappings from the TLB of the local processor. */
-TLB_HELPER(flush_xen_tlb_local, TLBIALLH, nsh);
+TLB_HELPER(flush_xen_tlb_local, TLBIALLH, nsh)
+
+#undef TLB_HELPER
 
 /* Flush TLB of local processor for address va. */
 static inline void __flush_xen_tlb_one_local(vaddr_t va)
diff --git a/xen/arch/arm/include/asm/arm64/flushtlb.h b/xen/arch/arm/include/asm/arm64/flushtlb.h
index 3a9092b814a9..56c6fc763b56 100644
--- a/xen/arch/arm/include/asm/arm64/flushtlb.h
+++ b/xen/arch/arm/include/asm/arm64/flushtlb.h
@@ -67,25 +67,28 @@ static inline void name(vaddr_t va)              \
 }
 
 /* Flush local TLBs, current VMID only. */
-TLB_HELPER(flush_guest_tlb_local, vmalls12e1, nsh);
+TLB_HELPER(flush_guest_tlb_local, vmalls12e1, nsh)
 
 /* Flush innershareable TLBs, current VMID only */
-TLB_HELPER(flush_guest_tlb, vmalls12e1is, ish);
+TLB_HELPER(flush_guest_tlb, vmalls12e1is, ish)
 
 /* Flush local TLBs, all VMIDs, non-hypervisor mode */
-TLB_HELPER(flush_all_guests_tlb_local, alle1, nsh);
+TLB_HELPER(flush_all_guests_tlb_local, alle1, nsh)
 
 /* Flush innershareable TLBs, all VMIDs, non-hypervisor mode */
-TLB_HELPER(flush_all_guests_tlb, alle1is, ish);
+TLB_HELPER(flush_all_guests_tlb, alle1is, ish)
 
 /* Flush all hypervisor mappings from the TLB of the local processor. */
-TLB_HELPER(flush_xen_tlb_local, alle2, nsh);
+TLB_HELPER(flush_xen_tlb_local, alle2, nsh)
 
 /* Flush TLB of local processor for address va. */
-TLB_HELPER_VA(__flush_xen_tlb_one_local, vae2);
+TLB_HELPER_VA(__flush_xen_tlb_one_local, vae2)
 
 /* Flush TLB of all processors in the inner-shareable domain for address va. */
-TLB_HELPER_VA(__flush_xen_tlb_one, vae2is);
+TLB_HELPER_VA(__flush_xen_tlb_one, vae2is)
+
+#undef TLB_HELPER
+#undef TLB_HELPER_VA
 
 #endif /* __ASM_ARM_ARM64_FLUSHTLB_H__ */
 /*
diff --git a/xen/arch/arm/include/asm/vreg.h b/xen/arch/arm/include/asm/vreg.h
index d92450017bc4..bf945eebbde4 100644
--- a/xen/arch/arm/include/asm/vreg.h
+++ b/xen/arch/arm/include/asm/vreg.h
@@ -140,8 +140,8 @@ static inline void vreg_reg##sz##_clearbits(uint##sz##_t *reg,          \
     *reg &= ~(((uint##sz##_t)bits & mask) << shift);                    \
 }
 
-VREG_REG_HELPERS(64, 0x7);
-VREG_REG_HELPERS(32, 0x3);
+VREG_REG_HELPERS(64, 0x7)
+VREG_REG_HELPERS(32, 0x3)
 
 #undef VREG_REG_HELPERS
 

base-commit: 2f69ef96801f0d2b9646abf6396e60f99c56e3a0
-- 
2.25.1
Re: [PATCH] xen/arm: Remove stray semicolon at VREG_REG_HELPERS/TLB_HELPER* callers
Posted by Stefano Stabellini 11 months ago
On Wed, 14 Jun 2023, Michal Orzel wrote:
> This is inconsistent with the rest of the code where macros are used
> to define functions, as it results in an empty declaration (i.e.
> semicolon with nothing before it) after function definition. This is also
> not allowed by C99.
> 
> Take the opportunity to undefine TLB_HELPER* macros after last use.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Discussion:
> https://lore.kernel.org/xen-devel/17C59D5C-795E-4591-A7C9-A4C5179BF373@arm.com/
> 
> Other empty declarations appear at callers of TYPE_SAFE and Linux module
> macros like EXPORT_SYMBOL for which we need some sort of agreement.
> ---
>  xen/arch/arm/include/asm/arm32/flushtlb.h | 12 +++++++-----
>  xen/arch/arm/include/asm/arm64/flushtlb.h | 17 ++++++++++-------
>  xen/arch/arm/include/asm/vreg.h           |  4 ++--
>  3 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/arm32/flushtlb.h b/xen/arch/arm/include/asm/arm32/flushtlb.h
> index 7ae6a12f8155..22ee3b317b4d 100644
> --- a/xen/arch/arm/include/asm/arm32/flushtlb.h
> +++ b/xen/arch/arm/include/asm/arm32/flushtlb.h
> @@ -29,19 +29,21 @@ static inline void name(void)       \
>  }
>  
>  /* Flush local TLBs, current VMID only */
> -TLB_HELPER(flush_guest_tlb_local, TLBIALL, nsh);
> +TLB_HELPER(flush_guest_tlb_local, TLBIALL, nsh)
>  
>  /* Flush inner shareable TLBs, current VMID only */
> -TLB_HELPER(flush_guest_tlb, TLBIALLIS, ish);
> +TLB_HELPER(flush_guest_tlb, TLBIALLIS, ish)
>  
>  /* Flush local TLBs, all VMIDs, non-hypervisor mode */
> -TLB_HELPER(flush_all_guests_tlb_local, TLBIALLNSNH, nsh);
> +TLB_HELPER(flush_all_guests_tlb_local, TLBIALLNSNH, nsh)
>  
>  /* Flush innershareable TLBs, all VMIDs, non-hypervisor mode */
> -TLB_HELPER(flush_all_guests_tlb, TLBIALLNSNHIS, ish);
> +TLB_HELPER(flush_all_guests_tlb, TLBIALLNSNHIS, ish)
>  
>  /* Flush all hypervisor mappings from the TLB of the local processor. */
> -TLB_HELPER(flush_xen_tlb_local, TLBIALLH, nsh);
> +TLB_HELPER(flush_xen_tlb_local, TLBIALLH, nsh)
> +
> +#undef TLB_HELPER
>  
>  /* Flush TLB of local processor for address va. */
>  static inline void __flush_xen_tlb_one_local(vaddr_t va)
> diff --git a/xen/arch/arm/include/asm/arm64/flushtlb.h b/xen/arch/arm/include/asm/arm64/flushtlb.h
> index 3a9092b814a9..56c6fc763b56 100644
> --- a/xen/arch/arm/include/asm/arm64/flushtlb.h
> +++ b/xen/arch/arm/include/asm/arm64/flushtlb.h
> @@ -67,25 +67,28 @@ static inline void name(vaddr_t va)              \
>  }
>  
>  /* Flush local TLBs, current VMID only. */
> -TLB_HELPER(flush_guest_tlb_local, vmalls12e1, nsh);
> +TLB_HELPER(flush_guest_tlb_local, vmalls12e1, nsh)
>  
>  /* Flush innershareable TLBs, current VMID only */
> -TLB_HELPER(flush_guest_tlb, vmalls12e1is, ish);
> +TLB_HELPER(flush_guest_tlb, vmalls12e1is, ish)
>  
>  /* Flush local TLBs, all VMIDs, non-hypervisor mode */
> -TLB_HELPER(flush_all_guests_tlb_local, alle1, nsh);
> +TLB_HELPER(flush_all_guests_tlb_local, alle1, nsh)
>  
>  /* Flush innershareable TLBs, all VMIDs, non-hypervisor mode */
> -TLB_HELPER(flush_all_guests_tlb, alle1is, ish);
> +TLB_HELPER(flush_all_guests_tlb, alle1is, ish)
>  
>  /* Flush all hypervisor mappings from the TLB of the local processor. */
> -TLB_HELPER(flush_xen_tlb_local, alle2, nsh);
> +TLB_HELPER(flush_xen_tlb_local, alle2, nsh)
>  
>  /* Flush TLB of local processor for address va. */
> -TLB_HELPER_VA(__flush_xen_tlb_one_local, vae2);
> +TLB_HELPER_VA(__flush_xen_tlb_one_local, vae2)
>  
>  /* Flush TLB of all processors in the inner-shareable domain for address va. */
> -TLB_HELPER_VA(__flush_xen_tlb_one, vae2is);
> +TLB_HELPER_VA(__flush_xen_tlb_one, vae2is)
> +
> +#undef TLB_HELPER
> +#undef TLB_HELPER_VA
>  
>  #endif /* __ASM_ARM_ARM64_FLUSHTLB_H__ */
>  /*
> diff --git a/xen/arch/arm/include/asm/vreg.h b/xen/arch/arm/include/asm/vreg.h
> index d92450017bc4..bf945eebbde4 100644
> --- a/xen/arch/arm/include/asm/vreg.h
> +++ b/xen/arch/arm/include/asm/vreg.h
> @@ -140,8 +140,8 @@ static inline void vreg_reg##sz##_clearbits(uint##sz##_t *reg,          \
>      *reg &= ~(((uint##sz##_t)bits & mask) << shift);                    \
>  }
>  
> -VREG_REG_HELPERS(64, 0x7);
> -VREG_REG_HELPERS(32, 0x3);
> +VREG_REG_HELPERS(64, 0x7)
> +VREG_REG_HELPERS(32, 0x3)
>  
>  #undef VREG_REG_HELPERS
>  
> 
> base-commit: 2f69ef96801f0d2b9646abf6396e60f99c56e3a0
> -- 
> 2.25.1
>
Re: [PATCH] xen/arm: Remove stray semicolon at VREG_REG_HELPERS/TLB_HELPER* callers
Posted by Julien Grall 11 months ago
Hi,

On 15/06/2023 00:46, Stefano Stabellini wrote:
> On Wed, 14 Jun 2023, Michal Orzel wrote:
>> This is inconsistent with the rest of the code where macros are used
>> to define functions, as it results in an empty declaration (i.e.
>> semicolon with nothing before it) after function definition. This is also
>> not allowed by C99.
>>
>> Take the opportunity to undefine TLB_HELPER* macros after last use.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

I have committed the patch.

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: Remove stray semicolon at VREG_REG_HELPERS/TLB_HELPER* callers
Posted by Jan Beulich 11 months ago
On 14.06.2023 11:41, Michal Orzel wrote:
> This is inconsistent with the rest of the code where macros are used
> to define functions, as it results in an empty declaration (i.e.
> semicolon with nothing before it) after function definition. This is also
> not allowed by C99.
> 
> Take the opportunity to undefine TLB_HELPER* macros after last use.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
> Discussion:
> https://lore.kernel.org/xen-devel/17C59D5C-795E-4591-A7C9-A4C5179BF373@arm.com/
> 
> Other empty declarations appear at callers of TYPE_SAFE and Linux module
> macros like EXPORT_SYMBOL for which we need some sort of agreement.

As said elsewhere, I think we should finally get rid of EXPORT_SYMBOL().

For TYPE_SAFE() I think some re-arrangement of the macros can address
the Misra concern.

Jan