[XEN PATCH 0/2] x86: address remaining violation of Rule 7.2

[XEN PATCH 0/2] x86: address remaining violation of Rule 7.2

[Alessandro Zucchelli]

This patch series addresses the last violations of MISRA C:2012 Rule 7.2.
This rule will soon be tagged as clean for ECLAIR in a future patch. 

Alessandro Zucchelli (2):
  pci: add suffix 'U' to PCI_CONF_ADDRESS macro.
  x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

 xen/arch/x86/include/asm/msr-index.h | 2 +-
 xen/arch/x86/x86_64/pci.c            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.25.1

[XEN PATCH 1/2] pci: add suffix 'U' to PCI_CONF_ADDRESS macro.

[Alessandro Zucchelli]

This addresses violations of MISRA C:2012 Rule 7.2 which states as
following: A “u” or “U” suffix shall be applied to all integer constants
that are represented in an unsigned type.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 xen/arch/x86/x86_64/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index aad1c3f7cf..8d33429103 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -9,7 +9,7 @@
 #include <asm/io.h>
 
 #define PCI_CONF_ADDRESS(sbdf, reg) \
-    (0x80000000 | ((sbdf).bdf << 8) | ((reg) & ~3))
+    (0x80000000U | ((sbdf).bdf << 8) | ((reg) & ~3))
 
 uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
 {
-- 
2.25.1

Re: [XEN PATCH 1/2] pci: add suffix 'U' to PCI_CONF_ADDRESS macro.

[Jan Beulich]

On 24.04.2024 14:11, Alessandro Zucchelli wrote:
> This addresses violations of MISRA C:2012 Rule 7.2 which states as
> following: A “u” or “U” suffix shall be applied to all integer constants
> that are represented in an unsigned type.
> 
> No functional change.
> 
> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

> --- a/xen/arch/x86/x86_64/pci.c
> +++ b/xen/arch/x86/x86_64/pci.c
> @@ -9,7 +9,7 @@
>  #include <asm/io.h>
>  
>  #define PCI_CONF_ADDRESS(sbdf, reg) \
> -    (0x80000000 | ((sbdf).bdf << 8) | ((reg) & ~3))
> +    (0x80000000U | ((sbdf).bdf << 8) | ((reg) & ~3))
>  
>  uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
>  {

This is x86'es PCI, so the subject prefix may ant to be "x86/pci:".

Jan

Re: [XEN PATCH 1/2] pci: add suffix 'U' to PCI_CONF_ADDRESS macro.

[Alessandro Zucchelli]

On 2024-04-24 14:47, Jan Beulich wrote:
> On 24.04.2024 14:11, Alessandro Zucchelli wrote:
>> This addresses violations of MISRA C:2012 Rule 7.2 which states as
>> following: A “u” or “U” suffix shall be applied to all integer 
>> constants
>> that are represented in an unsigned type.
>> 
>> No functional change.
>> 
>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
>> --- a/xen/arch/x86/x86_64/pci.c
>> +++ b/xen/arch/x86/x86_64/pci.c
>> @@ -9,7 +9,7 @@
>>  #include <asm/io.h>
>> 
>>  #define PCI_CONF_ADDRESS(sbdf, reg) \
>> -    (0x80000000 | ((sbdf).bdf << 8) | ((reg) & ~3))
>> +    (0x80000000U | ((sbdf).bdf << 8) | ((reg) & ~3))
>> 
>>  uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
>>  {
> 
> This is x86'es PCI, so the subject prefix may ant to be "x86/pci:".

Noted, thanks.

-- 
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)

[XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

[Alessandro Zucchelli]

This addresses violations of MISRA C:2012 Rule 7.2 which states as
following: A “u” or “U” suffix shall be applied to all integer constants
that are represented in an unsigned type.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 xen/arch/x86/include/asm/msr-index.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 92dd9fa496..9cdb5b2625 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -236,7 +236,7 @@
 
 #define MSR_VIRT_SPEC_CTRL                  _AC(0xc001011f, U) /* Layout matches MSR_SPEC_CTRL */
 
-#define MSR_AMD_CSTATE_CFG                  0xc0010296
+#define MSR_AMD_CSTATE_CFG                  0xc0010296U
 
 /*
  * Legacy MSR constants in need of cleanup.  No new MSRs below this comment.
-- 
2.25.1

Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

[Jan Beulich]

On 24.04.2024 14:11, Alessandro Zucchelli wrote:
> This addresses violations of MISRA C:2012 Rule 7.2 which states as
> following: A “u” or “U” suffix shall be applied to all integer constants
> that are represented in an unsigned type.
> 
> No functional change.

I'm inclined to suggest
Fixes: 652683e1aeaa ("x86/hvm: address violations of MISRA C:2012 Rule 7.2")
as that change clearly should have taken care of this already. The line
changed here is even visible in patch context there.

> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

[Andrew Cooper]

On 24/04/2024 1:51 pm, Jan Beulich wrote:
> On 24.04.2024 14:11, Alessandro Zucchelli wrote:
>> This addresses violations of MISRA C:2012 Rule 7.2 which states as
>> following: A “u” or “U” suffix shall be applied to all integer constants
>> that are represented in an unsigned type.
>>
>> No functional change.
> I'm inclined to suggest
> Fixes: 652683e1aeaa ("x86/hvm: address violations of MISRA C:2012 Rule 7.2")
> as that change clearly should have taken care of this already. The line
> changed here is even visible in patch context there.
>
>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
>

I expect it was a race condition.  MSR_AMD_CSTATE_CFG is a recent addition.

~Andrew

Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

[Teddy Astie]

Le 24/04/2024 à 14:11, Alessandro Zucchelli a écrit :
> This addresses violations of MISRA C:2012 Rule 7.2 which states as
> following: A “u” or “U” suffix shall be applied to all integer constants
> that are represented in an unsigned type.
>
> No functional change.
>
> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
> ---
>   xen/arch/x86/include/asm/msr-index.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
> index 92dd9fa496..9cdb5b2625 100644
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -236,7 +236,7 @@
>
>   #define MSR_VIRT_SPEC_CTRL                  _AC(0xc001011f, U) /* Layout matches MSR_SPEC_CTRL */
>
> -#define MSR_AMD_CSTATE_CFG                  0xc0010296
> +#define MSR_AMD_CSTATE_CFG                  0xc0010296U
>
>   /*
>    * Legacy MSR constants in need of cleanup.  No new MSRs below this comment.

Hello, thanks for the patches

I wonder if the same approach should be also taken for all the other MSR
constants of this file that are similar ?

Teddy


Teddy Astie | Vates XCP-ng Intern

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

[Alessandro Zucchelli]

On 2024-04-24 14:53, Teddy Astie wrote:
> Le 24/04/2024 à 14:11, Alessandro Zucchelli a écrit :
>> This addresses violations of MISRA C:2012 Rule 7.2 which states as
>> following: A “u” or “U” suffix shall be applied to all integer 
>> constants
>> that are represented in an unsigned type.
>> 
>> No functional change.
>> 
>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
>> ---
>>   xen/arch/x86/include/asm/msr-index.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/xen/arch/x86/include/asm/msr-index.h 
>> b/xen/arch/x86/include/asm/msr-index.h
>> index 92dd9fa496..9cdb5b2625 100644
>> --- a/xen/arch/x86/include/asm/msr-index.h
>> +++ b/xen/arch/x86/include/asm/msr-index.h
>> @@ -236,7 +236,7 @@
>> 
>>   #define MSR_VIRT_SPEC_CTRL                  _AC(0xc001011f, U) /* 
>> Layout matches MSR_SPEC_CTRL */
>> 
>> -#define MSR_AMD_CSTATE_CFG                  0xc0010296
>> +#define MSR_AMD_CSTATE_CFG                  0xc0010296U
>> 
>>   /*
>>    * Legacy MSR constants in need of cleanup.  No new MSRs below this 
>> comment.
> 
> Hello, thanks for the patches
> 
> I wonder if the same approach should be also taken for all the other 
> MSR
> constants of this file that are similar ?
> 
Hello,

from a (strict) point of view of Rule 7.2, the suffix is needed if the
numeric constant cannot be represented using an int and it is therefore
represented using an unsigned integer.
Every violation in the MSR file has already been addressed.
-- 
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)