Re: [PATCH v3 18/18] arm64: Kill PSCI_GET_VERSION as a variant-2 workaround

[Hanjun Guo]

Hi Marc,

Thank you for keeping me in the loop, just minor comments below.

On 2018/2/1 19:46, Marc Zyngier wrote:
> Now that we've standardised on SMCCC v1.1 to perform the branch
> prediction invalidation, let's drop the previous band-aid.
> If vendors haven't updated their firmware to do SMCCC 1.1, they
> haven't updated PSCI either, so we don't loose anything.
> 
> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm64/kernel/bpi.S| 24 ---
>  arch/arm64/kernel/cpu_errata.c | 43 
> --
>  arch/arm64/kvm/hyp/switch.c| 14 --
>  3 files changed, 12 insertions(+), 69 deletions(-)
> 
> diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S
> index fdeed629f2c6..e5de33513b5d 100644
> --- a/arch/arm64/kernel/bpi.S
> +++ b/arch/arm64/kernel/bpi.S
> @@ -54,30 +54,6 @@ ENTRY(__bp_harden_hyp_vecs_start)
>   vectors __kvm_hyp_vector
>   .endr
>  ENTRY(__bp_harden_hyp_vecs_end)
> -ENTRY(__psci_hyp_bp_inval_start)
> - sub sp, sp, #(8 * 18)
> - stp x16, x17, [sp, #(16 * 0)]
> - stp x14, x15, [sp, #(16 * 1)]
> - stp x12, x13, [sp, #(16 * 2)]
> - stp x10, x11, [sp, #(16 * 3)]
> - stp x8, x9, [sp, #(16 * 4)]
> - stp x6, x7, [sp, #(16 * 5)]
> - stp x4, x5, [sp, #(16 * 6)]
> - stp x2, x3, [sp, #(16 * 7)]
> - stp x0, x1, [sp, #(16 * 8)]
> - mov x0, #0x8400
> - smc #0
> - ldp x16, x17, [sp, #(16 * 0)]
> - ldp x14, x15, [sp, #(16 * 1)]
> - ldp x12, x13, [sp, #(16 * 2)]
> - ldp x10, x11, [sp, #(16 * 3)]
> - ldp x8, x9, [sp, #(16 * 4)]
> - ldp x6, x7, [sp, #(16 * 5)]
> - ldp x4, x5, [sp, #(16 * 6)]
> - ldp x2, x3, [sp, #(16 * 7)]
> - ldp x0, x1, [sp, #(16 * 8)]
> - add sp, sp, #(8 * 18)
> -ENTRY(__psci_hyp_bp_inval_end)
>  
>  ENTRY(__qcom_hyp_sanitize_link_stack_start)
>   stp x29, x30, [sp, #-16]!
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 9e77809a3b23..b8279a11f57b 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -67,7 +67,6 @@ static int cpu_enable_trap_ctr_access(void *__unused)
>  DEFINE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, bp_hardening_data);
>  
>  #ifdef CONFIG_KVM
> -extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
>  extern char __qcom_hyp_sanitize_link_stack_start[];
>  extern char __qcom_hyp_sanitize_link_stack_end[];
>  extern char __smccc_workaround_1_smc_start[];
> @@ -116,8 +115,6 @@ static void __install_bp_hardening_cb(bp_hardening_cb_t 
> fn,
>   spin_unlock(&bp_lock);
>  }
>  #else
> -#define __psci_hyp_bp_inval_startNULL
> -#define __psci_hyp_bp_inval_end  NULL
>  #define __qcom_hyp_sanitize_link_stack_start NULL
>  #define __qcom_hyp_sanitize_link_stack_end   NULL
>  #define __smccc_workaround_1_smc_start   NULL
> @@ -164,14 +161,15 @@ static void call_hvc_arch_workaround_1(void)
>   arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
>  }
>  
> -static bool check_smccc_arch_workaround_1(const struct 
> arm64_cpu_capabilities *entry)
> +static int smccc_arch_workaround_1(void *data)
>  {
> + const struct arm64_cpu_capabilities *entry = data;
>   bp_hardening_cb_t cb;
>   void *smccc_start, *smccc_end;
>   struct arm_smccc_res res;
>  
>   if (!entry->matches(entry, SCOPE_LOCAL_CPU))

entry->matches() will be called twice in this function, another
one is in install_bp_hardening_cb() below, but install_bp_hardening_cb()
will be called in qcom_enable_link_stack_sanitization(), and
this is in the init path, so I think it's fine to keep as it is now.

> - return false;
> + return 0;
>  
>   if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
>   return false;

return 0;

> @@ -181,7 +179,7 @@ static bool check_smccc_arch_workaround_1(const struct 
> arm64_cpu_capabilities *e
>   arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
> ARM_SMCCC_ARCH_WORKAROUND_1, &res);
>   if (res.a0)
> - return false;
> + return 0;
>   cb = call_hvc_arch_workaround_1;
>   smccc_start = __smccc_workaround_1_hvc_start;
>   smccc_end = __smccc_workaround_1_hvc_end;
> @@ -191,35 +189,18 @@ static bool check_smccc_arch_workaround_1(const struct 
> arm64_cpu_capabilities *e
>   arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
> ARM_SMCCC_ARCH_WORKAROUND_1, &res);
>   if (res.a0)
> - return false;
> + return 0;
>   cb = call_smc_arch_workaround_1;
>   smccc_start = __smccc_workaround_1_smc_start;
>   smccc_end = __smccc_workaround_1_smc_end;
>   break;
>  
>   def

Re: [PATCH v2 16/16] arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Hanjun Guo]

On 2018/2/1 16:53, Marc Zyngier wrote:
[...]
 ... and actually, perhaps it makes sense for the
 SMCCC_ARCH_WORKAROUND_1 check to be completely independent of MIDR
 based errata matching?

 I.e., if SMCCC v1.1 and SMCCC_ARCH_WORKAROUND_1 are both implemented,
 we should probably invoke it even if the MIDR is not known to belong
 to an affected implementation.
>>>
>>> This would have an impact on big-little systems, for which there is
>>> often a bunch of unaffected CPUs.
>>
>> I think it's what we are doing now, SMCCC v1.1 didn't provide the ability
>> to report per-cpu SMCCC_ARCH_WORKAROUND_1, and it said:
>>  - The discovery call must return the same result on all PEs in the system.
>>  - In heterogeneous systems with some PEs that require mitigation and others
>>that do not, the firmware must provide a safe implementation of this
>>function on all PEs.
>>
>> So from the spec that it's the firmware to take care of unaffected CPUs,
>> to the kernel it's the same.
> 
> The spec makes it safe. The MIDR list makes it fast.

Got it, thank you for clarifying this.

Thanks
Hanjun

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 14/18] firmware/psci: Expose SMCCC version through psci_ops

[Ard Biesheuvel]

On 1 February 2018 at 11:46, Marc Zyngier  wrote:
> Since PSCI 1.0 allows the SMCCC version to be (indirectly) probed,
> let's do that at boot time, and expose the version of the calling
> convention as part of the psci_ops structure.
>
> Acked-by: Lorenzo Pieralisi 
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/firmware/psci.c | 19 +++
>  include/linux/psci.h|  6 ++
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index e9493da2b111..8631906c414c 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -61,6 +61,7 @@ bool psci_tos_resident_on(int cpu)
>
>  struct psci_operations psci_ops = {
> .conduit = PSCI_CONDUIT_NONE,
> +   .smccc_version = SMCCC_VERSION_1_0,
>  };
>
>  typedef unsigned long (psci_fn)(unsigned long, unsigned long,
> @@ -511,6 +512,23 @@ static void __init psci_init_migrate(void)
> pr_info("Trusted OS resident on physical CPU 0x%lx\n", cpuid);
>  }
>
> +static void __init psci_init_smccc(u32 ver)
> +{
> +   int feature;
> +
> +   feature = psci_features(ARM_SMCCC_VERSION_FUNC_ID);
> +
> +   if (feature != PSCI_RET_NOT_SUPPORTED) {
> +   ver = invoke_psci_fn(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
> +   if (ver != ARM_SMCCC_VERSION_1_1)
> +   psci_ops.smccc_version = SMCCC_VERSION_1_0;
> +   else
> +   psci_ops.smccc_version = SMCCC_VERSION_1_1;
> +   }
> +
> +   pr_info("SMC Calling Convention v1.%d\n", psci_ops.smccc_version);

This is a bit nasty: you are returning the numeric value of the enum
as the minor number, and hardcoding the major version number as 1,
while the return value of ARM_SMCCC_VERSION_FUNC_ID gives you the
exact numbers. I assume nobody is expecting SMCCC v2.3 anytime soon,
but it would still be a lot nicer to simply decode the value of 'ver'
(and make it default to ARM_SMCCC_VERSION_1_0 if the PSCI feature call
fails)


> +}
> +
>  static void __init psci_0_2_set_functions(void)
>  {
> pr_info("Using standard PSCI v0.2 function IDs\n");
> @@ -559,6 +577,7 @@ static int __init psci_probe(void)
> psci_init_migrate();
>
> if (PSCI_VERSION_MAJOR(ver) >= 1) {
> +   psci_init_smccc(ver);
> psci_init_cpu_suspend();
> psci_init_system_suspend();
> }
> diff --git a/include/linux/psci.h b/include/linux/psci.h
> index f2679e5faa4f..8b1b3b5935ab 100644
> --- a/include/linux/psci.h
> +++ b/include/linux/psci.h
> @@ -31,6 +31,11 @@ enum psci_conduit {
> PSCI_CONDUIT_HVC,
>  };
>
> +enum smccc_version {
> +   SMCCC_VERSION_1_0,
> +   SMCCC_VERSION_1_1,
> +};
> +
>  struct psci_operations {
> u32 (*get_version)(void);
> int (*cpu_suspend)(u32 state, unsigned long entry_point);
> @@ -41,6 +46,7 @@ struct psci_operations {
> unsigned long lowest_affinity_level);
> int (*migrate_info_type)(void);
> enum psci_conduit conduit;
> +   enum smccc_version smccc_version;
>  };
>
>  extern struct psci_operations psci_ops;
> --
> 2.14.2
>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 00/41] Optimize KVM/ARM for VHE systems

[Yury Norov]

On Thu, Feb 01, 2018 at 02:57:59PM +0100, Tomasz Nowicki wrote:
> Hi Christoffer,
> 
> I created simple module for VM kernel. It is spinning on PSCI version
> hypercall to measure the base exit cost as you suggested. Also, I measured
> CPU cycles for each loop and here are my results:
> 
> My setup:
> 1-socket ThunderX2 running VM - 1VCPU
> 
> Tested baselines:
> a) host kernel v4.15-rc3 and VM kernel v4.15-rc3
> b) host kernel v4.15-rc3 + vhe-optimize-v3-with-fixes and VM kernel
> v4.15-rc3
> 
> Module was loaded from VM and the results are presented in [%] relative to
> average CPU cycles spending on PSCI version hypercall for vanilla VHE host
> kernel v4.15-rc3:
> 
>  VHE  |  nVHE
> =
> baseline a)  100% |  130%
> =
> baseline a)  36%  |  123%
> 
> So I confirm significant performance improvement, especially for VHE case.
> Additionally, I run network throughput tests with vhost-net but for that
> case no differences.

Hi Tomasz,

Can you share your test?

Yury
 
> Thanks,
> Tomasz
> 
> On 12.01.2018 13:07, Christoffer Dall wrote:
> > This series redesigns parts of KVM/ARM to optimize the performance on
> > VHE systems.  The general approach is to try to do as little work as
> > possible when transitioning between the VM and the hypervisor.  This has
> > the benefit of lower latency when waiting for interrupts and delivering
> > virtual interrupts, and reduces the overhead of emulating behavior and
> > I/O in the host kernel.
> > 
> > Patches 01 through 06 are not VHE specific, but rework parts of KVM/ARM
> > that can be generally improved.  We then add infrastructure to move more
> > logic into vcpu_load and vcpu_put, we improve handling of VFP and debug
> > registers.
> > 
> > We then introduce a new world-switch function for VHE systems, which we
> > can tweak and optimize for VHE systems.  To do that, we rework a lot of
> > the system register save/restore handling and emulation code that may
> > need access to system registers, so that we can defer as many system
> > register save/restore operations to vcpu_load and vcpu_put, and move
> > this logic out of the VHE world switch function.
> > 
> > We then optimize the configuration of traps.  On non-VHE systems, both
> > the host and VM kernels run in EL1, but because the host kernel should
> > have full access to the underlying hardware, but the VM kernel should
> > not, we essentially make the host kernel more privileged than the VM
> > kernel despite them both running at the same privilege level by enabling
> > VE traps when entering the VM and disabling those traps when exiting the
> > VM.  On VHE systems, the host kernel runs in EL2 and has full access to
> > the hardware (as much as allowed by secure side software), and is
> > unaffected by the trap configuration.  That means we can configure the
> > traps for VMs running in EL1 once, and don't have to switch them on and
> > off for every entry/exit to/from the VM.
> > 
> > Finally, we improve our VGIC handling by moving all save/restore logic
> > out of the VHE world-switch, and we make it possible to truly only
> > evaluate if the AP list is empty and not do *any* VGIC work if that is
> > the case, and only do the minimal amount of work required in the course
> > of the VGIC processing when we have virtual interrupts in flight.
> > 
> > The patches are based on v4.15-rc3, v9 of the level-triggered mapped
> > interrupts support series [1], and the first five patches of James' SDEI
> > series [2].
> > 
> > I've given the patches a fair amount of testing on Thunder-X, Mustang,
> > Seattle, and TC2 (32-bit) for non-VHE testing, and tested VHE
> > functionality on the Foundation model, running both 64-bit VMs and
> > 32-bit VMs side-by-side and using both GICv3-on-GICv3 and
> > GICv2-on-GICv3.
> > 
> > The patches are also available in the vhe-optimize-v3 branch on my
> > kernel.org repository [3].  The vhe-optimize-v3-base branch contains
> > prerequisites of this series.
> > 
> > Changes since v2:
> >   - Rebased on v4.15-rc3.
> >   - Includes two additional patches that only does vcpu_load after
> > kvm_vcpu_first_run_init and only for KVM_RUN.
> >   - Addressed review comments from v2 (detailed changelogs are in the
> > individual patches).
> > 
> > Thanks,
> > -Christoffer
> > 
> > [1]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git 
> > level-mapped-v9
> > [2]: git://linux-arm.org/linux-jm.git sdei/v5/base
> > [3]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git 
> > vhe-optimize-v3
> > 
> > Christoffer Dall (40):
> >KVM: arm/arm64: Avoid vcpu_load for other vcpu ioctls than KVM_RUN
> >KVM: arm/arm64: Move vcpu_load call after kvm_vcpu_first_run_init
> >KVM: arm64: Avoid storing the vcpu pointer on the stack
> >KVM: arm64: Rework hyp_panic for VHE and non-VHE
> >KVM: arm/arm64: Get rid of vcpu->arch.irq_lines
> >KVM: arm/arm64: Add kvm_vcpu_load_sysregs and kvm_vcpu_put_sy

Re: [PATCH v3 00/18] arm64: Add SMCCC v1.1 support and CVE-2017-5715 (Spectre variant 2) mitigation

[Marc Zyngier]

On Thu, 01 Feb 2018 13:59:45 +,
Ard Biesheuvel wrote:
> 
> On 1 February 2018 at 11:46, Marc Zyngier  wrote:
> > ARM has recently published a SMC Calling Convention (SMCCC)
> > specification update[1] that provides an optimised calling convention
> > and optional, discoverable support for mitigating CVE-2017-5715. ARM
> > Trusted Firmware (ATF) has already gained such an implementation[2].
> >
> > This series addresses a few things:
> >
> > - It provides a KVM implementation of PSCI v1.0, which is a
> >   prerequisite for being able to discover SMCCC v1.1, together with a
> >   new userspace API to control the PSCI revision number that the guest
> >   sees.
> >
> > - It allows KVM to advertise SMCCC v1.1, which is de-facto supported
> >   already (it never corrupts any of the guest registers).
> >
> > - It implements KVM support for the ARCH_WORKAROUND_1 function that is
> >   used to mitigate CVE-2017-5715 in a guest (if such mitigation is
> >   available on the host).
> >
> > - It implements SMCCC v1.1 and ARCH_WORKAROUND_1 discovery support in
> >   the kernel itself.
> >
> > - It finally provides firmware callbacks for CVE-2017-5715 for both
> >   kernel and KVM and drop the initial PSCI_GET_VERSION based
> >   mitigation.
> >
> > Patch 1 is already merged, and included here for reference. Patches on
> > top of arm64/for-next/core. Tested on Seattle and Juno, the latter
> > with ATF implementing SMCCC v1.1.
> >
> > [1]: https://developer.arm.com/support/security-update/downloads/
> >
> > [2]: https://github.com/ARM-software/arm-trusted-firmware/pull/1240
> >
> > * From v2:
> >   - Fixed SMC handling in KVM
> >   - PSCI fixes and tidying up
> >   - SMCCC primitive rework for better code generation (both efficiency
> >   and correctness)
> >   - Remove PSCI_GET_VERSION as a mitigation vector
> >
> > * From v1:
> >   - Fixed 32bit build
> >   - Fix function number sign extension (Ard)
> >   - Inline SMCCC v1.1 primitives (cpp soup)
> >   - Prevent SMCCC spamming on feature probing
> >   - Random fixes and tidying up
> >
> > Marc Zyngier (18):
> >   arm64: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls
> >   arm: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls
> >   arm64: KVM: Increment PC after handling an SMC trap
> >   arm/arm64: KVM: Consolidate the PSCI include files
> >   arm/arm64: KVM: Add PSCI_VERSION helper
> >   arm/arm64: KVM: Add smccc accessors to PSCI code
> >   arm/arm64: KVM: Implement PSCI 1.0 support
> >   arm/arm64: KVM: Add PSCI version selection API
> >   arm/arm64: KVM: Advertise SMCCC v1.1
> >   arm/arm64: KVM: Turn kvm_psci_version into a static inline
> >   arm64: KVM: Report SMCCC_ARCH_WORKAROUND_1 BP hardening support
> >   arm64: KVM: Add SMCCC_ARCH_WORKAROUND_1 fast handling
> >   firmware/psci: Expose PSCI conduit
> >   firmware/psci: Expose SMCCC version through psci_ops
> >   arm/arm64: smccc: Make function identifiers an unsigned quantity
> >   arm/arm64: smccc: Implement SMCCC v1.1 inline primitive
> >   arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support
> >   arm64: Kill PSCI_GET_VERSION as a variant-2 workaround
> >
> 
> I have given this a spin on my Overdrive, and everything seems to work
> as expected, both in the host and in the guest (I single stepped
> through the guest to ensure that it gets the expected answer from the
> SMCCC feature info call)
> 
> Tested-by: Ard Biesheuvel 

Awesome, thanks Ard.

M.

-- 
Jazz is not dead, it just smell funny.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 16/18] arm/arm64: smccc: Implement SMCCC v1.1 inline primitive

[Robin Murphy]

On 01/02/18 13:54, Marc Zyngier wrote:

On 01/02/18 13:34, Robin Murphy wrote:

On 01/02/18 11:46, Marc Zyngier wrote:

One of the major improvement of SMCCC v1.1 is that it only clobbers
the first 4 registers, both on 32 and 64bit. This means that it
becomes very easy to provide an inline version of the SMC call
primitive, and avoid performing a function call to stash the
registers that would otherwise be clobbered by SMCCC v1.0.

Signed-off-by: Marc Zyngier 
---
   include/linux/arm-smccc.h | 143 
++
   1 file changed, 143 insertions(+)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index dd44d8458c04..575aabe85905 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -150,5 +150,148 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, 
unsigned long a1,
   
   #define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
   
+/* SMCCC v1.1 implementation madness follows */

+#ifdef CONFIG_ARM64
+
+#define SMCCC_SMC_INST "smc   #0"
+#define SMCCC_HVC_INST "hvc   #0"


Nit: Maybe the argument can go in the template and we just define the
instruction mnemonics here?


+
+#endif
+
+#ifdef CONFIG_ARM


#elif ?


Sure, why not.




+#include 
+#include 
+
+#define SMCCC_SMC_INST __SMC(0)
+#define SMCCC_HVC_INST __HVC(0)


Oh, I see, it was to line up with this :(

I do wonder if we could just embed an asm(".arch armv7-a+virt\n") (if
even necessary) for ARM, then take advantage of the common mnemonics for
all 3 instruction sets instead of needing manual encoding tricks? I
don't think we should ever be pulling this file in for non-v7 builds.

I suppose that strictly that appears to need binutils 2.21 rather than
the offical supported minimum of 2.20, but are people going to be
throwing SMCCC configs at antique toolchains in practice?


It has been an issue in the past, back when we merged KVM. We settled on
a hybrid solution where code outside of KVM would not rely on a newer
toolchain, hence the macros that Dave introduced. Maybe we've moved on
and we can take that bold step?


Either way I think we can happily throw that on the "future cleanup" 
pile right now as it's not directly relevant to the purpose of the 
patch; I'm sure we don't want to make potential backporting even more 
difficult.





+
+#endif
+
+#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
+
+#define __count_args(...)  \
+   ___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
+
+#define __constraint_write_0   \
+   "+r" (r0), "=&r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_1   \
+   "+r" (r0), "+r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_2   \
+   "+r" (r0), "+r" (r1), "+r" (r2), "=&r" (r3)
+#define __constraint_write_3   \
+   "+r" (r0), "+r" (r1), "+r" (r2), "+r" (r3)
+#define __constraint_write_4   __constraint_write_3
+#define __constraint_write_5   __constraint_write_4
+#define __constraint_write_6   __constraint_write_5
+#define __constraint_write_7   __constraint_write_6
+
+#define __constraint_read_0
+#define __constraint_read_1
+#define __constraint_read_2
+#define __constraint_read_3
+#define __constraint_read_4"r" (r4)
+#define __constraint_read_5__constraint_read_4, "r" (r5)
+#define __constraint_read_6__constraint_read_5, "r" (r6)
+#define __constraint_read_7__constraint_read_6, "r" (r7)
+
+#define __declare_arg_0(a0, res)   \
+   struct arm_smccc_res   *___res = res;   \


Looks like the declaration of ___res could simply be factored out to the
template...


Tried that. But...




+   register u32   r0 asm("r0") = a0; \
+   register unsigned long r1 asm("r1");  \
+   register unsigned long r2 asm("r2");  \
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_1(a0, a1, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0; \
+   register typeof(a1)r1 asm("r1") = a1; \
+   register unsigned long r2 asm("r2");  \
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_2(a0, a1, a2, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0; \
+   register typeof(a1)r1 asm("r1") = a1; \
+   register typeof(a2)r2 asm("r2") = a2; \
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_3(a0, a1, a2, a3, res) 

Re: [PATCH v3 00/18] arm64: Add SMCCC v1.1 support and CVE-2017-5715 (Spectre variant 2) mitigation

[Ard Biesheuvel]

On 1 February 2018 at 11:46, Marc Zyngier  wrote:
> ARM has recently published a SMC Calling Convention (SMCCC)
> specification update[1] that provides an optimised calling convention
> and optional, discoverable support for mitigating CVE-2017-5715. ARM
> Trusted Firmware (ATF) has already gained such an implementation[2].
>
> This series addresses a few things:
>
> - It provides a KVM implementation of PSCI v1.0, which is a
>   prerequisite for being able to discover SMCCC v1.1, together with a
>   new userspace API to control the PSCI revision number that the guest
>   sees.
>
> - It allows KVM to advertise SMCCC v1.1, which is de-facto supported
>   already (it never corrupts any of the guest registers).
>
> - It implements KVM support for the ARCH_WORKAROUND_1 function that is
>   used to mitigate CVE-2017-5715 in a guest (if such mitigation is
>   available on the host).
>
> - It implements SMCCC v1.1 and ARCH_WORKAROUND_1 discovery support in
>   the kernel itself.
>
> - It finally provides firmware callbacks for CVE-2017-5715 for both
>   kernel and KVM and drop the initial PSCI_GET_VERSION based
>   mitigation.
>
> Patch 1 is already merged, and included here for reference. Patches on
> top of arm64/for-next/core. Tested on Seattle and Juno, the latter
> with ATF implementing SMCCC v1.1.
>
> [1]: https://developer.arm.com/support/security-update/downloads/
>
> [2]: https://github.com/ARM-software/arm-trusted-firmware/pull/1240
>
> * From v2:
>   - Fixed SMC handling in KVM
>   - PSCI fixes and tidying up
>   - SMCCC primitive rework for better code generation (both efficiency
>   and correctness)
>   - Remove PSCI_GET_VERSION as a mitigation vector
>
> * From v1:
>   - Fixed 32bit build
>   - Fix function number sign extension (Ard)
>   - Inline SMCCC v1.1 primitives (cpp soup)
>   - Prevent SMCCC spamming on feature probing
>   - Random fixes and tidying up
>
> Marc Zyngier (18):
>   arm64: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls
>   arm: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls
>   arm64: KVM: Increment PC after handling an SMC trap
>   arm/arm64: KVM: Consolidate the PSCI include files
>   arm/arm64: KVM: Add PSCI_VERSION helper
>   arm/arm64: KVM: Add smccc accessors to PSCI code
>   arm/arm64: KVM: Implement PSCI 1.0 support
>   arm/arm64: KVM: Add PSCI version selection API
>   arm/arm64: KVM: Advertise SMCCC v1.1
>   arm/arm64: KVM: Turn kvm_psci_version into a static inline
>   arm64: KVM: Report SMCCC_ARCH_WORKAROUND_1 BP hardening support
>   arm64: KVM: Add SMCCC_ARCH_WORKAROUND_1 fast handling
>   firmware/psci: Expose PSCI conduit
>   firmware/psci: Expose SMCCC version through psci_ops
>   arm/arm64: smccc: Make function identifiers an unsigned quantity
>   arm/arm64: smccc: Implement SMCCC v1.1 inline primitive
>   arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support
>   arm64: Kill PSCI_GET_VERSION as a variant-2 workaround
>

I have given this a spin on my Overdrive, and everything seems to work
as expected, both in the host and in the guest (I single stepped
through the guest to ensure that it gets the expected answer from the
SMCCC feature info call)

Tested-by: Ard Biesheuvel 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 00/41] Optimize KVM/ARM for VHE systems

[Tomasz Nowicki]

Hi Christoffer,

I created simple module for VM kernel. It is spinning on PSCI version 
hypercall to measure the base exit cost as you suggested. Also, I 
measured CPU cycles for each loop and here are my results:


My setup:
1-socket ThunderX2 running VM - 1VCPU

Tested baselines:
a) host kernel v4.15-rc3 and VM kernel v4.15-rc3
b) host kernel v4.15-rc3 + vhe-optimize-v3-with-fixes and VM kernel 
v4.15-rc3


Module was loaded from VM and the results are presented in [%] relative 
to average CPU cycles spending on PSCI version hypercall for vanilla VHE 
host kernel v4.15-rc3:


 VHE  |  nVHE
=
baseline a)  100% |  130%
=
baseline a)  36%  |  123%

So I confirm significant performance improvement, especially for VHE 
case. Additionally, I run network throughput tests with vhost-net but 
for that case no differences.


Thanks,
Tomasz

On 12.01.2018 13:07, Christoffer Dall wrote:

This series redesigns parts of KVM/ARM to optimize the performance on
VHE systems.  The general approach is to try to do as little work as
possible when transitioning between the VM and the hypervisor.  This has
the benefit of lower latency when waiting for interrupts and delivering
virtual interrupts, and reduces the overhead of emulating behavior and
I/O in the host kernel.

Patches 01 through 06 are not VHE specific, but rework parts of KVM/ARM
that can be generally improved.  We then add infrastructure to move more
logic into vcpu_load and vcpu_put, we improve handling of VFP and debug
registers.

We then introduce a new world-switch function for VHE systems, which we
can tweak and optimize for VHE systems.  To do that, we rework a lot of
the system register save/restore handling and emulation code that may
need access to system registers, so that we can defer as many system
register save/restore operations to vcpu_load and vcpu_put, and move
this logic out of the VHE world switch function.

We then optimize the configuration of traps.  On non-VHE systems, both
the host and VM kernels run in EL1, but because the host kernel should
have full access to the underlying hardware, but the VM kernel should
not, we essentially make the host kernel more privileged than the VM
kernel despite them both running at the same privilege level by enabling
VE traps when entering the VM and disabling those traps when exiting the
VM.  On VHE systems, the host kernel runs in EL2 and has full access to
the hardware (as much as allowed by secure side software), and is
unaffected by the trap configuration.  That means we can configure the
traps for VMs running in EL1 once, and don't have to switch them on and
off for every entry/exit to/from the VM.

Finally, we improve our VGIC handling by moving all save/restore logic
out of the VHE world-switch, and we make it possible to truly only
evaluate if the AP list is empty and not do *any* VGIC work if that is
the case, and only do the minimal amount of work required in the course
of the VGIC processing when we have virtual interrupts in flight.

The patches are based on v4.15-rc3, v9 of the level-triggered mapped
interrupts support series [1], and the first five patches of James' SDEI
series [2].

I've given the patches a fair amount of testing on Thunder-X, Mustang,
Seattle, and TC2 (32-bit) for non-VHE testing, and tested VHE
functionality on the Foundation model, running both 64-bit VMs and
32-bit VMs side-by-side and using both GICv3-on-GICv3 and
GICv2-on-GICv3.

The patches are also available in the vhe-optimize-v3 branch on my
kernel.org repository [3].  The vhe-optimize-v3-base branch contains
prerequisites of this series.

Changes since v2:
  - Rebased on v4.15-rc3.
  - Includes two additional patches that only does vcpu_load after
kvm_vcpu_first_run_init and only for KVM_RUN.
  - Addressed review comments from v2 (detailed changelogs are in the
individual patches).

Thanks,
-Christoffer

[1]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git 
level-mapped-v9
[2]: git://linux-arm.org/linux-jm.git sdei/v5/base
[3]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git 
vhe-optimize-v3

Christoffer Dall (40):
   KVM: arm/arm64: Avoid vcpu_load for other vcpu ioctls than KVM_RUN
   KVM: arm/arm64: Move vcpu_load call after kvm_vcpu_first_run_init
   KVM: arm64: Avoid storing the vcpu pointer on the stack
   KVM: arm64: Rework hyp_panic for VHE and non-VHE
   KVM: arm/arm64: Get rid of vcpu->arch.irq_lines
   KVM: arm/arm64: Add kvm_vcpu_load_sysregs and kvm_vcpu_put_sysregs
   KVM: arm/arm64: Introduce vcpu_el1_is_32bit
   KVM: arm64: Defer restoring host VFP state to vcpu_put
   KVM: arm64: Move debug dirty flag calculation out of world switch
   KVM: arm64: Slightly improve debug save/restore functions
   KVM: arm64: Improve debug register save/restore flow
   KVM: arm64: Factor out fault info population and gic workarounds
   KVM: arm64: Introduce VHE-specific kvm_vcpu_run
   KVM: arm64: Remove kern_hyp_va() us

Re: [PATCH v3 16/18] arm/arm64: smccc: Implement SMCCC v1.1 inline primitive

[Marc Zyngier]

On 01/02/18 13:34, Robin Murphy wrote:
> On 01/02/18 11:46, Marc Zyngier wrote:
>> One of the major improvement of SMCCC v1.1 is that it only clobbers
>> the first 4 registers, both on 32 and 64bit. This means that it
>> becomes very easy to provide an inline version of the SMC call
>> primitive, and avoid performing a function call to stash the
>> registers that would otherwise be clobbered by SMCCC v1.0.
>>
>> Signed-off-by: Marc Zyngier 
>> ---
>>   include/linux/arm-smccc.h | 143 
>> ++
>>   1 file changed, 143 insertions(+)
>>
>> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
>> index dd44d8458c04..575aabe85905 100644
>> --- a/include/linux/arm-smccc.h
>> +++ b/include/linux/arm-smccc.h
>> @@ -150,5 +150,148 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, 
>> unsigned long a1,
>>   
>>   #define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
>>   
>> +/* SMCCC v1.1 implementation madness follows */
>> +#ifdef CONFIG_ARM64
>> +
>> +#define SMCCC_SMC_INST  "smc#0"
>> +#define SMCCC_HVC_INST  "hvc#0"
> 
> Nit: Maybe the argument can go in the template and we just define the 
> instruction mnemonics here?
> 
>> +
>> +#endif
>> +
>> +#ifdef CONFIG_ARM
> 
> #elif ?

Sure, why not.

> 
>> +#include 
>> +#include 
>> +
>> +#define SMCCC_SMC_INST  __SMC(0)
>> +#define SMCCC_HVC_INST  __HVC(0)
> 
> Oh, I see, it was to line up with this :(
> 
> I do wonder if we could just embed an asm(".arch armv7-a+virt\n") (if 
> even necessary) for ARM, then take advantage of the common mnemonics for 
> all 3 instruction sets instead of needing manual encoding tricks? I 
> don't think we should ever be pulling this file in for non-v7 builds.
> 
> I suppose that strictly that appears to need binutils 2.21 rather than 
> the offical supported minimum of 2.20, but are people going to be 
> throwing SMCCC configs at antique toolchains in practice?

It has been an issue in the past, back when we merged KVM. We settled on
a hybrid solution where code outside of KVM would not rely on a newer
toolchain, hence the macros that Dave introduced. Maybe we've moved on
and we can take that bold step?

> 
>> +
>> +#endif
>> +
>> +#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
>> +
>> +#define __count_args(...)   \
>> +___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
>> +
>> +#define __constraint_write_0
>> \
>> +"+r" (r0), "=&r" (r1), "=&r" (r2), "=&r" (r3)
>> +#define __constraint_write_1
>> \
>> +"+r" (r0), "+r" (r1), "=&r" (r2), "=&r" (r3)
>> +#define __constraint_write_2
>> \
>> +"+r" (r0), "+r" (r1), "+r" (r2), "=&r" (r3)
>> +#define __constraint_write_3
>> \
>> +"+r" (r0), "+r" (r1), "+r" (r2), "+r" (r3)
>> +#define __constraint_write_4__constraint_write_3
>> +#define __constraint_write_5__constraint_write_4
>> +#define __constraint_write_6__constraint_write_5
>> +#define __constraint_write_7__constraint_write_6
>> +
>> +#define __constraint_read_0
>> +#define __constraint_read_1
>> +#define __constraint_read_2
>> +#define __constraint_read_3
>> +#define __constraint_read_4 "r" (r4)
>> +#define __constraint_read_5 __constraint_read_4, "r" (r5)
>> +#define __constraint_read_6 __constraint_read_5, "r" (r6)
>> +#define __constraint_read_7 __constraint_read_6, "r" (r7)
>> +
>> +#define __declare_arg_0(a0, res)\
>> +struct arm_smccc_res   *___res = res;   \
> 
> Looks like the declaration of ___res could simply be factored out to the 
> template...

Tried that. But...

> 
>> +register u32   r0 asm("r0") = a0;   \
>> +register unsigned long r1 asm("r1");\
>> +register unsigned long r2 asm("r2");\
>> +register unsigned long r3 asm("r3")
>> +
>> +#define __declare_arg_1(a0, a1, res)
>> \
>> +struct arm_smccc_res   *___res = res;   \
>> +register u32   r0 asm("r0") = a0;   \
>> +register typeof(a1)r1 asm("r1") = a1;   \
>> +register unsigned long r2 asm("r2");\
>> +register unsigned long r3 asm("r3")
>> +
>> +#define __declare_arg_2(a0, a1, a2, res)\
>> +struct arm_smccc_res   *___res = res;   \
>> +register u32   r0 asm("r0") = a0;   \
>> +register typeof(a1)r1 asm("r1") = a1;   \
>> +register typeof(a2)r2 asm("r2") = a2;   \
>> +register unsigned long r3 asm("r3")
>> +
>> +#

Re: [PATCH] irqchip/gic-v3: Use wmb() instead of smb_wmb() in gic_raise_softirq()

[Adam Wallis]

On 2/1/2018 8:24 AM, Marc Zyngier wrote:
> On 01/02/18 12:55, Shanker Donthineni wrote:
>> Hi Will, Thanks for your quick reply.
>>
>> On 02/01/2018 04:33 AM, Will Deacon wrote:
>>> Hi Shanker,
>>>
>>> On Wed, Jan 31, 2018 at 06:03:42PM -0600, Shanker Donthineni wrote:
 A DMB instruction can be used to ensure the relative order of only
 memory accesses before and after the barrier. Since writes to system
 registers are not memory operations, barrier DMB is not sufficient
 for observability of memory accesses that occur before ICC_SGI1R_EL1
 writes.

 A DSB instruction ensures that no instructions that appear in program
 order after the DSB instruction, can execute until the DSB instruction
 has completed.

 Signed-off-by: Shanker Donthineni 
 ---
  drivers/irqchip/irq-gic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
 index b56c3e2..980ae8e 100644
 --- a/drivers/irqchip/irq-gic-v3.c
 +++ b/drivers/irqchip/irq-gic-v3.c
 @@ -688,7 +688,7 @@ static void gic_raise_softirq(const struct cpumask 
 *mask, unsigned int irq)
 * Ensure that stores to Normal memory are visible to the
 * other CPUs before issuing the IPI.
 */
 -  smp_wmb();
 +  wmb();
>>>
>>> I think this is the right thing to do and the smp_wmb() was accidentally
>>> pulled in here as a copy-paste from the GICv2 driver where it is sufficient
>>> in practice.
>>>
>>> Did you spot this by code inspection, or did the DMB actually cause
>>> observable failures? (trying to figure out whether or not this need to go
>>> to -stable).
>>>
>>
>> We've inspected the code because kernel was causing failures in 
>> scheduler/IPI_RESCHDULE.
>> After some time of debugging, we landed in GIC driver and found that the 
>> issue was due
>> to the DMB barrier. 
> 
> OK. I've applied this with a cc: stable and Will's Ack.
> 
>> Side note, we're also missing synchronization barriers in GIC driver after 
>> writing some
>> of the ICC_XXX system registers. I'm planning to post those changes for 
>> comments.
>>
>> e.g: gic_write_sgi1r(val) and gic_write_eoir(irqnr);
> 
> Thanks,
> 
>   M.
> 

Tested-by: Adam Wallis 

-- 
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 16/18] arm/arm64: smccc: Implement SMCCC v1.1 inline primitive

[Robin Murphy]

On 01/02/18 11:46, Marc Zyngier wrote:

One of the major improvement of SMCCC v1.1 is that it only clobbers
the first 4 registers, both on 32 and 64bit. This means that it
becomes very easy to provide an inline version of the SMC call
primitive, and avoid performing a function call to stash the
registers that would otherwise be clobbered by SMCCC v1.0.

Signed-off-by: Marc Zyngier 
---
  include/linux/arm-smccc.h | 143 ++
  1 file changed, 143 insertions(+)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index dd44d8458c04..575aabe85905 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -150,5 +150,148 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, 
unsigned long a1,
  
  #define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
  
+/* SMCCC v1.1 implementation madness follows */

+#ifdef CONFIG_ARM64
+
+#define SMCCC_SMC_INST "smc   #0"
+#define SMCCC_HVC_INST "hvc   #0"


Nit: Maybe the argument can go in the template and we just define the 
instruction mnemonics here?



+
+#endif
+
+#ifdef CONFIG_ARM


#elif ?


+#include 
+#include 
+
+#define SMCCC_SMC_INST __SMC(0)
+#define SMCCC_HVC_INST __HVC(0)


Oh, I see, it was to line up with this :(

I do wonder if we could just embed an asm(".arch armv7-a+virt\n") (if 
even necessary) for ARM, then take advantage of the common mnemonics for 
all 3 instruction sets instead of needing manual encoding tricks? I 
don't think we should ever be pulling this file in for non-v7 builds.


I suppose that strictly that appears to need binutils 2.21 rather than 
the offical supported minimum of 2.20, but are people going to be 
throwing SMCCC configs at antique toolchains in practice?



+
+#endif
+
+#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
+
+#define __count_args(...)  \
+   ___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
+
+#define __constraint_write_0   \
+   "+r" (r0), "=&r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_1   \
+   "+r" (r0), "+r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_2   \
+   "+r" (r0), "+r" (r1), "+r" (r2), "=&r" (r3)
+#define __constraint_write_3   \
+   "+r" (r0), "+r" (r1), "+r" (r2), "+r" (r3)
+#define __constraint_write_4   __constraint_write_3
+#define __constraint_write_5   __constraint_write_4
+#define __constraint_write_6   __constraint_write_5
+#define __constraint_write_7   __constraint_write_6
+
+#define __constraint_read_0
+#define __constraint_read_1
+#define __constraint_read_2
+#define __constraint_read_3
+#define __constraint_read_4"r" (r4)
+#define __constraint_read_5__constraint_read_4, "r" (r5)
+#define __constraint_read_6__constraint_read_5, "r" (r6)
+#define __constraint_read_7__constraint_read_6, "r" (r7)
+
+#define __declare_arg_0(a0, res)   \
+   struct arm_smccc_res   *___res = res;   \


Looks like the declaration of ___res could simply be factored out to the 
template...



+   register u32   r0 asm("r0") = a0; \
+   register unsigned long r1 asm("r1");  \
+   register unsigned long r2 asm("r2");  \
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_1(a0, a1, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0; \
+   register typeof(a1)r1 asm("r1") = a1; \
+   register unsigned long r2 asm("r2");  \
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_2(a0, a1, a2, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0; \
+   register typeof(a1)r1 asm("r1") = a1; \
+   register typeof(a2)r2 asm("r2") = a2; \
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_3(a0, a1, a2, a3, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0; \
+   register typeof(a1)r1 asm("r1") = a1; \
+   register typeof(a2)r2 asm("r2") = a2; \
+   register typeof(a3)r3 asm("r3") = a3
+
+#define __declare_arg_4(a0, a1, a2, a3, a4, res)   \
+   __declare_arg_3(a0, a1, a2, a3, res);   \
+   register typeof(a4) r4 asm("r4") = a4
+
+#define __declare_arg_5(

Re: [PATCH] irqchip/gic-v3: Use wmb() instead of smb_wmb() in gic_raise_softirq()

[Marc Zyngier]

On 01/02/18 12:55, Shanker Donthineni wrote:
> Hi Will, Thanks for your quick reply.
> 
> On 02/01/2018 04:33 AM, Will Deacon wrote:
>> Hi Shanker,
>>
>> On Wed, Jan 31, 2018 at 06:03:42PM -0600, Shanker Donthineni wrote:
>>> A DMB instruction can be used to ensure the relative order of only
>>> memory accesses before and after the barrier. Since writes to system
>>> registers are not memory operations, barrier DMB is not sufficient
>>> for observability of memory accesses that occur before ICC_SGI1R_EL1
>>> writes.
>>>
>>> A DSB instruction ensures that no instructions that appear in program
>>> order after the DSB instruction, can execute until the DSB instruction
>>> has completed.
>>>
>>> Signed-off-by: Shanker Donthineni 
>>> ---
>>>  drivers/irqchip/irq-gic-v3.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>>> index b56c3e2..980ae8e 100644
>>> --- a/drivers/irqchip/irq-gic-v3.c
>>> +++ b/drivers/irqchip/irq-gic-v3.c
>>> @@ -688,7 +688,7 @@ static void gic_raise_softirq(const struct cpumask 
>>> *mask, unsigned int irq)
>>>  * Ensure that stores to Normal memory are visible to the
>>>  * other CPUs before issuing the IPI.
>>>  */
>>> -   smp_wmb();
>>> +   wmb();
>>
>> I think this is the right thing to do and the smp_wmb() was accidentally
>> pulled in here as a copy-paste from the GICv2 driver where it is sufficient
>> in practice.
>>
>> Did you spot this by code inspection, or did the DMB actually cause
>> observable failures? (trying to figure out whether or not this need to go
>> to -stable).
>>
> 
> We've inspected the code because kernel was causing failures in 
> scheduler/IPI_RESCHDULE.
> After some time of debugging, we landed in GIC driver and found that the 
> issue was due
> to the DMB barrier. 

OK. I've applied this with a cc: stable and Will's Ack.

> Side note, we're also missing synchronization barriers in GIC driver after 
> writing some
> of the ICC_XXX system registers. I'm planning to post those changes for 
> comments.
> 
> e.g: gic_write_sgi1r(val) and gic_write_eoir(irqnr);

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] irqchip/gic-v3: Use wmb() instead of smb_wmb() in gic_raise_softirq()

[Shanker Donthineni]

Hi Will, Thanks for your quick reply.

On 02/01/2018 04:33 AM, Will Deacon wrote:
> Hi Shanker,
> 
> On Wed, Jan 31, 2018 at 06:03:42PM -0600, Shanker Donthineni wrote:
>> A DMB instruction can be used to ensure the relative order of only
>> memory accesses before and after the barrier. Since writes to system
>> registers are not memory operations, barrier DMB is not sufficient
>> for observability of memory accesses that occur before ICC_SGI1R_EL1
>> writes.
>>
>> A DSB instruction ensures that no instructions that appear in program
>> order after the DSB instruction, can execute until the DSB instruction
>> has completed.
>>
>> Signed-off-by: Shanker Donthineni 
>> ---
>>  drivers/irqchip/irq-gic-v3.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>> index b56c3e2..980ae8e 100644
>> --- a/drivers/irqchip/irq-gic-v3.c
>> +++ b/drivers/irqchip/irq-gic-v3.c
>> @@ -688,7 +688,7 @@ static void gic_raise_softirq(const struct cpumask 
>> *mask, unsigned int irq)
>>   * Ensure that stores to Normal memory are visible to the
>>   * other CPUs before issuing the IPI.
>>   */
>> -smp_wmb();
>> +wmb();
> 
> I think this is the right thing to do and the smp_wmb() was accidentally
> pulled in here as a copy-paste from the GICv2 driver where it is sufficient
> in practice.
> 
> Did you spot this by code inspection, or did the DMB actually cause
> observable failures? (trying to figure out whether or not this need to go
> to -stable).
> 

We've inspected the code because kernel was causing failures in 
scheduler/IPI_RESCHDULE.
After some time of debugging, we landed in GIC driver and found that the issue 
was due
to the DMB barrier. 

Side note, we're also missing synchronization barriers in GIC driver after 
writing some
of the ICC_XXX system registers. I'm planning to post those changes for 
comments.

e.g: gic_write_sgi1r(val) and gic_write_eoir(irqnr);
 

> Anyway:
> 
> Acked-by: Will Deacon 
> 
> Cheers,
> 
> Will
> 

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 14/18] firmware/psci: Expose SMCCC version through psci_ops

[Marc Zyngier]

On 01/02/18 12:32, Robin Murphy wrote:
> On 01/02/18 11:46, Marc Zyngier wrote:
>> Since PSCI 1.0 allows the SMCCC version to be (indirectly) probed,
>> let's do that at boot time, and expose the version of the calling
>> convention as part of the psci_ops structure.
>>
>> Acked-by: Lorenzo Pieralisi 
>> Signed-off-by: Marc Zyngier 
>> ---
>>   drivers/firmware/psci.c | 19 +++
>>   include/linux/psci.h|  6 ++
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
>> index e9493da2b111..8631906c414c 100644
>> --- a/drivers/firmware/psci.c
>> +++ b/drivers/firmware/psci.c
>> @@ -61,6 +61,7 @@ bool psci_tos_resident_on(int cpu)
>>   
>>   struct psci_operations psci_ops = {
>>  .conduit = PSCI_CONDUIT_NONE,
>> +.smccc_version = SMCCC_VERSION_1_0,
>>   };
>>   
>>   typedef unsigned long (psci_fn)(unsigned long, unsigned long,
>> @@ -511,6 +512,23 @@ static void __init psci_init_migrate(void)
>>  pr_info("Trusted OS resident on physical CPU 0x%lx\n", cpuid);
>>   }
>>   
>> +static void __init psci_init_smccc(u32 ver)
>> +{
>> +int feature;
>> +
>> +feature = psci_features(ARM_SMCCC_VERSION_FUNC_ID);
>> +
>> +if (feature != PSCI_RET_NOT_SUPPORTED) {
>> +ver = invoke_psci_fn(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
>> +if (ver != ARM_SMCCC_VERSION_1_1)
>> +psci_ops.smccc_version = SMCCC_VERSION_1_0;
> 
> AFAICS, unless you somehow run psci_probe() twice *and* have 
> schizophrenic firmware, this assignment now does precisely nothing.

That's a leftover of a previous tracing hack I had...  Embarrassing.

> With the condition flipped and the redundant else case removed (or an 
> explanation of why I'm wrong...)
> 
> Reviewed-by: Robin Murphy 

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 15/18] arm/arm64: smccc: Make function identifiers an unsigned quantity

[Ard Biesheuvel]

On 1 February 2018 at 12:40, Robin Murphy  wrote:
> On 01/02/18 11:46, Marc Zyngier wrote:
>>
>> Function identifiers are a 32bit, unsigned quantity. But we never
>> tell so to the compiler, resulting in the following:
>>
>>   4ac:   b26187e0mov x0, #0x8001
>>
>> We thus rely on the firmware narrowing it for us, which is not
>> always a reasonable expectation.
>
>
> I think technically it might be OK, since SMCCC states "A Function
> Identifier is passed in register W0.", which implies that a conforming
> implementation should also read w0, not x0, but it's certainly far easier to
> be completely right than to justify being possibly wrong.
>
> Reviewed-by: Robin Murphy 
>

In my case, the function identifier wasn't the issue, but the
argument, which, for SMCCC_ARCH_FEATURES is also defined as uint32_t,
but did end up being interpreted incorrectly by the SMCCCv1.1
implementation that is now upstream in ARM-TF



>
>> Cc: sta...@vger.kernel.org
>> Reported-by: Ard Biesheuvel 
>> Acked-by: Ard Biesheuvel 
>> Tested-by: Ard Biesheuvel 
>> Signed-off-by: Marc Zyngier 
>> ---
>>   include/linux/arm-smccc.h | 6 --
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
>> index e1ef944ef1da..dd44d8458c04 100644
>> --- a/include/linux/arm-smccc.h
>> +++ b/include/linux/arm-smccc.h
>> @@ -14,14 +14,16 @@
>>   #ifndef __LINUX_ARM_SMCCC_H
>>   #define __LINUX_ARM_SMCCC_H
>>   +#include 
>> +
>>   /*
>>* This file provides common defines for ARM SMC Calling Convention as
>>* specified in
>>* http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
>>*/
>>   -#define ARM_SMCCC_STD_CALL   0
>> -#define ARM_SMCCC_FAST_CALL1
>> +#define ARM_SMCCC_STD_CALL _AC(0,U)
>> +#define ARM_SMCCC_FAST_CALL_AC(1,U)
>>   #define ARM_SMCCC_TYPE_SHIFT  31
>> #define ARM_SMCCC_SMC_320
>>
>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 15/18] arm/arm64: smccc: Make function identifiers an unsigned quantity

[Robin Murphy]

On 01/02/18 11:46, Marc Zyngier wrote:

Function identifiers are a 32bit, unsigned quantity. But we never
tell so to the compiler, resulting in the following:

  4ac:   b26187e0mov x0, #0x8001

We thus rely on the firmware narrowing it for us, which is not
always a reasonable expectation.


I think technically it might be OK, since SMCCC states "A Function 
Identifier is passed in register W0.", which implies that a conforming 
implementation should also read w0, not x0, but it's certainly far 
easier to be completely right than to justify being possibly wrong.


Reviewed-by: Robin Murphy 


Cc: sta...@vger.kernel.org
Reported-by: Ard Biesheuvel 
Acked-by: Ard Biesheuvel 
Tested-by: Ard Biesheuvel 
Signed-off-by: Marc Zyngier 
---
  include/linux/arm-smccc.h | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index e1ef944ef1da..dd44d8458c04 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -14,14 +14,16 @@
  #ifndef __LINUX_ARM_SMCCC_H
  #define __LINUX_ARM_SMCCC_H
  
+#include 

+
  /*
   * This file provides common defines for ARM SMC Calling Convention as
   * specified in
   * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
   */
  
-#define ARM_SMCCC_STD_CALL		0

-#define ARM_SMCCC_FAST_CALL1
+#define ARM_SMCCC_STD_CALL _AC(0,U)
+#define ARM_SMCCC_FAST_CALL_AC(1,U)
  #define ARM_SMCCC_TYPE_SHIFT  31
  
  #define ARM_SMCCC_SMC_32		0



___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 14/18] firmware/psci: Expose SMCCC version through psci_ops

[Robin Murphy]

On 01/02/18 11:46, Marc Zyngier wrote:

Since PSCI 1.0 allows the SMCCC version to be (indirectly) probed,
let's do that at boot time, and expose the version of the calling
convention as part of the psci_ops structure.

Acked-by: Lorenzo Pieralisi 
Signed-off-by: Marc Zyngier 
---
  drivers/firmware/psci.c | 19 +++
  include/linux/psci.h|  6 ++
  2 files changed, 25 insertions(+)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index e9493da2b111..8631906c414c 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -61,6 +61,7 @@ bool psci_tos_resident_on(int cpu)
  
  struct psci_operations psci_ops = {

.conduit = PSCI_CONDUIT_NONE,
+   .smccc_version = SMCCC_VERSION_1_0,
  };
  
  typedef unsigned long (psci_fn)(unsigned long, unsigned long,

@@ -511,6 +512,23 @@ static void __init psci_init_migrate(void)
pr_info("Trusted OS resident on physical CPU 0x%lx\n", cpuid);
  }
  
+static void __init psci_init_smccc(u32 ver)

+{
+   int feature;
+
+   feature = psci_features(ARM_SMCCC_VERSION_FUNC_ID);
+
+   if (feature != PSCI_RET_NOT_SUPPORTED) {
+   ver = invoke_psci_fn(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
+   if (ver != ARM_SMCCC_VERSION_1_1)
+   psci_ops.smccc_version = SMCCC_VERSION_1_0;


AFAICS, unless you somehow run psci_probe() twice *and* have 
schizophrenic firmware, this assignment now does precisely nothing.


With the condition flipped and the redundant else case removed (or an 
explanation of why I'm wrong...)


Reviewed-by: Robin Murphy 


+   else
+   psci_ops.smccc_version = SMCCC_VERSION_1_1;
+   }
+
+   pr_info("SMC Calling Convention v1.%d\n", psci_ops.smccc_version);
+}
+
  static void __init psci_0_2_set_functions(void)
  {
pr_info("Using standard PSCI v0.2 function IDs\n");
@@ -559,6 +577,7 @@ static int __init psci_probe(void)
psci_init_migrate();
  
  	if (PSCI_VERSION_MAJOR(ver) >= 1) {

+   psci_init_smccc(ver);
psci_init_cpu_suspend();
psci_init_system_suspend();
}
diff --git a/include/linux/psci.h b/include/linux/psci.h
index f2679e5faa4f..8b1b3b5935ab 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -31,6 +31,11 @@ enum psci_conduit {
PSCI_CONDUIT_HVC,
  };
  
+enum smccc_version {

+   SMCCC_VERSION_1_0,
+   SMCCC_VERSION_1_1,
+};
+
  struct psci_operations {
u32 (*get_version)(void);
int (*cpu_suspend)(u32 state, unsigned long entry_point);
@@ -41,6 +46,7 @@ struct psci_operations {
unsigned long lowest_affinity_level);
int (*migrate_info_type)(void);
enum psci_conduit conduit;
+   enum smccc_version smccc_version;
  };
  
  extern struct psci_operations psci_ops;



___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 13/18] firmware/psci: Expose PSCI conduit

[Robin Murphy]

On 01/02/18 11:46, Marc Zyngier wrote:

In order to call into the firmware to apply workarounds, it is
useful to find out whether we're using HVC or SMC. Let's expose
this through the psci_ops.


Reviewed-by: Robin Murphy 


Acked-by: Lorenzo Pieralisi 
Signed-off-by: Marc Zyngier 
---
  drivers/firmware/psci.c | 28 +++-
  include/linux/psci.h|  7 +++
  2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 8b25d31e8401..e9493da2b111 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -59,7 +59,9 @@ bool psci_tos_resident_on(int cpu)
return cpu == resident_cpu;
  }
  
-struct psci_operations psci_ops;

+struct psci_operations psci_ops = {
+   .conduit = PSCI_CONDUIT_NONE,
+};
  
  typedef unsigned long (psci_fn)(unsigned long, unsigned long,

unsigned long, unsigned long);
@@ -210,6 +212,22 @@ static unsigned long psci_migrate_info_up_cpu(void)
  0, 0, 0);
  }
  
+static void set_conduit(enum psci_conduit conduit)

+{
+   switch (conduit) {
+   case PSCI_CONDUIT_HVC:
+   invoke_psci_fn = __invoke_psci_fn_hvc;
+   break;
+   case PSCI_CONDUIT_SMC:
+   invoke_psci_fn = __invoke_psci_fn_smc;
+   break;
+   default:
+   WARN(1, "Unexpected PSCI conduit %d\n", conduit);
+   }
+
+   psci_ops.conduit = conduit;
+}
+
  static int get_set_conduit_method(struct device_node *np)
  {
const char *method;
@@ -222,9 +240,9 @@ static int get_set_conduit_method(struct device_node *np)
}
  
  	if (!strcmp("hvc", method)) {

-   invoke_psci_fn = __invoke_psci_fn_hvc;
+   set_conduit(PSCI_CONDUIT_HVC);
} else if (!strcmp("smc", method)) {
-   invoke_psci_fn = __invoke_psci_fn_smc;
+   set_conduit(PSCI_CONDUIT_SMC);
} else {
pr_warn("invalid \"method\" property: %s\n", method);
return -EINVAL;
@@ -654,9 +672,9 @@ int __init psci_acpi_init(void)
pr_info("probing for conduit method from ACPI.\n");
  
  	if (acpi_psci_use_hvc())

-   invoke_psci_fn = __invoke_psci_fn_hvc;
+   set_conduit(PSCI_CONDUIT_HVC);
else
-   invoke_psci_fn = __invoke_psci_fn_smc;
+   set_conduit(PSCI_CONDUIT_SMC);
  
  	return psci_probe();

  }
diff --git a/include/linux/psci.h b/include/linux/psci.h
index f724fd8c78e8..f2679e5faa4f 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -25,6 +25,12 @@ bool psci_tos_resident_on(int cpu);
  int psci_cpu_init_idle(unsigned int cpu);
  int psci_cpu_suspend_enter(unsigned long index);
  
+enum psci_conduit {

+   PSCI_CONDUIT_NONE,
+   PSCI_CONDUIT_SMC,
+   PSCI_CONDUIT_HVC,
+};
+
  struct psci_operations {
u32 (*get_version)(void);
int (*cpu_suspend)(u32 state, unsigned long entry_point);
@@ -34,6 +40,7 @@ struct psci_operations {
int (*affinity_info)(unsigned long target_affinity,
unsigned long lowest_affinity_level);
int (*migrate_info_type)(void);
+   enum psci_conduit conduit;
  };
  
  extern struct psci_operations psci_ops;



___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 18/18] arm64: Kill PSCI_GET_VERSION as a variant-2 workaround

[Marc Zyngier]

Now that we've standardised on SMCCC v1.1 to perform the branch
prediction invalidation, let's drop the previous band-aid.
If vendors haven't updated their firmware to do SMCCC 1.1, they
haven't updated PSCI either, so we don't loose anything.

Signed-off-by: Marc Zyngier 
---
 arch/arm64/kernel/bpi.S| 24 ---
 arch/arm64/kernel/cpu_errata.c | 43 --
 arch/arm64/kvm/hyp/switch.c| 14 --
 3 files changed, 12 insertions(+), 69 deletions(-)

diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S
index fdeed629f2c6..e5de33513b5d 100644
--- a/arch/arm64/kernel/bpi.S
+++ b/arch/arm64/kernel/bpi.S
@@ -54,30 +54,6 @@ ENTRY(__bp_harden_hyp_vecs_start)
vectors __kvm_hyp_vector
.endr
 ENTRY(__bp_harden_hyp_vecs_end)
-ENTRY(__psci_hyp_bp_inval_start)
-   sub sp, sp, #(8 * 18)
-   stp x16, x17, [sp, #(16 * 0)]
-   stp x14, x15, [sp, #(16 * 1)]
-   stp x12, x13, [sp, #(16 * 2)]
-   stp x10, x11, [sp, #(16 * 3)]
-   stp x8, x9, [sp, #(16 * 4)]
-   stp x6, x7, [sp, #(16 * 5)]
-   stp x4, x5, [sp, #(16 * 6)]
-   stp x2, x3, [sp, #(16 * 7)]
-   stp x0, x1, [sp, #(16 * 8)]
-   mov x0, #0x8400
-   smc #0
-   ldp x16, x17, [sp, #(16 * 0)]
-   ldp x14, x15, [sp, #(16 * 1)]
-   ldp x12, x13, [sp, #(16 * 2)]
-   ldp x10, x11, [sp, #(16 * 3)]
-   ldp x8, x9, [sp, #(16 * 4)]
-   ldp x6, x7, [sp, #(16 * 5)]
-   ldp x4, x5, [sp, #(16 * 6)]
-   ldp x2, x3, [sp, #(16 * 7)]
-   ldp x0, x1, [sp, #(16 * 8)]
-   add sp, sp, #(8 * 18)
-ENTRY(__psci_hyp_bp_inval_end)
 
 ENTRY(__qcom_hyp_sanitize_link_stack_start)
stp x29, x30, [sp, #-16]!
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 9e77809a3b23..b8279a11f57b 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -67,7 +67,6 @@ static int cpu_enable_trap_ctr_access(void *__unused)
 DEFINE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, bp_hardening_data);
 
 #ifdef CONFIG_KVM
-extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
 extern char __qcom_hyp_sanitize_link_stack_start[];
 extern char __qcom_hyp_sanitize_link_stack_end[];
 extern char __smccc_workaround_1_smc_start[];
@@ -116,8 +115,6 @@ static void __install_bp_hardening_cb(bp_hardening_cb_t fn,
spin_unlock(&bp_lock);
 }
 #else
-#define __psci_hyp_bp_inval_start  NULL
-#define __psci_hyp_bp_inval_endNULL
 #define __qcom_hyp_sanitize_link_stack_start   NULL
 #define __qcom_hyp_sanitize_link_stack_end NULL
 #define __smccc_workaround_1_smc_start NULL
@@ -164,14 +161,15 @@ static void call_hvc_arch_workaround_1(void)
arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
 }
 
-static bool check_smccc_arch_workaround_1(const struct arm64_cpu_capabilities 
*entry)
+static int smccc_arch_workaround_1(void *data)
 {
+   const struct arm64_cpu_capabilities *entry = data;
bp_hardening_cb_t cb;
void *smccc_start, *smccc_end;
struct arm_smccc_res res;
 
if (!entry->matches(entry, SCOPE_LOCAL_CPU))
-   return false;
+   return 0;
 
if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
return false;
@@ -181,7 +179,7 @@ static bool check_smccc_arch_workaround_1(const struct 
arm64_cpu_capabilities *e
arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
if (res.a0)
-   return false;
+   return 0;
cb = call_hvc_arch_workaround_1;
smccc_start = __smccc_workaround_1_hvc_start;
smccc_end = __smccc_workaround_1_hvc_end;
@@ -191,35 +189,18 @@ static bool check_smccc_arch_workaround_1(const struct 
arm64_cpu_capabilities *e
arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
if (res.a0)
-   return false;
+   return 0;
cb = call_smc_arch_workaround_1;
smccc_start = __smccc_workaround_1_smc_start;
smccc_end = __smccc_workaround_1_smc_end;
break;
 
default:
-   return false;
+   return 0;
}
 
install_bp_hardening_cb(entry, cb, smccc_start, smccc_end);
 
-   return true;
-}
-
-static int enable_psci_bp_hardening(void *data)
-{
-   const struct arm64_cpu_capabilities *entry = data;
-
-   if (psci_ops.get_version) {
-   if (check_smccc_arch_workaround_1(entry))
-   return 0;
-
-   install_bp_hardening_cb(entry,
-  (bp_hardening

[PATCH v3 17/18] arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Marc Zyngier]

Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
It is lovely. Really.

Signed-off-by: Marc Zyngier 
---
 arch/arm64/kernel/bpi.S| 20 +
 arch/arm64/kernel/cpu_errata.c | 68 +-
 2 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S
index 76225c2611ea..fdeed629f2c6 100644
--- a/arch/arm64/kernel/bpi.S
+++ b/arch/arm64/kernel/bpi.S
@@ -17,6 +17,7 @@
  */
 
 #include 
+#include 
 
 .macro ventry target
.rept 31
@@ -85,3 +86,22 @@ ENTRY(__qcom_hyp_sanitize_link_stack_start)
.endr
ldp x29, x30, [sp], #16
 ENTRY(__qcom_hyp_sanitize_link_stack_end)
+
+.macro smccc_workaround_1 inst
+   sub sp, sp, #(8 * 4)
+   stp x2, x3, [sp, #(8 * 0)]
+   stp x0, x1, [sp, #(8 * 2)]
+   mov w0, #ARM_SMCCC_ARCH_WORKAROUND_1
+   \inst   #0
+   ldp x2, x3, [sp, #(8 * 0)]
+   ldp x0, x1, [sp, #(8 * 2)]
+   add sp, sp, #(8 * 4)
+.endm
+
+ENTRY(__smccc_workaround_1_smc_start)
+   smccc_workaround_1  smc
+ENTRY(__smccc_workaround_1_smc_end)
+
+ENTRY(__smccc_workaround_1_hvc_start)
+   smccc_workaround_1  hvc
+ENTRY(__smccc_workaround_1_hvc_end)
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index ed6881882231..9e77809a3b23 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -70,6 +70,10 @@ DEFINE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, 
bp_hardening_data);
 extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
 extern char __qcom_hyp_sanitize_link_stack_start[];
 extern char __qcom_hyp_sanitize_link_stack_end[];
+extern char __smccc_workaround_1_smc_start[];
+extern char __smccc_workaround_1_smc_end[];
+extern char __smccc_workaround_1_hvc_start[];
+extern char __smccc_workaround_1_hvc_end[];
 
 static void __copy_hyp_vect_bpi(int slot, const char *hyp_vecs_start,
const char *hyp_vecs_end)
@@ -116,6 +120,10 @@ static void __install_bp_hardening_cb(bp_hardening_cb_t fn,
 #define __psci_hyp_bp_inval_endNULL
 #define __qcom_hyp_sanitize_link_stack_start   NULL
 #define __qcom_hyp_sanitize_link_stack_end NULL
+#define __smccc_workaround_1_smc_start NULL
+#define __smccc_workaround_1_smc_end   NULL
+#define __smccc_workaround_1_hvc_start NULL
+#define __smccc_workaround_1_hvc_end   NULL
 
 static void __install_bp_hardening_cb(bp_hardening_cb_t fn,
  const char *hyp_vecs_start,
@@ -142,17 +150,75 @@ static void  install_bp_hardening_cb(const struct 
arm64_cpu_capabilities *entry,
__install_bp_hardening_cb(fn, hyp_vecs_start, hyp_vecs_end);
 }
 
+#include 
+#include 
 #include 
 
+static void call_smc_arch_workaround_1(void)
+{
+   arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+
+static void call_hvc_arch_workaround_1(void)
+{
+   arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+
+static bool check_smccc_arch_workaround_1(const struct arm64_cpu_capabilities 
*entry)
+{
+   bp_hardening_cb_t cb;
+   void *smccc_start, *smccc_end;
+   struct arm_smccc_res res;
+
+   if (!entry->matches(entry, SCOPE_LOCAL_CPU))
+   return false;
+
+   if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
+   return false;
+
+   switch (psci_ops.conduit) {
+   case PSCI_CONDUIT_HVC:
+   arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+   if (res.a0)
+   return false;
+   cb = call_hvc_arch_workaround_1;
+   smccc_start = __smccc_workaround_1_hvc_start;
+   smccc_end = __smccc_workaround_1_hvc_end;
+   break;
+
+   case PSCI_CONDUIT_SMC:
+   arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+ ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+   if (res.a0)
+   return false;
+   cb = call_smc_arch_workaround_1;
+   smccc_start = __smccc_workaround_1_smc_start;
+   smccc_end = __smccc_workaround_1_smc_end;
+   break;
+
+   default:
+   return false;
+   }
+
+   install_bp_hardening_cb(entry, cb, smccc_start, smccc_end);
+
+   return true;
+}
+
 static int enable_psci_bp_hardening(void *data)
 {
const struct arm64_cpu_capabilities *entry = data;
 
-   if (psci_ops.get_version)
+   if (psci_ops.get_version) {
+   if (check_smccc_arch_workaround_1(entry))
+   return 0;
+
install_bp_hardening_cb(entry,
   (bp_hardening_cb_t)psci_ops.get_version,
   __psci_hyp_bp_inval_start,
  

[PATCH v3 16/18] arm/arm64: smccc: Implement SMCCC v1.1 inline primitive

[Marc Zyngier]

One of the major improvement of SMCCC v1.1 is that it only clobbers
the first 4 registers, both on 32 and 64bit. This means that it
becomes very easy to provide an inline version of the SMC call
primitive, and avoid performing a function call to stash the
registers that would otherwise be clobbered by SMCCC v1.0.

Signed-off-by: Marc Zyngier 
---
 include/linux/arm-smccc.h | 143 ++
 1 file changed, 143 insertions(+)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index dd44d8458c04..575aabe85905 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -150,5 +150,148 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, 
unsigned long a1,
 
 #define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
 
+/* SMCCC v1.1 implementation madness follows */
+#ifdef CONFIG_ARM64
+
+#define SMCCC_SMC_INST "smc#0"
+#define SMCCC_HVC_INST "hvc#0"
+
+#endif
+
+#ifdef CONFIG_ARM
+#include 
+#include 
+
+#define SMCCC_SMC_INST __SMC(0)
+#define SMCCC_HVC_INST __HVC(0)
+
+#endif
+
+#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
+
+#define __count_args(...)  \
+   ___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
+
+#define __constraint_write_0   \
+   "+r" (r0), "=&r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_1   \
+   "+r" (r0), "+r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_2   \
+   "+r" (r0), "+r" (r1), "+r" (r2), "=&r" (r3)
+#define __constraint_write_3   \
+   "+r" (r0), "+r" (r1), "+r" (r2), "+r" (r3)
+#define __constraint_write_4   __constraint_write_3
+#define __constraint_write_5   __constraint_write_4
+#define __constraint_write_6   __constraint_write_5
+#define __constraint_write_7   __constraint_write_6
+
+#define __constraint_read_0
+#define __constraint_read_1
+#define __constraint_read_2
+#define __constraint_read_3
+#define __constraint_read_4"r" (r4)
+#define __constraint_read_5__constraint_read_4, "r" (r5)
+#define __constraint_read_6__constraint_read_5, "r" (r6)
+#define __constraint_read_7__constraint_read_6, "r" (r7)
+
+#define __declare_arg_0(a0, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0;   \
+   register unsigned long r1 asm("r1");\
+   register unsigned long r2 asm("r2");\
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_1(a0, a1, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0;   \
+   register typeof(a1)r1 asm("r1") = a1;   \
+   register unsigned long r2 asm("r2");\
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_2(a0, a1, a2, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0;   \
+   register typeof(a1)r1 asm("r1") = a1;   \
+   register typeof(a2)r2 asm("r2") = a2;   \
+   register unsigned long r3 asm("r3")
+
+#define __declare_arg_3(a0, a1, a2, a3, res)   \
+   struct arm_smccc_res   *___res = res;   \
+   register u32   r0 asm("r0") = a0;   \
+   register typeof(a1)r1 asm("r1") = a1;   \
+   register typeof(a2)r2 asm("r2") = a2;   \
+   register typeof(a3)r3 asm("r3") = a3
+
+#define __declare_arg_4(a0, a1, a2, a3, a4, res)   \
+   __declare_arg_3(a0, a1, a2, a3, res);   \
+   register typeof(a4) r4 asm("r4") = a4
+
+#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)   \
+   __declare_arg_4(a0, a1, a2, a3, a4, res);   \
+   register typeof(a5) r5 asm("r5") = a5
+
+#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)   \
+   __declare_arg_5(a0, a1, a2, a3, a4, a5, res);   \
+   register typeof(a6) r6 asm("r6") = a6
+
+#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)   \
+   __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);   \
+   register typeof(a7) r7 asm("r7") = a7
+
+#define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
+#define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
+
+#define ___constraints(count) 

[PATCH v3 13/18] firmware/psci: Expose PSCI conduit

[Marc Zyngier]

In order to call into the firmware to apply workarounds, it is
useful to find out whether we're using HVC or SMC. Let's expose
this through the psci_ops.

Acked-by: Lorenzo Pieralisi 
Signed-off-by: Marc Zyngier 
---
 drivers/firmware/psci.c | 28 +++-
 include/linux/psci.h|  7 +++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 8b25d31e8401..e9493da2b111 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -59,7 +59,9 @@ bool psci_tos_resident_on(int cpu)
return cpu == resident_cpu;
 }
 
-struct psci_operations psci_ops;
+struct psci_operations psci_ops = {
+   .conduit = PSCI_CONDUIT_NONE,
+};
 
 typedef unsigned long (psci_fn)(unsigned long, unsigned long,
unsigned long, unsigned long);
@@ -210,6 +212,22 @@ static unsigned long psci_migrate_info_up_cpu(void)
  0, 0, 0);
 }
 
+static void set_conduit(enum psci_conduit conduit)
+{
+   switch (conduit) {
+   case PSCI_CONDUIT_HVC:
+   invoke_psci_fn = __invoke_psci_fn_hvc;
+   break;
+   case PSCI_CONDUIT_SMC:
+   invoke_psci_fn = __invoke_psci_fn_smc;
+   break;
+   default:
+   WARN(1, "Unexpected PSCI conduit %d\n", conduit);
+   }
+
+   psci_ops.conduit = conduit;
+}
+
 static int get_set_conduit_method(struct device_node *np)
 {
const char *method;
@@ -222,9 +240,9 @@ static int get_set_conduit_method(struct device_node *np)
}
 
if (!strcmp("hvc", method)) {
-   invoke_psci_fn = __invoke_psci_fn_hvc;
+   set_conduit(PSCI_CONDUIT_HVC);
} else if (!strcmp("smc", method)) {
-   invoke_psci_fn = __invoke_psci_fn_smc;
+   set_conduit(PSCI_CONDUIT_SMC);
} else {
pr_warn("invalid \"method\" property: %s\n", method);
return -EINVAL;
@@ -654,9 +672,9 @@ int __init psci_acpi_init(void)
pr_info("probing for conduit method from ACPI.\n");
 
if (acpi_psci_use_hvc())
-   invoke_psci_fn = __invoke_psci_fn_hvc;
+   set_conduit(PSCI_CONDUIT_HVC);
else
-   invoke_psci_fn = __invoke_psci_fn_smc;
+   set_conduit(PSCI_CONDUIT_SMC);
 
return psci_probe();
 }
diff --git a/include/linux/psci.h b/include/linux/psci.h
index f724fd8c78e8..f2679e5faa4f 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -25,6 +25,12 @@ bool psci_tos_resident_on(int cpu);
 int psci_cpu_init_idle(unsigned int cpu);
 int psci_cpu_suspend_enter(unsigned long index);
 
+enum psci_conduit {
+   PSCI_CONDUIT_NONE,
+   PSCI_CONDUIT_SMC,
+   PSCI_CONDUIT_HVC,
+};
+
 struct psci_operations {
u32 (*get_version)(void);
int (*cpu_suspend)(u32 state, unsigned long entry_point);
@@ -34,6 +40,7 @@ struct psci_operations {
int (*affinity_info)(unsigned long target_affinity,
unsigned long lowest_affinity_level);
int (*migrate_info_type)(void);
+   enum psci_conduit conduit;
 };
 
 extern struct psci_operations psci_ops;
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 15/18] arm/arm64: smccc: Make function identifiers an unsigned quantity

[Marc Zyngier]

Function identifiers are a 32bit, unsigned quantity. But we never
tell so to the compiler, resulting in the following:

 4ac:   b26187e0mov x0, #0x8001

We thus rely on the firmware narrowing it for us, which is not
always a reasonable expectation.

Cc: sta...@vger.kernel.org
Reported-by: Ard Biesheuvel 
Acked-by: Ard Biesheuvel 
Tested-by: Ard Biesheuvel 
Signed-off-by: Marc Zyngier 
---
 include/linux/arm-smccc.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index e1ef944ef1da..dd44d8458c04 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -14,14 +14,16 @@
 #ifndef __LINUX_ARM_SMCCC_H
 #define __LINUX_ARM_SMCCC_H
 
+#include 
+
 /*
  * This file provides common defines for ARM SMC Calling Convention as
  * specified in
  * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
  */
 
-#define ARM_SMCCC_STD_CALL 0
-#define ARM_SMCCC_FAST_CALL1
+#define ARM_SMCCC_STD_CALL _AC(0,U)
+#define ARM_SMCCC_FAST_CALL_AC(1,U)
 #define ARM_SMCCC_TYPE_SHIFT   31
 
 #define ARM_SMCCC_SMC_32   0
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 12/18] arm64: KVM: Add SMCCC_ARCH_WORKAROUND_1 fast handling

[Marc Zyngier]

We want SMCCC_ARCH_WORKAROUND_1 to be fast. As fast as possible.
So let's intercept it as early as we can by testing for the
function call number as soon as we've identified a HVC call
coming from the guest.

Signed-off-by: Marc Zyngier 
---
 arch/arm64/kvm/hyp/hyp-entry.S | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index e4f37b9dd47c..f36464bd57c5 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -15,6 +15,7 @@
  * along with this program.  If not, see .
  */
 
+#include 
 #include 
 
 #include 
@@ -64,10 +65,11 @@ alternative_endif
lsr x0, x1, #ESR_ELx_EC_SHIFT
 
cmp x0, #ESR_ELx_EC_HVC64
+   ccmpx0, #ESR_ELx_EC_HVC32, #4, ne
b.neel1_trap
 
-   mrs x1, vttbr_el2   // If vttbr is valid, the 64bit guest
-   cbnzx1, el1_trap// called HVC
+   mrs x1, vttbr_el2   // If vttbr is valid, the guest
+   cbnzx1, el1_hvc_guest   // called HVC
 
/* Here, we're pretty sure the host called HVC. */
ldp x0, x1, [sp], #16
@@ -100,6 +102,20 @@ alternative_endif
 
eret
 
+el1_hvc_guest:
+   /*
+* Fastest possible path for ARM_SMCCC_ARCH_WORKAROUND_1.
+* The workaround has already been applied on the host,
+* so let's quickly get back to the guest. We don't bother
+* restoring x1, as it can be clobbered anyway.
+*/
+   ldr x1, [sp]// Guest's x0
+   eor w1, w1, #ARM_SMCCC_ARCH_WORKAROUND_1
+   cbnzw1, el1_trap
+   mov x0, x1
+   add sp, sp, #16
+   eret
+
 el1_trap:
/*
 * x0: ESR_EC
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 14/18] firmware/psci: Expose SMCCC version through psci_ops

[Marc Zyngier]

Since PSCI 1.0 allows the SMCCC version to be (indirectly) probed,
let's do that at boot time, and expose the version of the calling
convention as part of the psci_ops structure.

Acked-by: Lorenzo Pieralisi 
Signed-off-by: Marc Zyngier 
---
 drivers/firmware/psci.c | 19 +++
 include/linux/psci.h|  6 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index e9493da2b111..8631906c414c 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -61,6 +61,7 @@ bool psci_tos_resident_on(int cpu)
 
 struct psci_operations psci_ops = {
.conduit = PSCI_CONDUIT_NONE,
+   .smccc_version = SMCCC_VERSION_1_0,
 };
 
 typedef unsigned long (psci_fn)(unsigned long, unsigned long,
@@ -511,6 +512,23 @@ static void __init psci_init_migrate(void)
pr_info("Trusted OS resident on physical CPU 0x%lx\n", cpuid);
 }
 
+static void __init psci_init_smccc(u32 ver)
+{
+   int feature;
+
+   feature = psci_features(ARM_SMCCC_VERSION_FUNC_ID);
+
+   if (feature != PSCI_RET_NOT_SUPPORTED) {
+   ver = invoke_psci_fn(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
+   if (ver != ARM_SMCCC_VERSION_1_1)
+   psci_ops.smccc_version = SMCCC_VERSION_1_0;
+   else
+   psci_ops.smccc_version = SMCCC_VERSION_1_1;
+   }
+
+   pr_info("SMC Calling Convention v1.%d\n", psci_ops.smccc_version);
+}
+
 static void __init psci_0_2_set_functions(void)
 {
pr_info("Using standard PSCI v0.2 function IDs\n");
@@ -559,6 +577,7 @@ static int __init psci_probe(void)
psci_init_migrate();
 
if (PSCI_VERSION_MAJOR(ver) >= 1) {
+   psci_init_smccc(ver);
psci_init_cpu_suspend();
psci_init_system_suspend();
}
diff --git a/include/linux/psci.h b/include/linux/psci.h
index f2679e5faa4f..8b1b3b5935ab 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -31,6 +31,11 @@ enum psci_conduit {
PSCI_CONDUIT_HVC,
 };
 
+enum smccc_version {
+   SMCCC_VERSION_1_0,
+   SMCCC_VERSION_1_1,
+};
+
 struct psci_operations {
u32 (*get_version)(void);
int (*cpu_suspend)(u32 state, unsigned long entry_point);
@@ -41,6 +46,7 @@ struct psci_operations {
unsigned long lowest_affinity_level);
int (*migrate_info_type)(void);
enum psci_conduit conduit;
+   enum smccc_version smccc_version;
 };
 
 extern struct psci_operations psci_ops;
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 11/18] arm64: KVM: Report SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Marc Zyngier]

A new feature of SMCCC 1.1 is that it offers firmware-based CPU
workarounds. In particular, SMCCC_ARCH_WORKAROUND_1 provides
BP hardening for CVE-2017-5715.

If the host has some mitigation for this issue, report that
we deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the
host workaround on every guest exit.

Signed-off-by: Marc Zyngier 
---
 arch/arm/include/asm/kvm_host.h   | 7 +++
 arch/arm64/include/asm/kvm_host.h | 6 ++
 include/linux/arm-smccc.h | 5 +
 virt/kvm/arm/psci.c   | 9 -
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index e9d57060d88c..6c05e3b13081 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -309,4 +309,11 @@ static inline void kvm_fpsimd_flush_cpu_state(void) {}
 
 static inline void kvm_arm_vhe_guest_enter(void) {}
 static inline void kvm_arm_vhe_guest_exit(void) {}
+
+static inline bool kvm_arm_harden_branch_predictor(void)
+{
+   /* No way to detect it yet, pretend it is not there. */
+   return false;
+}
+
 #endif /* __ARM_KVM_HOST_H__ */
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 10af386642c6..448d3b9a58cb 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -418,4 +418,10 @@ static inline void kvm_arm_vhe_guest_exit(void)
 {
local_daif_restore(DAIF_PROCCTX_NOIRQ);
 }
+
+static inline bool kvm_arm_harden_branch_predictor(void)
+{
+   return cpus_have_const_cap(ARM64_HARDEN_BRANCH_PREDICTOR);
+}
+
 #endif /* __ARM64_KVM_HOST_H__ */
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index dc68aa5a7261..e1ef944ef1da 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -73,6 +73,11 @@
   ARM_SMCCC_SMC_32,\
   0, 1)
 
+#define ARM_SMCCC_ARCH_WORKAROUND_1\
+   ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+  ARM_SMCCC_SMC_32,\
+  0, 0x8000)
+
 #ifndef __ASSEMBLY__
 
 #include 
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 2efacbe7b1a2..22c24561d07d 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -406,13 +406,20 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 {
u32 func_id = smccc_get_function(vcpu);
u32 val = PSCI_RET_NOT_SUPPORTED;
+   u32 feature;
 
switch (func_id) {
case ARM_SMCCC_VERSION_FUNC_ID:
val = ARM_SMCCC_VERSION_1_1;
break;
case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
-   /* Nothing supported yet */
+   feature = smccc_get_arg1(vcpu);
+   switch(feature) {
+   case ARM_SMCCC_ARCH_WORKAROUND_1:
+   if (kvm_arm_harden_branch_predictor())
+   val = 0;
+   break;
+   }
break;
default:
return kvm_psci_call(vcpu);
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 10/18] arm/arm64: KVM: Turn kvm_psci_version into a static inline

[Marc Zyngier]

We're about to need kvm_psci_version in HYP too. So let's turn it
into a static inline, and pass the kvm structure as a second
parameter (so that HYP can do a kern_hyp_va on it).

Signed-off-by: Marc Zyngier 
---
 arch/arm64/kvm/hyp/switch.c | 20 
 include/kvm/arm_psci.h  | 26 +-
 virt/kvm/arm/psci.c | 25 +++--
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 036e1f3d77a6..408c04d789a5 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -19,6 +19,8 @@
 #include 
 #include 
 
+#include 
+
 #include 
 #include 
 #include 
@@ -350,14 +352,16 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 
if (exit_code == ARM_EXCEPTION_TRAP &&
(kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_HVC64 ||
-kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_HVC32) &&
-   vcpu_get_reg(vcpu, 0) == PSCI_0_2_FN_PSCI_VERSION) {
-   u64 val = PSCI_RET_NOT_SUPPORTED;
-   if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
-   val = 2;
-
-   vcpu_set_reg(vcpu, 0, val);
-   goto again;
+kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_HVC32)) {
+   u32 val = vcpu_get_reg(vcpu, 0);
+
+   if (val == PSCI_0_2_FN_PSCI_VERSION) {
+   val = kvm_psci_version(vcpu, kern_hyp_va(vcpu->kvm));
+   if (unlikely(val == KVM_ARM_PSCI_0_1))
+   val = PSCI_RET_NOT_SUPPORTED;
+   vcpu_set_reg(vcpu, 0, val);
+   goto again;
+   }
}
 
if (static_branch_unlikely(&vgic_v2_cpuif_trap) &&
diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index 7b2e12697d4f..9b699f91171f 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -18,6 +18,7 @@
 #ifndef __KVM_ARM_PSCI_H__
 #define __KVM_ARM_PSCI_H__
 
+#include 
 #include 
 
 #define KVM_ARM_PSCI_0_1   PSCI_VERSION(0, 1)
@@ -26,7 +27,30 @@
 
 #define KVM_ARM_PSCI_LATESTKVM_ARM_PSCI_1_0
 
-int kvm_psci_version(struct kvm_vcpu *vcpu);
+/*
+ * We need the KVM pointer independently from the vcpu as we can call
+ * this from HYP, and need to apply kern_hyp_va on it...
+ */
+static inline int kvm_psci_version(struct kvm_vcpu *vcpu, struct kvm *kvm)
+{
+   /*
+* Our PSCI implementation stays the same across versions from
+* v0.2 onward, only adding the few mandatory functions (such
+* as FEATURES with 1.0) that are required by newer
+* revisions. It is thus safe to return the latest, unless
+* userspace has instructed us otherwise.
+*/
+   if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features)) {
+   if (kvm->arch.psci_version)
+   return kvm->arch.psci_version;
+
+   return KVM_ARM_PSCI_LATEST;
+   }
+
+   return KVM_ARM_PSCI_0_1;
+}
+
+
 int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
 
 struct kvm_one_reg;
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 53272e8e0d37..2efacbe7b1a2 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -124,7 +124,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu 
*source_vcpu)
if (!vcpu)
return PSCI_RET_INVALID_PARAMS;
if (!vcpu->arch.power_off) {
-   if (kvm_psci_version(source_vcpu) != KVM_ARM_PSCI_0_1)
+   if (kvm_psci_version(source_vcpu, kvm) != KVM_ARM_PSCI_0_1)
return PSCI_RET_ALREADY_ON;
else
return PSCI_RET_INVALID_PARAMS;
@@ -233,25 +233,6 @@ static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET);
 }
 
-int kvm_psci_version(struct kvm_vcpu *vcpu)
-{
-   /*
-* Our PSCI implementation stays the same across versions from
-* v0.2 onward, only adding the few mandatory functions (such
-* as FEATURES with 1.0) that are required by newer
-* revisions. It is thus safe to return the latest, unless
-* userspace has instructed us otherwise.
-*/
-   if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features)) {
-   if (vcpu->kvm->arch.psci_version)
-   return vcpu->kvm->arch.psci_version;
-
-   return KVM_ARM_PSCI_LATEST;
-   }
-
-   return KVM_ARM_PSCI_0_1;
-}
-
 static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 {
struct kvm *kvm = vcpu->kvm;
@@ -409,7 +390,7 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
  */
 static int kvm_psci_call(struct kvm_vcpu *vcpu)
 {
-   switch (kvm_psci_version(vcpu)) {
+   switch (kvm_psci_version(vcpu, vcpu->kvm)) {
case KVM_ARM_PSCI_1_0:
return kvm_psci_1_0_call(vcpu);
case KVM_ARM_PSCI_0_2:
@@ -46

[PATCH v3 09/18] arm/arm64: KVM: Advertise SMCCC v1.1

[Marc Zyngier]

The new SMC Calling Convention (v1.1) allows for a reduced overhead
when calling into the firmware, and provides a new feature discovery
mechanism.

Make it visible to KVM guests.

Signed-off-by: Marc Zyngier 
---
 arch/arm/kvm/handle_exit.c   |  2 +-
 arch/arm64/kvm/handle_exit.c |  2 +-
 include/kvm/arm_psci.h   |  2 +-
 include/linux/arm-smccc.h| 13 +
 virt/kvm/arm/psci.c  | 24 +++-
 5 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index 230ae4079108..910bd8dabb3c 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -36,7 +36,7 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run 
*run)
  kvm_vcpu_hvc_get_imm(vcpu));
vcpu->stat.hvc_exit_stat++;
 
-   ret = kvm_psci_call(vcpu);
+   ret = kvm_hvc_call_handler(vcpu);
if (ret < 0) {
vcpu_set_reg(vcpu, 0, ~0UL);
return 1;
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 588f910632a7..e5e741bfffe1 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -52,7 +52,7 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run 
*run)
kvm_vcpu_hvc_get_imm(vcpu));
vcpu->stat.hvc_exit_stat++;
 
-   ret = kvm_psci_call(vcpu);
+   ret = kvm_hvc_call_handler(vcpu);
if (ret < 0) {
vcpu_set_reg(vcpu, 0, ~0UL);
return 1;
diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index 4ee098c39e01..7b2e12697d4f 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -27,7 +27,7 @@
 #define KVM_ARM_PSCI_LATESTKVM_ARM_PSCI_1_0
 
 int kvm_psci_version(struct kvm_vcpu *vcpu);
-int kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
 
 struct kvm_one_reg;
 
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 4c5bca38c653..dc68aa5a7261 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -60,6 +60,19 @@
 #define ARM_SMCCC_QUIRK_NONE   0
 #define ARM_SMCCC_QUIRK_QCOM_A61 /* Save/restore register a6 */
 
+#define ARM_SMCCC_VERSION_1_0  0x1
+#define ARM_SMCCC_VERSION_1_1  0x10001
+
+#define ARM_SMCCC_VERSION_FUNC_ID  \
+   ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+  ARM_SMCCC_SMC_32,\
+  0, 0)
+
+#define ARM_SMCCC_ARCH_FEATURES_FUNC_ID
\
+   ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+  ARM_SMCCC_SMC_32,\
+  0, 1)
+
 #ifndef __ASSEMBLY__
 
 #include 
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 5c8366b71639..53272e8e0d37 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -15,6 +15,7 @@
  * along with this program.  If not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -351,6 +352,7 @@ static int kvm_psci_1_0_call(struct kvm_vcpu *vcpu)
case PSCI_0_2_FN_SYSTEM_OFF:
case PSCI_0_2_FN_SYSTEM_RESET:
case PSCI_1_0_FN_PSCI_FEATURES:
+   case ARM_SMCCC_VERSION_FUNC_ID:
val = 0;
break;
default:
@@ -405,7 +407,7 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
  * Errors:
  * -EINVAL: Unrecognized PSCI function
  */
-int kvm_psci_call(struct kvm_vcpu *vcpu)
+static int kvm_psci_call(struct kvm_vcpu *vcpu)
 {
switch (kvm_psci_version(vcpu)) {
case KVM_ARM_PSCI_1_0:
@@ -419,6 +421,26 @@ int kvm_psci_call(struct kvm_vcpu *vcpu)
};
 }
 
+int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
+{
+   u32 func_id = smccc_get_function(vcpu);
+   u32 val = PSCI_RET_NOT_SUPPORTED;
+
+   switch (func_id) {
+   case ARM_SMCCC_VERSION_FUNC_ID:
+   val = ARM_SMCCC_VERSION_1_1;
+   break;
+   case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
+   /* Nothing supported yet */
+   break;
+   default:
+   return kvm_psci_call(vcpu);
+   }
+
+   smccc_set_retval(vcpu, val, 0, 0, 0);
+   return 1;
+}
+
 int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
 {
return 1;   /* PSCI version */
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 05/18] arm/arm64: KVM: Add PSCI_VERSION helper

[Marc Zyngier]

As we're about to trigger a PSCI version explosion, it doesn't
hurt to introduce a PSCI_VERSION helper that is going to be
used everywhere.

Signed-off-by: Marc Zyngier 
---
 include/kvm/arm_psci.h| 6 --
 include/uapi/linux/psci.h | 3 +++
 virt/kvm/arm/psci.c   | 4 +---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index 2042bb909474..5659343580a3 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -18,8 +18,10 @@
 #ifndef __KVM_ARM_PSCI_H__
 #define __KVM_ARM_PSCI_H__
 
-#define KVM_ARM_PSCI_0_1   1
-#define KVM_ARM_PSCI_0_2   2
+#include 
+
+#define KVM_ARM_PSCI_0_1   PSCI_VERSION(0, 1)
+#define KVM_ARM_PSCI_0_2   PSCI_VERSION(0, 2)
 
 int kvm_psci_version(struct kvm_vcpu *vcpu);
 int kvm_psci_call(struct kvm_vcpu *vcpu);
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 760e52a9640f..b3bcabe380da 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -88,6 +88,9 @@
(((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT)
 #define PSCI_VERSION_MINOR(ver)\
((ver) & PSCI_VERSION_MINOR_MASK)
+#define PSCI_VERSION(maj, min) \
+   maj) << PSCI_VERSION_MAJOR_SHIFT) & PSCI_VERSION_MAJOR_MASK) | \
+((min) & PSCI_VERSION_MINOR_MASK))
 
 /* PSCI features decoding (>=1.0) */
 #define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT 1
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index b322e46fd142..999f94d6bb98 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -25,8 +25,6 @@
 
 #include 
 
-#include 
-
 /*
  * This is an implementation of the Power State Coordination Interface
  * as described in ARM document number ARM DEN 0022A.
@@ -222,7 +220,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 * Bits[31:16] = Major Version = 0
 * Bits[15:0] = Minor Version = 2
 */
-   val = 2;
+   val = KVM_ARM_PSCI_0_2;
break;
case PSCI_0_2_FN_CPU_SUSPEND:
case PSCI_0_2_FN64_CPU_SUSPEND:
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 08/18] arm/arm64: KVM: Add PSCI version selection API

[Marc Zyngier]

Although we've implemented PSCI 1.0 and 1.1, nothing can select them
Since all the new PSCI versions are backward compatible, we decide to
default to the latest version of the PSCI implementation. This is no
different from doing a firmware upgrade on KVM.

But in order to give a chance to hypothetical badly implemented guests
that would have a fit by discovering something other than PSCI 0.2,
let's provide a new API that allows userspace to pick one particular
version of the API.

This is implemented as a new class of "firmware" registers, where
we expose the PSCI version. This allows the PSCI version to be
save/restored as part of a guest migration, and also set to
any supported version if the guest requires it.

Signed-off-by: Marc Zyngier 
---
 Documentation/virtual/kvm/api.txt  |  3 +-
 Documentation/virtual/kvm/arm/psci.txt | 30 +++
 arch/arm/include/asm/kvm_host.h|  3 ++
 arch/arm/include/uapi/asm/kvm.h|  6 +++
 arch/arm/kvm/guest.c   | 13 +++
 arch/arm64/include/asm/kvm_host.h  |  3 ++
 arch/arm64/include/uapi/asm/kvm.h  |  6 +++
 arch/arm64/kvm/guest.c | 14 ++-
 include/kvm/arm_psci.h |  9 +
 virt/kvm/arm/psci.c| 68 +-
 10 files changed, 151 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/virtual/kvm/arm/psci.txt

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 57d3ee9e4bde..334905202141 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2493,7 +2493,8 @@ Possible features:
  and execute guest code when KVM_RUN is called.
- KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
  Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
-   - KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 for the CPU.
+   - KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 (or a future revision
+  backward compatible with v0.2) for the CPU.
  Depends on KVM_CAP_ARM_PSCI_0_2.
- KVM_ARM_VCPU_PMU_V3: Emulate PMUv3 for the CPU.
  Depends on KVM_CAP_ARM_PMU_V3.
diff --git a/Documentation/virtual/kvm/arm/psci.txt 
b/Documentation/virtual/kvm/arm/psci.txt
new file mode 100644
index ..aafdab887b04
--- /dev/null
+++ b/Documentation/virtual/kvm/arm/psci.txt
@@ -0,0 +1,30 @@
+KVM implements the PSCI (Power State Coordination Interface)
+specification in order to provide services such as CPU on/off, reset
+and power-off to the guest.
+
+The PSCI specification is regularly updated to provide new features,
+and KVM implements these updates if they make sense from a virtualization
+point of view.
+
+This means that a guest booted on two different versions of KVM can
+observe two different "firmware" revisions. This could cause issues if
+a given guest is tied to a particular PSCI revision (unlikely), or if
+a migration causes a different PSCI version to be exposed out of the
+blue to an unsuspecting guest.
+
+In order to remedy this situation, KVM exposes a set of "firmware
+pseudo-registers" that can be manipulated using the GET/SET_ONE_REG
+interface. These registers can be saved/restored by userspace, and set
+to a convenient value if required.
+
+The following register is defined:
+
+* KVM_REG_ARM_PSCI_VERSION:
+
+  - Only valid if the vcpu has the KVM_ARM_VCPU_PSCI_0_2 feature set
+(and thus has already been initialized)
+  - Returns the current PSCI version on GET_ONE_REG (defaulting to the
+highest PSCI version implemented by KVM and compatible with v0.2)
+  - Allows any PSCI version implemented by KVM and compatible with
+v0.2 to be set with SET_ONE_REG
+  - Affects the whole VM (even if the register view is per-vcpu)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index acbf9ec7b396..e9d57060d88c 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -75,6 +75,9 @@ struct kvm_arch {
/* Interrupt controller */
struct vgic_distvgic;
int max_vcpus;
+
+   /* Mandated version of PSCI */
+   u32 psci_version;
 };
 
 #define KVM_NR_MEM_OBJS 40
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 6edd177bb1c7..47dfc99f5cd0 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -186,6 +186,12 @@ struct kvm_arch_memory_slot {
 #define KVM_REG_ARM_VFP_FPINST 0x1009
 #define KVM_REG_ARM_VFP_FPINST20x100A
 
+/* KVM-as-firmware specific pseudo-registers */
+#define KVM_REG_ARM_FW (0x0014 << KVM_REG_ARM_COPROC_SHIFT)
+#define KVM_REG_ARM_FW_REG(r)  (KVM_REG_ARM | KVM_REG_SIZE_U64 | \
+KVM_REG_ARM_FW | ((r) & 0x))
+#define KVM_REG_ARM_PSCI_VERSION   KVM_REG_ARM_FW_REG(0)
+
 /* Device Control API: ARM VGIC */
 #define KVM_DEV_ARM_VGIC_GRP_ADDR  0
 #define KVM_DEV_ARM

[PATCH v3 07/18] arm/arm64: KVM: Implement PSCI 1.0 support

[Marc Zyngier]

PSCI 1.0 can be trivially implemented by having PSCI 0.2 and
the FEATURES call. Of, and returning 1.0 as the PSCI version.

We happily ignore everything else, as it is optional.

Signed-off-by: Marc Zyngier 
---
 include/kvm/arm_psci.h |  1 +
 virt/kvm/arm/psci.c| 43 +++
 2 files changed, 44 insertions(+)

diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index 5659343580a3..5446435457c2 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -22,6 +22,7 @@
 
 #define KVM_ARM_PSCI_0_1   PSCI_VERSION(0, 1)
 #define KVM_ARM_PSCI_0_2   PSCI_VERSION(0, 2)
+#define KVM_ARM_PSCI_1_0   PSCI_VERSION(1, 0)
 
 int kvm_psci_version(struct kvm_vcpu *vcpu);
 int kvm_psci_call(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index c41553d35110..291874cff85e 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -313,6 +313,47 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
return ret;
 }
 
+static int kvm_psci_1_0_call(struct kvm_vcpu *vcpu)
+{
+   u32 psci_fn = smccc_get_function(vcpu);
+   u32 feature;
+   unsigned long val;
+   int ret = 1;
+
+   switch(psci_fn) {
+   case PSCI_0_2_FN_PSCI_VERSION:
+   val = KVM_ARM_PSCI_1_0;
+   break;
+   case PSCI_1_0_FN_PSCI_FEATURES:
+   feature = smccc_get_arg1(vcpu);
+   switch(feature) {
+   case PSCI_0_2_FN_PSCI_VERSION:
+   case PSCI_0_2_FN_CPU_SUSPEND:
+   case PSCI_0_2_FN64_CPU_SUSPEND:
+   case PSCI_0_2_FN_CPU_OFF:
+   case PSCI_0_2_FN_CPU_ON:
+   case PSCI_0_2_FN64_CPU_ON:
+   case PSCI_0_2_FN_AFFINITY_INFO:
+   case PSCI_0_2_FN64_AFFINITY_INFO:
+   case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+   case PSCI_0_2_FN_SYSTEM_OFF:
+   case PSCI_0_2_FN_SYSTEM_RESET:
+   case PSCI_1_0_FN_PSCI_FEATURES:
+   val = 0;
+   break;
+   default:
+   val = PSCI_RET_NOT_SUPPORTED;
+   break;
+   }
+   break;
+   default:
+   return kvm_psci_0_2_call(vcpu);
+   }
+
+   smccc_set_retval(vcpu, val, 0, 0, 0);
+   return ret;
+}
+
 static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 {
struct kvm *kvm = vcpu->kvm;
@@ -355,6 +396,8 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 int kvm_psci_call(struct kvm_vcpu *vcpu)
 {
switch (kvm_psci_version(vcpu)) {
+   case KVM_ARM_PSCI_1_0:
+   return kvm_psci_1_0_call(vcpu);
case KVM_ARM_PSCI_0_2:
return kvm_psci_0_2_call(vcpu);
case KVM_ARM_PSCI_0_1:
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 06/18] arm/arm64: KVM: Add smccc accessors to PSCI code

[Marc Zyngier]

Instead of open coding the accesses to the various registers,
let's add explicit SMCCC accessors.

Signed-off-by: Marc Zyngier 
---
 virt/kvm/arm/psci.c | 52 ++--
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 999f94d6bb98..c41553d35110 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -32,6 +32,38 @@
 
 #define AFFINITY_MASK(level)   ~((0x1UL << ((level) * MPIDR_LEVEL_BITS)) - 1)
 
+static u32 smccc_get_function(struct kvm_vcpu *vcpu)
+{
+   return vcpu_get_reg(vcpu, 0);
+}
+
+static unsigned long smccc_get_arg1(struct kvm_vcpu *vcpu)
+{
+   return vcpu_get_reg(vcpu, 1);
+}
+
+static unsigned long smccc_get_arg2(struct kvm_vcpu *vcpu)
+{
+   return vcpu_get_reg(vcpu, 2);
+}
+
+static unsigned long smccc_get_arg3(struct kvm_vcpu *vcpu)
+{
+   return vcpu_get_reg(vcpu, 3);
+}
+
+static void smccc_set_retval(struct kvm_vcpu *vcpu,
+unsigned long a0,
+unsigned long a1,
+unsigned long a2,
+unsigned long a3)
+{
+   vcpu_set_reg(vcpu, 0, a0);
+   vcpu_set_reg(vcpu, 1, a1);
+   vcpu_set_reg(vcpu, 2, a2);
+   vcpu_set_reg(vcpu, 3, a3);
+}
+
 static unsigned long psci_affinity_mask(unsigned long affinity_level)
 {
if (affinity_level <= 3)
@@ -77,7 +109,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu 
*source_vcpu)
unsigned long context_id;
phys_addr_t target_pc;
 
-   cpu_id = vcpu_get_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
+   cpu_id = smccc_get_arg1(source_vcpu) & MPIDR_HWID_BITMASK;
if (vcpu_mode_is_32bit(source_vcpu))
cpu_id &= ~((u32) 0);
 
@@ -96,8 +128,8 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu 
*source_vcpu)
return PSCI_RET_INVALID_PARAMS;
}
 
-   target_pc = vcpu_get_reg(source_vcpu, 2);
-   context_id = vcpu_get_reg(source_vcpu, 3);
+   target_pc = smccc_get_arg2(source_vcpu);
+   context_id = smccc_get_arg3(source_vcpu);
 
kvm_reset_vcpu(vcpu);
 
@@ -116,7 +148,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu 
*source_vcpu)
 * NOTE: We always update r0 (or x0) because for PSCI v0.1
 * the general puspose registers are undefined upon CPU_ON.
 */
-   vcpu_set_reg(vcpu, 0, context_id);
+   smccc_set_retval(vcpu, context_id, 0, 0, 0);
vcpu->arch.power_off = false;
smp_mb();   /* Make sure the above is visible */
 
@@ -136,8 +168,8 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct 
kvm_vcpu *vcpu)
struct kvm *kvm = vcpu->kvm;
struct kvm_vcpu *tmp;
 
-   target_affinity = vcpu_get_reg(vcpu, 1);
-   lowest_affinity_level = vcpu_get_reg(vcpu, 2);
+   target_affinity = smccc_get_arg1(vcpu);
+   lowest_affinity_level = smccc_get_arg2(vcpu);
 
/* Determine target affinity mask */
target_affinity_mask = psci_affinity_mask(lowest_affinity_level);
@@ -210,7 +242,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
 static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 {
struct kvm *kvm = vcpu->kvm;
-   unsigned long psci_fn = vcpu_get_reg(vcpu, 0) & ~((u32) 0);
+   u32 psci_fn = smccc_get_function(vcpu);
unsigned long val;
int ret = 1;
 
@@ -277,14 +309,14 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
break;
}
 
-   vcpu_set_reg(vcpu, 0, val);
+   smccc_set_retval(vcpu, val, 0, 0, 0);
return ret;
 }
 
 static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 {
struct kvm *kvm = vcpu->kvm;
-   unsigned long psci_fn = vcpu_get_reg(vcpu, 0) & ~((u32) 0);
+   u32 psci_fn = smccc_get_function(vcpu);
unsigned long val;
 
switch (psci_fn) {
@@ -302,7 +334,7 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
break;
}
 
-   vcpu_set_reg(vcpu, 0, val);
+   smccc_set_retval(vcpu, val, 0, 0, 0);
return 1;
 }
 
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 03/18] arm64: KVM: Increment PC after handling an SMC trap

[Marc Zyngier]

When handling an SMC trap, the "preferred return address" is set
to that of the SMC, and not the next PC (which is a departure from
the behaviour of an SMC that isn't trapped).

Increment PC in the handler, as the guest is otherwise forever
stuck...

Cc: sta...@vger.kernel.org
Fixes: acfb3b883f6d ("arm64: KVM: Fix SMCCC handling of unimplemented SMC/HVC 
calls")
Signed-off-by: Marc Zyngier 
---
 arch/arm64/kvm/handle_exit.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 520b0dad3c62..5493bbefbd0d 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -62,7 +62,16 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run 
*run)
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
+   /*
+* "If an SMC instruction executed at Non-secure EL1 is
+* trapped to EL2 because HCR_EL2.TSC is 1, the exception is a
+* Trap exception, not a Secure Monitor Call exception [...]"
+*
+* We need to advance the PC after the trap, as it would
+* otherwise return to the same address...
+*/
vcpu_set_reg(vcpu, 0, ~0UL);
+   kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
return 1;
 }
 
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 04/18] arm/arm64: KVM: Consolidate the PSCI include files

[Marc Zyngier]

As we're about to update the PSCI support, and because I'm lazy,
let's move the PSCI include file to include/kvm so that both
ARM architectures can find it.

Signed-off-by: Marc Zyngier 
---
 arch/arm/include/asm/kvm_psci.h| 27 --
 arch/arm/kvm/handle_exit.c |  2 +-
 arch/arm64/kvm/handle_exit.c   |  3 ++-
 .../asm/kvm_psci.h => include/kvm/arm_psci.h   |  6 ++---
 virt/kvm/arm/arm.c |  2 +-
 virt/kvm/arm/psci.c|  3 ++-
 6 files changed, 9 insertions(+), 34 deletions(-)
 delete mode 100644 arch/arm/include/asm/kvm_psci.h
 rename arch/arm64/include/asm/kvm_psci.h => include/kvm/arm_psci.h (89%)

diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
deleted file mode 100644
index 6bda945d31fa..
--- a/arch/arm/include/asm/kvm_psci.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * Copyright (C) 2012 - ARM Ltd
- * Author: Marc Zyngier 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see .
- */
-
-#ifndef __ARM_KVM_PSCI_H__
-#define __ARM_KVM_PSCI_H__
-
-#define KVM_ARM_PSCI_0_1   1
-#define KVM_ARM_PSCI_0_2   2
-
-int kvm_psci_version(struct kvm_vcpu *vcpu);
-int kvm_psci_call(struct kvm_vcpu *vcpu);
-
-#endif /* __ARM_KVM_PSCI_H__ */
diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index a4bf0f6f024a..230ae4079108 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -21,7 +21,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 #include "trace.h"
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 5493bbefbd0d..588f910632a7 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -22,13 +22,14 @@
 #include 
 #include 
 
+#include 
+
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/arm64/include/asm/kvm_psci.h b/include/kvm/arm_psci.h
similarity index 89%
rename from arch/arm64/include/asm/kvm_psci.h
rename to include/kvm/arm_psci.h
index bc39e557c56c..2042bb909474 100644
--- a/arch/arm64/include/asm/kvm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -15,8 +15,8 @@
  * along with this program.  If not, see .
  */
 
-#ifndef __ARM64_KVM_PSCI_H__
-#define __ARM64_KVM_PSCI_H__
+#ifndef __KVM_ARM_PSCI_H__
+#define __KVM_ARM_PSCI_H__
 
 #define KVM_ARM_PSCI_0_1   1
 #define KVM_ARM_PSCI_0_2   2
@@ -24,4 +24,4 @@
 int kvm_psci_version(struct kvm_vcpu *vcpu);
 int kvm_psci_call(struct kvm_vcpu *vcpu);
 
-#endif /* __ARM64_KVM_PSCI_H__ */
+#endif /* __KVM_ARM_PSCI_H__ */
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 15bf026eb182..af3e98fc377e 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
@@ -46,7 +47,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #ifdef REQUIRES_VIRT
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index f1e363bab5e8..b322e46fd142 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -21,9 +21,10 @@
 
 #include 
 #include 
-#include 
 #include 
 
+#include 
+
 #include 
 
 /*
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 02/18] arm: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls

[Marc Zyngier]

KVM doesn't follow the SMCCC when it comes to unimplemented calls,
and inject an UNDEF instead of returning an error. Since firmware
calls are now used for security mitigation, they are becoming more
common, and the undef is counter productive.

Instead, let's follow the SMCCC which states that -1 must be returned
to the caller when getting an unknown function number.

Cc: 
Signed-off-by: Marc Zyngier 
---
 arch/arm/kvm/handle_exit.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index cf8bf6bf87c4..a4bf0f6f024a 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -38,7 +38,7 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run 
*run)
 
ret = kvm_psci_call(vcpu);
if (ret < 0) {
-   kvm_inject_undefined(vcpu);
+   vcpu_set_reg(vcpu, 0, ~0UL);
return 1;
}
 
@@ -47,7 +47,16 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run 
*run)
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-   kvm_inject_undefined(vcpu);
+   /*
+* "If an SMC instruction executed at Non-secure EL1 is
+* trapped to EL2 because HCR_EL2.TSC is 1, the exception is a
+* Trap exception, not a Secure Monitor Call exception [...]"
+*
+* We need to advance the PC after the trap, as it would
+* otherwise return to the same address...
+*/
+   vcpu_set_reg(vcpu, 0, ~0UL);
+   kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
return 1;
 }
 
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 01/18] arm64: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls

[Marc Zyngier]

KVM doesn't follow the SMCCC when it comes to unimplemented calls,
and inject an UNDEF instead of returning an error. Since firmware
calls are now used for security mitigation, they are becoming more
common, and the undef is counter productive.

Instead, let's follow the SMCCC which states that -1 must be returned
to the caller when getting an unknown function number.

Cc: 
Signed-off-by: Marc Zyngier 
Signed-off-by: Christoffer Dall 
---
 arch/arm64/kvm/handle_exit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index c09fc5a576c7..520b0dad3c62 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -53,7 +53,7 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run 
*run)
 
ret = kvm_psci_call(vcpu);
if (ret < 0) {
-   kvm_inject_undefined(vcpu);
+   vcpu_set_reg(vcpu, 0, ~0UL);
return 1;
}
 
@@ -62,7 +62,7 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run 
*run)
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-   kvm_inject_undefined(vcpu);
+   vcpu_set_reg(vcpu, 0, ~0UL);
return 1;
 }
 
-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 00/18] arm64: Add SMCCC v1.1 support and CVE-2017-5715 (Spectre variant 2) mitigation

[Marc Zyngier]

ARM has recently published a SMC Calling Convention (SMCCC)
specification update[1] that provides an optimised calling convention
and optional, discoverable support for mitigating CVE-2017-5715. ARM
Trusted Firmware (ATF) has already gained such an implementation[2].

This series addresses a few things:

- It provides a KVM implementation of PSCI v1.0, which is a
  prerequisite for being able to discover SMCCC v1.1, together with a
  new userspace API to control the PSCI revision number that the guest
  sees.

- It allows KVM to advertise SMCCC v1.1, which is de-facto supported
  already (it never corrupts any of the guest registers).

- It implements KVM support for the ARCH_WORKAROUND_1 function that is
  used to mitigate CVE-2017-5715 in a guest (if such mitigation is
  available on the host).

- It implements SMCCC v1.1 and ARCH_WORKAROUND_1 discovery support in
  the kernel itself.

- It finally provides firmware callbacks for CVE-2017-5715 for both
  kernel and KVM and drop the initial PSCI_GET_VERSION based
  mitigation.

Patch 1 is already merged, and included here for reference. Patches on
top of arm64/for-next/core. Tested on Seattle and Juno, the latter
with ATF implementing SMCCC v1.1.

[1]: https://developer.arm.com/support/security-update/downloads/

[2]: https://github.com/ARM-software/arm-trusted-firmware/pull/1240

* From v2:
  - Fixed SMC handling in KVM
  - PSCI fixes and tidying up
  - SMCCC primitive rework for better code generation (both efficiency
  and correctness)
  - Remove PSCI_GET_VERSION as a mitigation vector

* From v1:
  - Fixed 32bit build
  - Fix function number sign extension (Ard)
  - Inline SMCCC v1.1 primitives (cpp soup)
  - Prevent SMCCC spamming on feature probing
  - Random fixes and tidying up

Marc Zyngier (18):
  arm64: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls
  arm: KVM: Fix SMCCC handling of unimplemented SMC/HVC calls
  arm64: KVM: Increment PC after handling an SMC trap
  arm/arm64: KVM: Consolidate the PSCI include files
  arm/arm64: KVM: Add PSCI_VERSION helper
  arm/arm64: KVM: Add smccc accessors to PSCI code
  arm/arm64: KVM: Implement PSCI 1.0 support
  arm/arm64: KVM: Add PSCI version selection API
  arm/arm64: KVM: Advertise SMCCC v1.1
  arm/arm64: KVM: Turn kvm_psci_version into a static inline
  arm64: KVM: Report SMCCC_ARCH_WORKAROUND_1 BP hardening support
  arm64: KVM: Add SMCCC_ARCH_WORKAROUND_1 fast handling
  firmware/psci: Expose PSCI conduit
  firmware/psci: Expose SMCCC version through psci_ops
  arm/arm64: smccc: Make function identifiers an unsigned quantity
  arm/arm64: smccc: Implement SMCCC v1.1 inline primitive
  arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support
  arm64: Kill PSCI_GET_VERSION as a variant-2 workaround

 Documentation/virtual/kvm/api.txt  |   3 +-
 Documentation/virtual/kvm/arm/psci.txt |  30 +
 arch/arm/include/asm/kvm_host.h|  10 ++
 arch/arm/include/asm/kvm_psci.h|  27 -
 arch/arm/include/uapi/asm/kvm.h|   6 +
 arch/arm/kvm/guest.c   |  13 +++
 arch/arm/kvm/handle_exit.c |  17 ++-
 arch/arm64/include/asm/kvm_host.h  |   9 ++
 arch/arm64/include/asm/kvm_psci.h  |  27 -
 arch/arm64/include/uapi/asm/kvm.h  |   6 +
 arch/arm64/kernel/bpi.S|  44 
 arch/arm64/kernel/cpu_errata.c |  77 ++---
 arch/arm64/kvm/guest.c |  14 ++-
 arch/arm64/kvm/handle_exit.c   |  18 ++-
 arch/arm64/kvm/hyp/hyp-entry.S |  20 +++-
 arch/arm64/kvm/hyp/switch.c|  14 +--
 drivers/firmware/psci.c|  47 +++-
 include/kvm/arm_psci.h |  63 +++
 include/linux/arm-smccc.h  | 167 +++-
 include/linux/psci.h   |  13 +++
 include/uapi/linux/psci.h  |   3 +
 virt/kvm/arm/arm.c |   2 +-
 virt/kvm/arm/psci.c| 196 +
 23 files changed, 677 insertions(+), 149 deletions(-)
 create mode 100644 Documentation/virtual/kvm/arm/psci.txt
 delete mode 100644 arch/arm/include/asm/kvm_psci.h
 delete mode 100644 arch/arm64/include/asm/kvm_psci.h
 create mode 100644 include/kvm/arm_psci.h

-- 
2.14.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] irqchip/gic-v3: Use wmb() instead of smb_wmb() in gic_raise_softirq()

[Will Deacon]

Hi Shanker,

On Wed, Jan 31, 2018 at 06:03:42PM -0600, Shanker Donthineni wrote:
> A DMB instruction can be used to ensure the relative order of only
> memory accesses before and after the barrier. Since writes to system
> registers are not memory operations, barrier DMB is not sufficient
> for observability of memory accesses that occur before ICC_SGI1R_EL1
> writes.
> 
> A DSB instruction ensures that no instructions that appear in program
> order after the DSB instruction, can execute until the DSB instruction
> has completed.
> 
> Signed-off-by: Shanker Donthineni 
> ---
>  drivers/irqchip/irq-gic-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index b56c3e2..980ae8e 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -688,7 +688,7 @@ static void gic_raise_softirq(const struct cpumask *mask, 
> unsigned int irq)
>* Ensure that stores to Normal memory are visible to the
>* other CPUs before issuing the IPI.
>*/
> - smp_wmb();
> + wmb();

I think this is the right thing to do and the smp_wmb() was accidentally
pulled in here as a copy-paste from the GICv2 driver where it is sufficient
in practice.

Did you spot this by code inspection, or did the DMB actually cause
observable failures? (trying to figure out whether or not this need to go
to -stable).

Anyway:

Acked-by: Will Deacon 

Cheers,

Will
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 16/16] arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Marc Zyngier]

On 01/02/18 02:40, Hanjun Guo wrote:
> On 2018/1/31 23:05, Marc Zyngier wrote:
>> On 31/01/18 14:38, Ard Biesheuvel wrote:
>>> On 31 January 2018 at 14:35, Ard Biesheuvel  
>>> wrote:
 On 31 January 2018 at 14:11, Marc Zyngier  wrote:
> On 31/01/18 13:56, Hanjun Guo wrote:
>> Hi Marc,
>>
>> On 2018/1/30 1:45, Marc Zyngier wrote:
>>>  static int enable_psci_bp_hardening(void *data)
>>>  {
>>>  const struct arm64_cpu_capabilities *entry = data;
>>>
>>> -if (psci_ops.get_version)
>>> +if (psci_ops.get_version) {
>>> +if (check_smccc_arch_workaround_1(entry))
>>> +return 0;
>>
>> If I'm using the new version SMCCC, the firmware have the 
>> choicARM_SMCCC_ARCH_WORKAROUND_1e to decide
>> whether this machine needs the workaround, even if the CPU is vulnerable
>> for CVE-2017-5715, but..
>>
>>> +
>>>  install_bp_hardening_cb(entry,
>>> 
>>> (bp_hardening_cb_t)psci_ops.get_version,
>>> __psci_hyp_bp_inval_start,
>>> __psci_hyp_bp_inval_end);
>>
>> ..the code above seems will enable get_psci_version() for CPU and will
>> trap to trust firmware even the new version of firmware didn't say
>> we need the workaround, did I understand it correctly?
>
> Well, you only get there if we've established that your CPU is affected
> (it has an entry matching its MIDR with the HARDEN_BRANCH_PREDICTOR
> capability), and that entry points to enable_psci_bp_hardening. It is
> 
> I understand, but A53, A57, A72 and etc are always in the list :)
> 
> not the firmware that decides whether we need hardening, but the kernel.
> The firmware merely provides a facility to apply the hardening.
>
>> I'm ask this because some platform will not expose to users to
>> take advantage of CVE-2017-5715, and we can use different firmware
>> to report we need such workaround or not, then use a single kernel
>> image for both vulnerable platforms and no vulnerable ones.
>
> You cannot have your cake and eat it. If you don't want to workaround
> the issue, you can disable the hardening. But asking for the same kernel
> to do both depending on what the firmware reports doesn't make much
> sense to me.

 The SMCCC v1.1. document does appear to imply that systems that
 implement SMCCC v1.1 but don't implement ARM_SMCCC_ARCH_WORKAROUND_1
 should be assumed to be unaffected.

 """
 If the discovery call returns NOT_SUPPORTED:
 • SMCCC_ARCH_WORKAROUND_1 must not be invoked on any PE in the system, and
 • none of the PEs in the system require firmware mitigation for 
 CVE-2017-5715.
 """

 How to deal with conflicting information in this regard (quirk table
 vs firmware implementation) is a matter of policy, of course.
>>
>> Yup. And the current approach fits the spec, I believe. The
> 
> Yes, approach in this patch set fits the spec, it just conflicts with
> MIDR based approach.

No it doesn't. We know that A72 is affected. However you look at it,
there is no way around that fact. So if you have an affected CPU *and*
you implement SMCCC 1.1 with WORKAROUND_1, you get the mitigation.

If your firmware doesn't implement it, blame your provider for supplying
you with an outdated firmware.

>> PSCI_GET_VERSION band-aid should normally be removed shortly after these
>> patches hit mainline.
> 
> I'm a big fan of this :)
> 
>>
>>>
>>> ... and actually, perhaps it makes sense for the
>>> SMCCC_ARCH_WORKAROUND_1 check to be completely independent of MIDR
>>> based errata matching?
>>>
>>> I.e., if SMCCC v1.1 and SMCCC_ARCH_WORKAROUND_1 are both implemented,
>>> we should probably invoke it even if the MIDR is not known to belong
>>> to an affected implementation.
>>
>> This would have an impact on big-little systems, for which there is
>> often a bunch of unaffected CPUs.
> 
> I think it's what we are doing now, SMCCC v1.1 didn't provide the ability
> to report per-cpu SMCCC_ARCH_WORKAROUND_1, and it said:
>  - The discovery call must return the same result on all PEs in the system.
>  - In heterogeneous systems with some PEs that require mitigation and others
>that do not, the firmware must provide a safe implementation of this
>function on all PEs.
> 
> So from the spec that it's the firmware to take care of unaffected CPUs,
> to the kernel it's the same.

The spec makes it safe. The MIDR list makes it fast.

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm