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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 26 Oct 2023, Nicola Vetrini wrote:
> 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.

asmlinkage is good