[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 1/2] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
On Wed, Jun 15, 2022 at 08:01:28PM +0100, Julien Grall wrote: > Hi, > > On 15/06/2022 16:58, Jens Wiklander wrote: > > On Fri, Jun 10, 2022 at 05:41:33PM -0700, 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 couldn't find an ARM32 implementation for ARM_SMCCC_VERSION_1_2. But > > I'm not sure it's needed at this point. From what I've understood r4-17 > > are either preserved or updated by the function ID in question. So > > claiming ARM_SMCCC_VERSION_1_2 shouldn't break anything. > > So in Xen, we always take a snapshot of the registers on entry to the > hypervisor and only touch it when necessary. Therefore, it doesn't matter > whether we claim to be complaient with 1.1 or 1.2 based on the argument > passing convention. > > However, the spec is not only about arguments. For instance, SMCCC v1.1 also > added some mandatory functions (e.g. detection the version). I haven't > looked closely on whether the SMCCC v1.2 introduced such thing. Can you > confirm what mandatory feature comes with 1.2? There's a nice summary in a table at the end of the C version of DEN0028 you linked below. For SMCCC v1.2: Argument/Result register set: Permits calls to use R4—R7 as return register (Section 4.1). Permits calls to use X4—X17 as return registers(Section3.1). Permits calls to use X8—X17 as argument registers (Section 3.1). Introduces: SMCCC_ARCH_SOC_ID (Section 7.4) Deprecates: UID, Revision Queries on Arm Architecture Service (Section 6.2) Count Query on all services (Section 6.2) As far a I can tell nothing mandatory is introduced with version 1.2. > > Furthermore, your commit message explain why arm_smccc_1_2_smc() was > introduced. But it seems to miss some words about exposing SMCCC v2.1 to the > VM. > > In general, I think it is better to split the host support from the VM > support. The two are technically not independent (caller vs implementation) > and there are different problematics for each (see above for an example). I > don't think there are a lot to add here, so I would be OK to keep it in the > same patch with a few words. > > Lastly, can you provide a link to the spec in the commit message? This would > help to find the pdf easily. I think in this case, you are using > ARM DEN 0028C (this is not the latest but describe 1.2): > > https://developer.arm.com/documentation/den0028/c/?lang=en I'll update the commit message. Thanks, Jens
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |