Re: [XEN PATCH v2 3/7] x86: add deviation comments for asm-only functions

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 16 Oct 2023, Jan Beulich wrote:
> On 09.10.2023 08:54, Nicola Vetrini wrote:
> > As stated in rules.rst, functions used only in asm code
> > are allowed to have no prior declaration visible when being
> > defined, hence these functions are deviated.
> > This also fixes violations of MISRA C:2012 Rule 8.4.
> > 
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/hvm/svm/intr.c      | 1 +
> >  xen/arch/x86/hvm/svm/nestedsvm.c | 1 +
> >  xen/arch/x86/hvm/svm/svm.c       | 2 ++
> 
> Once again - why are you not also adjusting the respective VMX code?
> Iirc it was agreed long ago that scans should be extended to cover as
> much of the code base as possible.


Let me summarize here our past discussions on the subject to make sure 
we are all aligned.

With my AMD hat on, of course we want to work with the upstream
community as much as possible and improve the overall codebase. But it
is not a goal for AMD to improve Intel-specific drivers (VMX and
others). Our safety configuration for Xen, including the public ECLAIR
instance currently sponsored by AMD, only includes SVM files, not VMX
files. MISRA compliance costs time and effort; this was done both
because of lack of interest in VMX and also as a cost saving measure.

Upon maintainer's request we can expand the scope of individual patches.
For example, AMD is not interested in ARM32 either, but in the past we
did address certain MISRA C issues on ARM32 too, if nothing else for
consistency of the code base. It comes down to a compromise what we
should do for consistency of the codebase versus addressing things that
makes sense for AMD business. For sure we could work on a few violations
affecting Intel drivers, but overall I don't think AMD could be asked to
make Intel drivers MISRA compliant.


In addition to the above, we also discussed during one of the past MISRA
C working group meetings to have larger-than-usual ECLAIR scans. ECLAIR
takes a couple of hours to run, so it is a good idea to restrict its
configuration in the usual runs. However, at least once a week maybe on
a Sunday, it would be good to run a comprehensive scan including
components that are not currently targeted for safety. This would help
us detect regressions and in general spot potential bugs.

As part of this larger-than-usual ECLAIR scan we could certainly
include Intel drivers as well as other things currently unsupported.


Now, concrete action items. Nicola, I think we should look into having a
larger-than-usual ECLAIR scan every week which includes all of Intel
files and in general as much as possible of the codebase.

Jan, for this specific patch, I don't think we have the scan including
Intel vmx files yet. Nicola please correct me if I am wrong. So Nicola
wouldn't be able to easily expand this patch to also cover Intel vmx
violations of this rule because we don't have the list of violations
affecting those files.