Re: [PATCH v2 1/2] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers

From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	 Jens Wiklander <jens.wiklander@linaro.org>,
	xen-devel@lists.xenproject.org,
	 Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v2 1/2] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
Date: Wed, 15 Jun 2022 17:09:04 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2206151355000.2430546@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <9c39d8dc-4a74-297c-45fd-4e261fe9ef90@xen.org>

On Wed, 15 Jun 2022, Julien Grall wrote:
> On 11/06/2022 01:41, Stefano Stabellini wrote:
> > >   #endif /* __ASSEMBLY__ */
> > >     /*
> > > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> > > index 676740ef1520..6f90c08a6304 100644
> > > --- a/xen/arch/arm/vsmc.c
> > > +++ b/xen/arch/arm/vsmc.c
> > > @@ -93,7 +93,7 @@ static bool handle_arch(struct cpu_user_regs *regs)
> > >       switch ( fid )
> > >       {
> > >       case ARM_SMCCC_VERSION_FID:
> > > -        set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
> > > +        set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_2);
> > >           return true;
> >    This is going to be a problem for ARM32 given that ARM_SMCCC_VERSION_1_2
> > is unimplemented on ARM32. If there is an ARM32 implementation in Linux
> > for ARM_SMCCC_VERSION_1_2 you might as well import it too.
> > 
> > Otherwise we'll have to abstract it away, e.g.:
> > 
> > #ifdef CONFIG_ARM_64
> > #define ARM_VSMCCC_VERSION ARM_SMCCC_VERSION_1_2
> > #else
> > #define ARM_VSMCCC_VERSION ARM_SMCCC_VERSION_1_1
> > #endif
> 
> I don't understand why you want to tie the virtual and host SMCCC version.
> 
> In theory, it would be possible for us to implement a subsystem to fully
> emulate, lets say, FFA. We would to tell the VM that we are v1.2 compliant but
> we would not need the helper as no calls would be forwarded.
> 
> When a 32-bit guest is running on Xen Arm64, we are going to say that SMCCC
> v1.2 will be available. This is not much different from running a 32-bit guest
> on 32-bit hardware.

In a few places (especially platform specific code, such as Xilinx EEMI)
the guest SMC call traps in Xen, then Xen repeats the same SMC call to
the firmware.

I realize this is not a good reason to keep virtual SMCCC 1.1 because
the firmware could support SMCCC 1.0 or older. Some argument conversions
are to be expected.  In reality I have been working with SMCCC 1.1
virtual and SMCCC 1.1 firmware for a long time so the problem didn't
exist, and I didn't really think it through :-)

> So I think we should expose 1.2 unless we think there is a
> problem in the Xen 32-bit specific code.

Yeah, I am OK with that.