Re: [XEN PATCH][for-4.19 v4 3/8] x86: add deviation comments for asm-only functions

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 26/10/2023 00:36, Stefano Stabellini wrote:
> On Wed, 25 Oct 2023, Nicola Vetrini wrote:
> > On 24/10/2023 21:50, Stefano Stabellini wrote:
> > > On Tue, 24 Oct 2023, Nicola Vetrini wrote:
> > > > On 24/10/2023 10:14, Jan Beulich wrote:
> > > > > On 24.10.2023 10:01, Nicola Vetrini wrote:
> > > > > > On 24/10/2023 09:50, Jan Beulich wrote:
> > > > > > > On 23.10.2023 11:56, 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>
> > > > > > > > ---
> > > > > > > > Changes in v3:
> > > > > > > > - added SAF deviations for vmx counterparts to svm functions.
> > > > > > >
> > > > > > > Same comment regarding the R-b here as for patch 2.
> > > > > > >
> > > > > > > > --- a/xen/arch/x86/hvm/svm/intr.c
> > > > > > > > +++ b/xen/arch/x86/hvm/svm/intr.c
> > > > > > > > @@ -123,6 +123,7 @@ static void svm_enable_intr_window(struct vcpu
> > > > *v,
> > > > > > > > struct hvm_intack intack)
> > > > > > > >          vmcb, general1_intercepts | GENERAL1_INTERCEPT_VINTR);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +/* SAF-1-safe */
> > > > > > > >  void svm_intr_assist(void)
> > > > > > > >  {
> > > > > > > >      struct vcpu *v = current;
> > > > > > >
> > > > > > > Linux has the concept of "asmlinkage" for functions interfacing C
> > > > and
> > > > > > > assembly. Was it considered to use that - even if expanding to
> > > > nothing
> > > > > > > for all present architectures - as a way to annotate affected
> > > > > > > definitions
> > > > > > > in place of the SAF-*-safe comments?
> > > > > >
> > > > > > It was proposed by Julien a while ago (I think it the thread on
> > > > > > deviations.rst) to define
> > > > > > a macro asmcall that expands to nothing, to mark all such functions.
> > > > > > Right now, it's not
> > > > > > strictly necessary (given that there are already some uses of SAF in
> > > > > > Stefano's for-4.19 branch.
> > > > >
> > > > > Can this then be revisited please before any such reaches staging?
> > > > >
> > > > > Jan
> > > >
> > > > I'll let Stefano answer this one.
> > >
> > > Yes it can. If Nicola sends new patches I'll make sure to remove the
> > > corresponding ones from for-4.19.
> > >
> > > Nicola, you might want to send me privately the list of commits to take
> > > out from for-4.19.
> >
> > Actually I checked: the involved SAF comments are already in staging,
> > specifically all
> > were part of commit 5a415ef2b24d578d29479e38abda3d5285b9afed
> OK. In that case we can still use the asmcall macro to deviate/fix new
> violations. I suggest we do that. At some point anyone can go ahead and
> replace those SAF comments with asmcall macros.
Perhaps asmlinkage is a better fit, so that it wouldn't sound strange 
applying it to
variables.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)