Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.
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 --- 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)
Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.
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
> Acked-by: Jan Beulich
>
>
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.
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 > --- > 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.
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
Acked-by: Jan Beulich
