Re: [PATCH] KVM: PPC: Book3S: Provide information about hardware/firmware CVE workarounds

[Paul Mackerras <paulus@ozlabs.org>]

On Tue, Jan 16, 2018 at 03:45:11PM +0100, Paolo Bonzini wrote:
> On 16/01/2018 01:59, Paul Mackerras wrote:
> > This adds a new ioctl, KVM_PPC_GET_CPU_CHAR, that gives userspace
> > information about the underlying machine's level of vulnerability
> > to the recently announced vulnerabilities CVE-2017-5715,
> > CVE-2017-5753 and CVE-2017-5754, and whether the machine provides
> > instructions to assist software to work around the vulnerabilities.
> > 
> > The ioctl returns two u64 words describing characteristics of the
> > CPU and required software behaviour respectively, plus two mask
> > words which indicate which bits have been filled in by the kernel,
> > for extensibility.  The bit definitions are the same as for the
> > new H_GET_CPU_CHARACTERISTICS hypercall.
> > 
> > There is also a new capability, KVM_CAP_PPC_GET_CPU_CHAR, which
> > indicates whether the new ioctl is available.
> > 
> > Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> > ---
> >  Documentation/virtual/kvm/api.txt   |  46 +++++++++++++
> >  arch/powerpc/include/uapi/asm/kvm.h |  22 +++++++
> >  arch/powerpc/kvm/powerpc.c          | 124 ++++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/kvm.h            |   3 +
> >  4 files changed, 195 insertions(+)
> > 
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index f670e4b..85ca84a 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -3394,6 +3394,52 @@ invalid, if invalid pages are written to (e.g. after the end of memory)
> >  or if no page table is present for the addresses (e.g. when using
> >  hugepages).
> >  
> > +4.108 KVM_PPC_GET_CPU_CHAR
> > +
> > +Capability: KVM_CAP_PPC_GET_CPU_CHAR
> > +Architectures: powerpc
> > +Type: vm ioctl
> > +Parameters: struct kvm_ppc_cpu_char (out)
> > +Returns: 0 on successful completion
> > +	 -EFAULT if struct kvm_ppc_cpu_char cannot be written
> > +
> > +This ioctl gives userspace information about certain characteristics
> > +of the CPU relating to speculative execution of instructions and
> > +possible information leakage resulting from speculative execution (see
> > +CVE-2017-5715, CVE-2017-5753 and CVE-2017-5754).  The information is
> > +returned in struct kvm_ppc_cpu_char, which looks like this:
> > +
> > +struct kvm_ppc_cpu_char {
> > +	__u64	character;		/* characteristics of the CPU */
> > +	__u64	behaviour;		/* recommended software behaviour */
> > +	__u64	character_mask;		/* valid bits in character */
> > +	__u64	behaviour_mask;		/* valid bits in behaviour */
> > +};
> > +
> > +For extensibility, the character_mask and behaviour_mask fields
> > +indicate which bits of character and behaviour have been filled in by
> > +the kernel.  If the set of defined bits is extended in future then
> > +userspace will be able to tell whether it is running on a kernel that
> > +knows about the new bits.
> > +
> > +The character field describes attributes of the CPU which can help
> > +with preventing inadvertent information disclosure - specifically,
> > +whether there is an instruction to flash-invalidate the L1 data cache
> > +(ori 30,30,0 or mtspr SPRN_TRIG2,rN), whether the L1 data cache is set
> > +to a mode where entries can only be used by the thread that created
> > +them, whether the bcctr[l] instruction prevents speculation, and
> > +whether a speculation barrier instruction (ori 31,31,0) is provided.
> > +
> > +The behaviour field describes actions that software should take to
> > +prevent inadvertent information disclosure, and thus describes which
> > +vulnerabilities the hardware is subject to; specifically whether the
> > +L1 data cache should be flushed when returning to user mode from the
> > +kernel, and whether a speculation barrier should be placed between an
> > +array bounds check and the array access.
> > +
> > +These fields use the same bit definitions as the new
> > +H_GET_CPU_CHARACTERISTICS hypercall.
> > +
> >  5. The kvm_run structure
> >  ------------------------
> >  
> > diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> > index 61d6049..ce74bed 100644
> > --- a/arch/powerpc/include/uapi/asm/kvm.h
> > +++ b/arch/powerpc/include/uapi/asm/kvm.h
> > @@ -443,6 +443,28 @@ struct kvm_ppc_rmmu_info {
> >  	__u32	ap_encodings[8];
> >  };
> >  
> > +/* For KVM_PPC_GET_CPU_CHAR */
> > +struct kvm_ppc_cpu_char {
> > +	__u64	character;		/* characteristics of the CPU */
> > +	__u64	behaviour;		/* recommended software behaviour */
> > +	__u64	character_mask;		/* valid bits in character */
> > +	__u64	behaviour_mask;		/* valid bits in behaviour */
> > +};
> > +
> > +/*
> > + * Values for character and character_mask.
> > + * These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
> > + */
> > +#define KVM_PPC_CPU_CHAR_SPEC_BAR_ORI31		(1ULL << 63)
> > +#define KVM_PPC_CPU_CHAR_BCCTRL_SERIALISED	(1ULL << 62)
> > +#define KVM_PPC_CPU_CHAR_L1D_FLUSH_ORI30	(1ULL << 61)
> > +#define KVM_PPC_CPU_CHAR_L1D_FLUSH_TRIG2	(1ULL << 60)
> > +#define KVM_PPC_CPU_CHAR_L1D_THREAD_PRIV	(1ULL << 59)
> > +
> > +#define KVM_PPC_CPU_BEHAV_FAVOUR_SECURITY	(1ULL << 63)
> > +#define KVM_PPC_CPU_BEHAV_L1D_FLUSH_PR		(1ULL << 62)
> > +#define KVM_PPC_CPU_BEHAV_BNDS_CHK_SPEC_BAR	(1ULL << 61)
> > +
> >  /* Per-vcpu XICS interrupt controller state */
> >  #define KVM_REG_PPC_ICP_STATE	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8c)
> >  
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 1915e86..c9cecff 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -39,6 +39,10 @@
> >  #include <asm/iommu.h>
> >  #include <asm/switch_to.h>
> >  #include <asm/xive.h>
> > +#ifdef CONFIG_PPC_PSERIES
> > +#include <asm/hvcall.h>
> > +#include <asm/plpar_wrappers.h>
> > +#endif
> >  
> >  #include "timing.h"
> >  #include "irq.h"
> > @@ -548,6 +552,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >  #ifdef CONFIG_KVM_XICS
> >  	case KVM_CAP_IRQ_XICS:
> >  #endif
> > +	case KVM_CAP_PPC_GET_CPU_CHAR:
> >  		r = 1;
> >  		break;
> >  
> > @@ -1759,6 +1764,117 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >  	return r;
> >  }
> >  
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > +/*
> > + * These functions check whether the underlying hardware is safe
> > + * against attacks based on observing the effects of speculatively
> > + * executed instructions, and whether it supplies instructions for
> > + * use in workarounds.  The information comes from firmware, either
> > + * via the device tree on powernv platforms or from an hcall on
> > + * pseries platforms.
> > + */
> > +#ifdef CONFIG_PPC_PSERIES
> > +static int pseries_get_cpu_char(struct kvm_ppc_cpu_char *cp)
> > +{
> > +	struct h_cpu_char_result c;
> > +	unsigned long rc;
> > +
> > +	if (!machine_is(pseries))
> > +		return -ENOTTY;
> > +
> > +	rc = plpar_get_cpu_characteristics(&c);
> > +	if (rc == H_SUCCESS) {
> > +		cp->character = c.character;
> > +		cp->behaviour = c.behaviour;
> > +		cp->character_mask = KVM_PPC_CPU_CHAR_SPEC_BAR_ORI31 |
> > +			KVM_PPC_CPU_CHAR_BCCTRL_SERIALISED |
> > +			KVM_PPC_CPU_CHAR_L1D_FLUSH_ORI30 |
> > +			KVM_PPC_CPU_CHAR_L1D_FLUSH_TRIG2 |
> > +			KVM_PPC_CPU_CHAR_L1D_THREAD_PRIV;
> > +		cp->behaviour_mask = KVM_PPC_CPU_BEHAV_FAVOUR_SECURITY |
> > +			KVM_PPC_CPU_BEHAV_L1D_FLUSH_PR |
> > +			KVM_PPC_CPU_BEHAV_BNDS_CHK_SPEC_BAR;
> > +	}
> > +	return 0;
> > +}
> > +#else
> > +static int pseries_get_cpu_char(struct kvm_ppc_cpu_char *cp)
> > +{
> > +	return -ENOTTY;
> > +}
> > +#endif
> > +
> > +static inline bool have_fw_feat(struct device_node *fw_features,
> > +				const char *state, const char *name)
> > +{
> > +	struct device_node *np;
> > +	bool r = false;
> > +
> > +	np = of_get_child_by_name(fw_features, name);
> > +	if (np) {
> > +		r = of_property_read_bool(np, state);
> > +		of_node_put(np);
> > +	}
> > +	return r;
> > +}
> > +
> > +static int kvmppc_get_cpu_char(struct kvm_ppc_cpu_char *cp)
> > +{
> > +	struct device_node *np, *fw_features;
> > +	int r;
> > +
> > +	memset(cp, 0, sizeof(*cp));
> > +	r = pseries_get_cpu_char(cp);
> > +	if (r != -ENOTTY)
> > +		return r;
> > +
> > +	np = of_find_node_by_name(NULL, "ibm,opal");
> > +	if (np) {
> > +		fw_features = of_get_child_by_name(np, "fw-features");
> > +		of_node_put(np);
> > +		if (!fw_features)
> > +			return 0;
> > +		if (have_fw_feat(fw_features, "enabled",
> > +				 "inst-spec-barrier-ori31,31,0"))
> > +			cp->character |= KVM_PPC_CPU_CHAR_SPEC_BAR_ORI31;
> > +		if (have_fw_feat(fw_features, "enabled",
> > +				 "fw-bcctrl-serialized"))
> > +			cp->character |= KVM_PPC_CPU_CHAR_BCCTRL_SERIALISED;
> > +		if (have_fw_feat(fw_features, "enabled",
> > +				 "inst-l1d-flush-ori30,30,0"))
> > +			cp->character |= KVM_PPC_CPU_CHAR_L1D_FLUSH_ORI30;
> > +		if (have_fw_feat(fw_features, "enabled",
> > +				 "inst-l1d-flush-trig2"))
> > +			cp->character |= KVM_PPC_CPU_CHAR_L1D_FLUSH_TRIG2;
> > +		if (have_fw_feat(fw_features, "enabled",
> > +				 "fw-l1d-thread-split"))
> > +			cp->character |= KVM_PPC_CPU_CHAR_L1D_THREAD_PRIV;
> > +		cp->character_mask = KVM_PPC_CPU_CHAR_SPEC_BAR_ORI31 |
> > +			KVM_PPC_CPU_CHAR_BCCTRL_SERIALISED |
> > +			KVM_PPC_CPU_CHAR_L1D_FLUSH_ORI30 |
> > +			KVM_PPC_CPU_CHAR_L1D_FLUSH_TRIG2 |
> > +			KVM_PPC_CPU_CHAR_L1D_THREAD_PRIV;
> > +
> > +		if (have_fw_feat(fw_features, "enabled",
> > +				 "speculation-policy-favor-security"))
> > +			cp->behaviour |= KVM_PPC_CPU_BEHAV_FAVOUR_SECURITY;
> > +		if (!have_fw_feat(fw_features, "disabled",
> > +				  "needs-l1d-flush-msr-pr-0-to-1"))
> > +			cp->behaviour |= KVM_PPC_CPU_BEHAV_L1D_FLUSH_PR;
> > +		if (!have_fw_feat(fw_features, "disabled",
> > +				  "needs-spec-barrier-for-bound-checks"))
> > +			cp->behaviour |= KVM_PPC_CPU_BEHAV_BNDS_CHK_SPEC_BAR;
> > +		cp->behaviour_mask = KVM_PPC_CPU_BEHAV_FAVOUR_SECURITY |
> > +			KVM_PPC_CPU_BEHAV_L1D_FLUSH_PR |
> > +			KVM_PPC_CPU_BEHAV_BNDS_CHK_SPEC_BAR;
> > +
> > +		of_node_put(fw_features);
> > +	}
> > +
> > +	return 0;
> > +}
> > +#endif
> > +
> >  long kvm_arch_vm_ioctl(struct file *filp,
> >                         unsigned int ioctl, unsigned long arg)
> >  {
> > @@ -1861,6 +1977,14 @@ long kvm_arch_vm_ioctl(struct file *filp,
> >  			r = -EFAULT;
> >  		break;
> >  	}
> > +	case KVM_PPC_GET_CPU_CHAR: {
> > +		struct kvm_ppc_cpu_char cpuchar;
> > +
> > +		r = kvmppc_get_cpu_char(&cpuchar);
> > +		if (r >= 0 && copy_to_user(argp, &cpuchar, sizeof(cpuchar)))
> > +			r = -EFAULT;
> > +		break;
> > +	}
> >  	default: {
> >  		struct kvm *kvm = filp->private_data;
> >  		r = kvm->arch.kvm_ops->arch_vm_ioctl(filp, ioctl, arg);
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 282d7613..e96e629 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -932,6 +932,7 @@ struct kvm_ppc_resize_hpt {
> >  #define KVM_CAP_HYPERV_SYNIC2 148
> >  #define KVM_CAP_HYPERV_VP_INDEX 149
> >  #define KVM_CAP_S390_AIS_MIGRATION 150
> > +#define KVM_CAP_PPC_GET_CPU_CHAR 151
> >  
> >  #ifdef KVM_CAP_IRQ_ROUTING
> >  
> > @@ -1261,6 +1262,8 @@ struct kvm_s390_ucas_mapping {
> >  #define KVM_PPC_CONFIGURE_V3_MMU  _IOW(KVMIO,  0xaf, struct kvm_ppc_mmuv3_cfg)
> >  /* Available with KVM_CAP_PPC_RADIX_MMU */
> >  #define KVM_PPC_GET_RMMU_INFO	  _IOW(KVMIO,  0xb0, struct kvm_ppc_rmmu_info)
> > +/* Available with KVM_CAP_PPC_GET_CPU_CHAR */
> > +#define KVM_PPC_GET_CPU_CHAR	  _IOR(KVMIO,  0xb1, struct kvm_ppc_cpu_char)
> >  
> >  /* ioctl for vm fd */
> >  #define KVM_CREATE_DEVICE	  _IOWR(KVMIO,  0xe0, struct kvm_create_device)
> > 
> 
> Thanks, looks good.  Would you like this in 4.15?

Yes please.  Will you just apply the patch, or do you want me to put
it in a branch for you to pull?

The patch depends on 191eccb15809 ("powerpc/pseries: Add
H_GET_CPU_CHARACTERISTICS flags & wrapper", 2018-01-09) which is in
Linus' tree as of v4.15-rc8, and I see that the kvm master branch is
at rc8, so it should apply on the master branch just fine.

Thanks,
Paul.