x86/MCE-telem: adjust cookie definition

[PATCH] x86/MCE-telem: adjust cookie definition

Posted by Jan Beulich 8 months, 2 weeks ago

struct mctelem_ent is opaque outside of mcetelem.c; the cookie
abstraction exists - afaict - just to achieve this opaqueness. Then it
is irrelevant though which kind of pointer mctelem_cookie_t resolves to.
IOW we can as well use struct mctelem_ent there, allowing to remove the
casts from COOKIE2MCTE() and MCTE2COOKIE(). Their removal addresses
Misra C:2012 rule 11.2 ("Conversions shall not be performed between a
pointer to an incomplete type and any other type") violations.

No functional change intended.

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

--- a/xen/arch/x86/cpu/mcheck/mctelem.c
+++ b/xen/arch/x86/cpu/mcheck/mctelem.c
@@ -64,8 +64,8 @@ struct mctelem_ent {
 
 #define MC_NENT (MC_URGENT_NENT + MC_NONURGENT_NENT)
 
-#define	COOKIE2MCTE(c)		((struct mctelem_ent *)(c))
-#define	MCTE2COOKIE(tep)	((mctelem_cookie_t)(tep))
+#define	COOKIE2MCTE(c)		(c)
+#define	MCTE2COOKIE(tep)	(tep)
 
 static struct mc_telem_ctl {
 	/* Linked lists that thread the array members together.
--- a/xen/arch/x86/cpu/mcheck/mctelem.h
+++ b/xen/arch/x86/cpu/mcheck/mctelem.h
@@ -52,7 +52,7 @@
  * the element from the processing list.
  */
 
-typedef struct mctelem_cookie *mctelem_cookie_t;
+typedef struct mctelem_ent *mctelem_cookie_t;
 
 typedef enum mctelem_class {
     MC_URGENT,

Re: [PATCH] x86/MCE-telem: adjust cookie definition

Posted by Andrew Cooper 8 months, 2 weeks ago

On 19/02/2025 10:00 am, Jan Beulich wrote:
> struct mctelem_ent is opaque outside of mcetelem.c; the cookie
> abstraction exists - afaict - just to achieve this opaqueness. Then it
> is irrelevant though which kind of pointer mctelem_cookie_t resolves to.
> IOW we can as well use struct mctelem_ent there, allowing to remove the
> casts from COOKIE2MCTE() and MCTE2COOKIE(). Their removal addresses
> Misra C:2012 rule 11.2 ("Conversions shall not be performed between a
> pointer to an incomplete type and any other type") violations.
>
> No functional change intended.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9181587757

Eclair does appear to be happy with this approach (assuming I stripped
down to only checking R11.2 correctly, and making it fatal).

For the change itself, it's an almost identical binary, differing only
in the string section which I expect means some embedded line numbers.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH] x86/MCE-telem: adjust cookie definition

Posted by Stefano Stabellini 8 months, 2 weeks ago

On Wed, 19 Feb 2025, Andrew Cooper wrote:
> On 19/02/2025 10:00 am, Jan Beulich wrote:
> > struct mctelem_ent is opaque outside of mcetelem.c; the cookie
> > abstraction exists - afaict - just to achieve this opaqueness. Then it
> > is irrelevant though which kind of pointer mctelem_cookie_t resolves to.
> > IOW we can as well use struct mctelem_ent there, allowing to remove the
> > casts from COOKIE2MCTE() and MCTE2COOKIE(). Their removal addresses
> > Misra C:2012 rule 11.2 ("Conversions shall not be performed between a
> > pointer to an incomplete type and any other type") violations.
> >
> > No functional change intended.
> >
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9181587757
> 
> Eclair does appear to be happy with this approach (assuming I stripped
> down to only checking R11.2 correctly, and making it fatal).
> 
> For the change itself, it's an almost identical binary, differing only
> in the string section which I expect means some embedded line numbers.
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thank you very much Jan for writing the patch, and thank you Andrew for
running the pipeline. It is great that resolves all the 11.2 issues!

Oleksii, may I ask for a release-ack? I'll follow up with a patch to
mark 11.2 as clean.

Re: [PATCH] x86/MCE-telem: adjust cookie definition

Posted by Oleksii Kurochko 8 months, 1 week ago

On 2/20/25 2:50 AM, Stefano Stabellini wrote:
> On Wed, 19 Feb 2025, Andrew Cooper wrote:
>> On 19/02/2025 10:00 am, Jan Beulich wrote:
>>> struct mctelem_ent is opaque outside of mcetelem.c; the cookie
>>> abstraction exists - afaict - just to achieve this opaqueness. Then it
>>> is irrelevant though which kind of pointer mctelem_cookie_t resolves to.
>>> IOW we can as well use struct mctelem_ent there, allowing to remove the
>>> casts from COOKIE2MCTE() and MCTE2COOKIE(). Their removal addresses
>>> Misra C:2012 rule 11.2 ("Conversions shall not be performed between a
>>> pointer to an incomplete type and any other type") violations.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Jan Beulich<jbeulich@suse.com>
>> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9181587757
>>
>> Eclair does appear to be happy with this approach (assuming I stripped
>> down to only checking R11.2 correctly, and making it fatal).
>>
>> For the change itself, it's an almost identical binary, differing only
>> in the string section which I expect means some embedded line numbers.
>>
>> Reviewed-by: Andrew Cooper<andrew.cooper3@citrix.com>
> Thank you very much Jan for writing the patch, and thank you Andrew for
> running the pipeline. It is great that resolves all the 11.2 issues!
>
> Oleksii, may I ask for a release-ack? I'll follow up with a patch to
> mark 11.2 as clean.

Release-Acked-By: Oleksii Kurochko<oleksii.kurochko@gmail.com>

~ Oleksii

Re: [PATCH] x86/MCE-telem: adjust cookie definition

Posted by Nicola Vetrini 8 months, 2 weeks ago

On 2025-02-19 11:44, Andrew Cooper wrote:
> On 19/02/2025 10:00 am, Jan Beulich wrote:
>> struct mctelem_ent is opaque outside of mcetelem.c; the cookie
>> abstraction exists - afaict - just to achieve this opaqueness. Then it
>> is irrelevant though which kind of pointer mctelem_cookie_t resolves 
>> to.
>> IOW we can as well use struct mctelem_ent there, allowing to remove 
>> the
>> casts from COOKIE2MCTE() and MCTE2COOKIE(). Their removal addresses
>> Misra C:2012 rule 11.2 ("Conversions shall not be performed between a
>> pointer to an incomplete type and any other type") violations.
>> 
>> No functional change intended.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9181587757
> 
> Eclair does appear to be happy with this approach (assuming I stripped
> down to only checking R11.2 correctly, and making it fatal).
> 

The analysis settings are correct.

> For the change itself, it's an almost identical binary, differing only
> in the string section which I expect means some embedded line numbers.
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH] x86/MCE-telem: adjust cookie definition

Posted by Jan Beulich 8 months, 2 weeks ago

On 19.02.2025 11:44, Andrew Cooper wrote:
> On 19/02/2025 10:00 am, Jan Beulich wrote:
>> struct mctelem_ent is opaque outside of mcetelem.c; the cookie
>> abstraction exists - afaict - just to achieve this opaqueness. Then it
>> is irrelevant though which kind of pointer mctelem_cookie_t resolves to.
>> IOW we can as well use struct mctelem_ent there, allowing to remove the
>> casts from COOKIE2MCTE() and MCTE2COOKIE(). Their removal addresses
>> Misra C:2012 rule 11.2 ("Conversions shall not be performed between a
>> pointer to an incomplete type and any other type") violations.
>>
>> No functional change intended.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/9181587757
> 
> Eclair does appear to be happy with this approach (assuming I stripped
> down to only checking R11.2 correctly, and making it fatal).
> 
> For the change itself, it's an almost identical binary, differing only
> in the string section which I expect means some embedded line numbers.

Line number changes would be odd, given the patch doesn't add or remove
any lines. I expect its internal kind-of-sequence numbers of the compiler,
used to e.g. generate local label names.

> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

Jan