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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.10.2023 00:34, Stefano Stabellini wrote:
> 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. 

I'm afraid I disagree: There are likely direct VMX counterparts to the SVM
items adjusted. No scan is needed to spot those. Anything VMX-only can be
left separate, sure.

Jan