Re: [XEN PATCH] x86/HVM: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 19/07/23 12:23, Andrew Cooper wrote:
> On 19/07/2023 11:07 am, Federico Serafini wrote:
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > index 57363c2ae1..8f19a79f6f 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -319,7 +319,7 @@ static bool pat_valid(uint64_t val)
> >       return !(any_gt_7 | any_2_or_3);
> >   }
> >   
> > -int hvm_set_guest_pat(struct vcpu *v, uint64_t guest_pat)
> > +int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
> If there's a u64 vs uint64_t mismatch (or others for that matter),
> resolve in the way of the stdint types.  That way you're correcting in
> the direction of the Xen coding style, rather than away from it.
Changing the type in the declaration from u64 to uint64_t
touching hvm.h will lead to inconsistencies with adjacent declarations.
In such cases, should I propagate the change to the adjacent ones
as well?

> > diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
> > index 43245e13de..eb9c416307 100644
> > --- a/xen/arch/x86/hvm/svm/nestedhvm.h
> > +++ b/xen/arch/x86/hvm/svm/nestedhvm.h
> > @@ -42,7 +42,7 @@ int cf_check nsvm_vcpu_initialise(struct vcpu *v);
> >   int cf_check nsvm_vcpu_reset(struct vcpu *v);
> >   int nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs);
> >   int cf_check nsvm_vcpu_vmexit_event(struct vcpu *v,
> > -                                    const struct x86_event *event);
> > +                                    const struct x86_event *trap);
> This needs to stay as event.  Trap is a related technical term, and
> incorrect to use here.  (Yes, the implementation of
> nsvm_vcpu_vmexit_event() is wrong.)
Ok.
> > diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
> > b/xen/arch/x86/include/asm/hvm/hvm.h
> > index 3c37f522b9..7943e287cf 100644
> > --- a/xen/arch/x86/include/asm/hvm/hvm.h
> > +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> >       int (*msr_write_intercept)(unsigned int msr, uint64_t msr_content);
> >       void (*handle_cd)(struct vcpu *v, unsigned long value);
> >       void (*set_info_guest)(struct vcpu *v);
> > -    void (*set_rdtsc_exiting)(struct vcpu *v, bool_t);
> > -    void (*set_descriptor_access_exiting)(struct vcpu *v, bool);
> > +    void (*set_rdtsc_exiting)(struct vcpu *v, bool_t enable);
> In this case, we want to swap to bool as well as giving a name in the
> prototype.
Ok.

> xen.git/xen$ git grep -w bool_t | wc -l
> 441
>
> I'm seriously tempted just to do a bulk fix of bool_t to get it over and
> done with...
>
> ~Andrew
--
Federico Serafini, M.Sc.

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