| Message ID | 20180215150248.28922-4-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update | expand |
On Thu, 15 Feb 2018, Julien Grall wrote: > SMCCC 1.1 offers firmware-based CPU workarounds. In particular, > SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254 > (CVE-2017-5715). > > If the hypervisor has some mitigation for this issue, report that we > deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor > workaround on every guest exit. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes in v3: > - Fix minor conflict during rebase > > Changes in v2: > - Add Volodymyr's reviewed-by > --- > xen/arch/arm/vsmc.c | 22 ++++++++++++++++++++-- > xen/include/asm-arm/smccc.h | 6 ++++++ > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c > index 7ec492741b..40a80d5760 100644 > --- a/xen/arch/arm/vsmc.c > +++ b/xen/arch/arm/vsmc.c > @@ -18,6 +18,7 @@ > #include <xen/lib.h> > #include <xen/types.h> > #include <public/arch-arm/smccc.h> > +#include <asm/cpufeature.h> > #include <asm/monitor.h> > #include <asm/regs.h> > #include <asm/smccc.h> > @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs) > return true; > > case ARM_SMCCC_ARCH_FEATURES_FID: > - /* Nothing supported yet */ > - set_user_reg(regs, 0, ARM_SMCCC_NOT_SUPPORTED); > + { > + uint32_t arch_func_id = get_user_reg(regs, 1); > + int ret = ARM_SMCCC_NOT_SUPPORTED; > + > + switch ( arch_func_id ) > + { > + case ARM_SMCCC_ARCH_WORKAROUND_1_FID: > + if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) ) > + ret = 0; > + break; > + } > + > + set_user_reg(regs, 0, ret); > + > + return true; > + } > + > + case ARM_SMCCC_ARCH_WORKAROUND_1_FID: > + /* No return value */ > return true; > } > > diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h > index 629cc5150b..2951caa49d 100644 > --- a/xen/include/asm-arm/smccc.h > +++ b/xen/include/asm-arm/smccc.h > @@ -115,6 +115,12 @@ static inline uint32_t smccc_get_owner(register_t funcid) > ARM_SMCCC_OWNER_ARCH, \ > 0x1) > > +#define ARM_SMCCC_ARCH_WORKAROUND_1_FID \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_CONV_32, \ > + ARM_SMCCC_OWNER_ARCH, \ > + 0x8000) > + > /* SMCCC error codes */ > #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION (-1) > #define ARM_SMCCC_NOT_SUPPORTED (-1) > -- > 2.11.0 >
Hi, On 15/02/18 15:02, Julien Grall wrote: > SMCCC 1.1 offers firmware-based CPU workarounds. In particular, > SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254 > (CVE-2017-5715). > > If the hypervisor has some mitigation for this issue, report that we > deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor > workaround on every guest exit. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com> > > --- > Changes in v3: > - Fix minor conflict during rebase > > Changes in v2: > - Add Volodymyr's reviewed-by > --- > xen/arch/arm/vsmc.c | 22 ++++++++++++++++++++-- > xen/include/asm-arm/smccc.h | 6 ++++++ > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c > index 7ec492741b..40a80d5760 100644 > --- a/xen/arch/arm/vsmc.c > +++ b/xen/arch/arm/vsmc.c > @@ -18,6 +18,7 @@ > #include <xen/lib.h> > #include <xen/types.h> > #include <public/arch-arm/smccc.h> > +#include <asm/cpufeature.h> > #include <asm/monitor.h> > #include <asm/regs.h> > #include <asm/smccc.h> > @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs) > return true; > > case ARM_SMCCC_ARCH_FEATURES_FID: > - /* Nothing supported yet */ > - set_user_reg(regs, 0, ARM_SMCCC_NOT_SUPPORTED); > + { > + uint32_t arch_func_id = get_user_reg(regs, 1); Shouldn't the function identifier be in x0/r0? Cheers, Andre. > + int ret = ARM_SMCCC_NOT_SUPPORTED; > + > + switch ( arch_func_id ) > + { > + case ARM_SMCCC_ARCH_WORKAROUND_1_FID: > + if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) ) > + ret = 0; > + break; > + } > + > + set_user_reg(regs, 0, ret); > + > + return true; > + } > + > + case ARM_SMCCC_ARCH_WORKAROUND_1_FID: > + /* No return value */ > return true; > } > > diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h > index 629cc5150b..2951caa49d 100644 > --- a/xen/include/asm-arm/smccc.h > +++ b/xen/include/asm-arm/smccc.h > @@ -115,6 +115,12 @@ static inline uint32_t smccc_get_owner(register_t funcid) > ARM_SMCCC_OWNER_ARCH, \ > 0x1) > > +#define ARM_SMCCC_ARCH_WORKAROUND_1_FID \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_CONV_32, \ > + ARM_SMCCC_OWNER_ARCH, \ > + 0x8000) > + > /* SMCCC error codes */ > #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION (-1) > #define ARM_SMCCC_NOT_SUPPORTED (-1) >
On 21/02/18 16:34, Andre Przywara wrote: > Hi, Hi, > On 15/02/18 15:02, Julien Grall wrote: >> SMCCC 1.1 offers firmware-based CPU workarounds. In particular, >> SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254 >> (CVE-2017-5715). >> >> If the hypervisor has some mitigation for this issue, report that we >> deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor >> workaround on every guest exit. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com> >> >> --- >> Changes in v3: >> - Fix minor conflict during rebase >> >> Changes in v2: >> - Add Volodymyr's reviewed-by >> --- >> xen/arch/arm/vsmc.c | 22 ++++++++++++++++++++-- >> xen/include/asm-arm/smccc.h | 6 ++++++ >> 2 files changed, 26 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c >> index 7ec492741b..40a80d5760 100644 >> --- a/xen/arch/arm/vsmc.c >> +++ b/xen/arch/arm/vsmc.c >> @@ -18,6 +18,7 @@ >> #include <xen/lib.h> >> #include <xen/types.h> >> #include <public/arch-arm/smccc.h> >> +#include <asm/cpufeature.h> >> #include <asm/monitor.h> >> #include <asm/regs.h> >> #include <asm/smccc.h> >> @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs) >> return true; >> >> case ARM_SMCCC_ARCH_FEATURES_FID: >> - /* Nothing supported yet */ >> - set_user_reg(regs, 0, ARM_SMCCC_NOT_SUPPORTED); >> + { >> + uint32_t arch_func_id = get_user_reg(regs, 1); > > Shouldn't the function identifier be in x0/r0? x0/r0 contain the function identifier of the actual function (i.e ARM_SMCCC_ARCH_FEATURES_FID). What we want here is the arch_func_id to check if the firmware implement it. This is the first parameter of the function, according to the calling convention this will be stored in x1/r1. Cheers,
Hi, On 21/02/18 16:41, Julien Grall wrote: > > > On 21/02/18 16:34, Andre Przywara wrote: >> Hi, > > Hi, > >> On 15/02/18 15:02, Julien Grall wrote: >>> SMCCC 1.1 offers firmware-based CPU workarounds. In particular, >>> SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254 >>> (CVE-2017-5715). >>> >>> If the hypervisor has some mitigation for this issue, report that we >>> deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor >>> workaround on every guest exit. >>> >>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com> >>> >>> --- >>> Changes in v3: >>> - Fix minor conflict during rebase >>> >>> Changes in v2: >>> - Add Volodymyr's reviewed-by >>> --- >>> xen/arch/arm/vsmc.c | 22 ++++++++++++++++++++-- >>> xen/include/asm-arm/smccc.h | 6 ++++++ >>> 2 files changed, 26 insertions(+), 2 deletions(-) >>> >>> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c >>> index 7ec492741b..40a80d5760 100644 >>> --- a/xen/arch/arm/vsmc.c >>> +++ b/xen/arch/arm/vsmc.c >>> @@ -18,6 +18,7 @@ >>> #include <xen/lib.h> >>> #include <xen/types.h> >>> #include <public/arch-arm/smccc.h> >>> +#include <asm/cpufeature.h> >>> #include <asm/monitor.h> >>> #include <asm/regs.h> >>> #include <asm/smccc.h> >>> @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs) >>> return true; >>> case ARM_SMCCC_ARCH_FEATURES_FID: >>> - /* Nothing supported yet */ >>> - set_user_reg(regs, 0, ARM_SMCCC_NOT_SUPPORTED); >>> + { >>> + uint32_t arch_func_id = get_user_reg(regs, 1); >> >> Shouldn't the function identifier be in x0/r0? > > x0/r0 contain the function identifier of the actual function (i.e > ARM_SMCCC_ARCH_FEATURES_FID). What we want here is the arch_func_id to > check if the firmware implement it. This is the first parameter of the > function, according to the calling convention this will be stored in x1/r1. Ah, right. I guess the missing context in this patch confused me. Looks alright then: Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c index 7ec492741b..40a80d5760 100644 --- a/xen/arch/arm/vsmc.c +++ b/xen/arch/arm/vsmc.c @@ -18,6 +18,7 @@ #include <xen/lib.h> #include <xen/types.h> #include <public/arch-arm/smccc.h> +#include <asm/cpufeature.h> #include <asm/monitor.h> #include <asm/regs.h> #include <asm/smccc.h> @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs) return true; case ARM_SMCCC_ARCH_FEATURES_FID: - /* Nothing supported yet */ - set_user_reg(regs, 0, ARM_SMCCC_NOT_SUPPORTED); + { + uint32_t arch_func_id = get_user_reg(regs, 1); + int ret = ARM_SMCCC_NOT_SUPPORTED; + + switch ( arch_func_id ) + { + case ARM_SMCCC_ARCH_WORKAROUND_1_FID: + if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) ) + ret = 0; + break; + } + + set_user_reg(regs, 0, ret); + + return true; + } + + case ARM_SMCCC_ARCH_WORKAROUND_1_FID: + /* No return value */ return true; } diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h index 629cc5150b..2951caa49d 100644 --- a/xen/include/asm-arm/smccc.h +++ b/xen/include/asm-arm/smccc.h @@ -115,6 +115,12 @@ static inline uint32_t smccc_get_owner(register_t funcid) ARM_SMCCC_OWNER_ARCH, \ 0x1) +#define ARM_SMCCC_ARCH_WORKAROUND_1_FID \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_CONV_32, \ + ARM_SMCCC_OWNER_ARCH, \ + 0x8000) + /* SMCCC error codes */ #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION (-1) #define ARM_SMCCC_NOT_SUPPORTED (-1)