| Message ID | 20200319043301.GA13052@blackberry |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: PPC: Book3S HV: Add a capability for enabling secure guests | expand |
On Thu, 19 Mar 2020 15:33:01 +1100 Paul Mackerras <paulus@ozlabs.org> wrote: > At present, on Power systems with Protected Execution Facility > hardware and an ultravisor, a KVM guest can transition to being a > secure guest at will. Userspace (QEMU) has no way of knowing > whether a host system is capable of running secure guests. This > will present a problem in future when the ultravisor is capable of > migrating secure guests from one host to another, because > virtualization management software will have no way to ensure that > secure guests only run in domains where all of the hosts can > support secure guests. > > This adds a VM capability which has two functions: (a) userspace > can query it to find out whether the host can support secure guests, > and (b) userspace can enable it for a guest, which allows that > guest to become a secure guest. If userspace does not enable it, > KVM will return an error when the ultravisor does the hypercall > that indicates that the guest is starting to transition to a > secure guest. The ultravisor will then abort the transition and > the guest will terminate. > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> > --- Reviewed-by: Greg Kurz <groug@kaod.org> Is someone working on wiring this up in QEMU ? > Note, only compile-tested. Ram, please test. > > Documentation/virt/kvm/api.rst | 17 +++++++++++++++++ > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/include/asm/kvm_ppc.h | 1 + > arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++ > arch/powerpc/kvm/book3s_hv_uvmem.c | 4 ++++ > arch/powerpc/kvm/powerpc.c | 13 +++++++++++++ > include/uapi/linux/kvm.h | 1 + > 7 files changed, 50 insertions(+) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 158d118..a925500 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -5779,6 +5779,23 @@ it hard or impossible to use it correctly. The availability of > KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 signals that those bugs are fixed. > Userspace should not try to use KVM_CAP_MANUAL_DIRTY_LOG_PROTECT. > > +7.19 KVM_CAP_PPC_SECURE_GUEST > +------------------------------ > + > +:Architectures: ppc > + > +This capability indicates that KVM is running on a host that has > +ultravisor firmware and thus can support a secure guest. On such a > +system, a guest can ask the ultravisor to make it a secure guest, > +one whose memory is inaccessible to the host except for pages which > +are explicitly requested to be shared with the host. The ultravisor > +notifies KVM when a guest requests to become a secure guest, and KVM > +has the opportunity to veto the transition. > + > +If present, this capability can be enabled for a VM, meaning that KVM > +will allow the transition to secure guest mode. Otherwise KVM will > +veto the transition. > + > 8. Other capabilities. > ====================== > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 6e8b8ff..f99b433 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -303,6 +303,7 @@ struct kvm_arch { > u8 radix; > u8 fwnmi_enabled; > u8 secure_guest; > + u8 svm_enabled; > bool threads_indep; > bool nested_enable; > pgd_t *pgtable; > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 406ec46..0733618 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -316,6 +316,7 @@ struct kvmppc_ops { > int size); > int (*store_to_eaddr)(struct kvm_vcpu *vcpu, ulong *eaddr, void *ptr, > int size); > + int (*enable_svm)(struct kvm *kvm); > int (*svm_off)(struct kvm *kvm); > }; > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index fbc55a1..36da720 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -5423,6 +5423,18 @@ static void unpin_vpa_reset(struct kvm *kvm, struct kvmppc_vpa *vpa) > } > > /* > + * Enable a guest to become a secure VM. > + * Called when the KVM_CAP_PPC_SECURE_GUEST capability is enabled. > + */ > +static int kvmhv_enable_svm(struct kvm *kvm) > +{ > + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) > + return -EINVAL; > + kvm->arch.svm_enabled = 1; > + return 0; > +} > + > +/* > * IOCTL handler to turn off secure mode of guest > * > * - Release all device pages > @@ -5543,6 +5555,7 @@ static struct kvmppc_ops kvm_ops_hv = { > .enable_nested = kvmhv_enable_nested, > .load_from_eaddr = kvmhv_load_from_eaddr, > .store_to_eaddr = kvmhv_store_to_eaddr, > + .enable_svm = kvmhv_enable_svm, > .svm_off = kvmhv_svm_off, > }; > > diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c > index 79b1202..2ad999f 100644 > --- a/arch/powerpc/kvm/book3s_hv_uvmem.c > +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c > @@ -216,6 +216,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) > if (!kvm_is_radix(kvm)) > return H_UNSUPPORTED; > > + /* NAK the transition to secure if not enabled */ > + if (!kvm->arch.svm_enabled) > + return H_AUTHORITY; > + > srcu_idx = srcu_read_lock(&kvm->srcu); > slots = kvm_memslots(kvm); > kvm_for_each_memslot(memslot, slots) { > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 62ee66d..c32e6cc2 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -670,6 +670,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > (hv_enabled && cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)); > break; > #endif > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > + case KVM_CAP_PPC_SECURE_GUEST: > + r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR); > + break; > +#endif > default: > r = 0; > break; > @@ -2170,6 +2175,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, > r = kvm->arch.kvm_ops->enable_nested(kvm); > break; > #endif > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > + case KVM_CAP_PPC_SECURE_GUEST: > + r = -EINVAL; > + if (!is_kvmppc_hv_enabled(kvm) || !kvm->arch.kvm_ops->enable_svm) > + break; > + r = kvm->arch.kvm_ops->enable_svm(kvm); > + break; > +#endif > default: > r = -EINVAL; > break; > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 5e6234c..428c7dd 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1016,6 +1016,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_ARM_INJECT_EXT_DABT 178 > #define KVM_CAP_S390_VCPU_RESETS 179 > #define KVM_CAP_S390_PROTECTED 180 > +#define KVM_CAP_PPC_SECURE_GUEST 181 > > #ifdef KVM_CAP_IRQ_ROUTING >
Greg Kurz <groug@kaod.org> writes: > > Reviewed-by: Greg Kurz <groug@kaod.org> > > Is someone working on wiring this up in QEMU ? > I just did so I could test it. =) If no one else is doing it I could send my patch as an RFC and we iterate from that.
On 3/19/20 5:33 AM, Paul Mackerras wrote: > At present, on Power systems with Protected Execution Facility > hardware and an ultravisor, a KVM guest can transition to being a > secure guest at will. Userspace (QEMU) has no way of knowing > whether a host system is capable of running secure guests. This > will present a problem in future when the ultravisor is capable of > migrating secure guests from one host to another, because > virtualization management software will have no way to ensure that > secure guests only run in domains where all of the hosts can > support secure guests. > > This adds a VM capability which has two functions: (a) userspace > can query it to find out whether the host can support secure guests, > and (b) userspace can enable it for a guest, which allows that > guest to become a secure guest. If userspace does not enable it, > KVM will return an error when the ultravisor does the hypercall > that indicates that the guest is starting to transition to a > secure guest. The ultravisor will then abort the transition and > the guest will terminate. > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- > Note, only compile-tested. Ram, please test. > > Documentation/virt/kvm/api.rst | 17 +++++++++++++++++ > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/include/asm/kvm_ppc.h | 1 + > arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++ > arch/powerpc/kvm/book3s_hv_uvmem.c | 4 ++++ > arch/powerpc/kvm/powerpc.c | 13 +++++++++++++ > include/uapi/linux/kvm.h | 1 + > 7 files changed, 50 insertions(+) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 158d118..a925500 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -5779,6 +5779,23 @@ it hard or impossible to use it correctly. The availability of > KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 signals that those bugs are fixed. > Userspace should not try to use KVM_CAP_MANUAL_DIRTY_LOG_PROTECT. > > +7.19 KVM_CAP_PPC_SECURE_GUEST > +------------------------------ > + > +:Architectures: ppc > + > +This capability indicates that KVM is running on a host that has > +ultravisor firmware and thus can support a secure guest. On such a > +system, a guest can ask the ultravisor to make it a secure guest, > +one whose memory is inaccessible to the host except for pages which > +are explicitly requested to be shared with the host. The ultravisor > +notifies KVM when a guest requests to become a secure guest, and KVM > +has the opportunity to veto the transition. > + > +If present, this capability can be enabled for a VM, meaning that KVM > +will allow the transition to secure guest mode. Otherwise KVM will > +veto the transition. > + > 8. Other capabilities. > ====================== > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 6e8b8ff..f99b433 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -303,6 +303,7 @@ struct kvm_arch { > u8 radix; > u8 fwnmi_enabled; > u8 secure_guest; > + u8 svm_enabled; > bool threads_indep; > bool nested_enable; > pgd_t *pgtable; > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 406ec46..0733618 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -316,6 +316,7 @@ struct kvmppc_ops { > int size); > int (*store_to_eaddr)(struct kvm_vcpu *vcpu, ulong *eaddr, void *ptr, > int size); > + int (*enable_svm)(struct kvm *kvm); > int (*svm_off)(struct kvm *kvm); > }; > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index fbc55a1..36da720 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -5423,6 +5423,18 @@ static void unpin_vpa_reset(struct kvm *kvm, struct kvmppc_vpa *vpa) > } > > /* > + * Enable a guest to become a secure VM. > + * Called when the KVM_CAP_PPC_SECURE_GUEST capability is enabled. > + */ > +static int kvmhv_enable_svm(struct kvm *kvm) > +{ > + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) > + return -EINVAL; > + kvm->arch.svm_enabled = 1; > + return 0; > +} > + > +/* > * IOCTL handler to turn off secure mode of guest > * > * - Release all device pages > @@ -5543,6 +5555,7 @@ static struct kvmppc_ops kvm_ops_hv = { > .enable_nested = kvmhv_enable_nested, > .load_from_eaddr = kvmhv_load_from_eaddr, > .store_to_eaddr = kvmhv_store_to_eaddr, > + .enable_svm = kvmhv_enable_svm, > .svm_off = kvmhv_svm_off, > }; > > diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c > index 79b1202..2ad999f 100644 > --- a/arch/powerpc/kvm/book3s_hv_uvmem.c > +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c > @@ -216,6 +216,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) > if (!kvm_is_radix(kvm)) > return H_UNSUPPORTED; > > + /* NAK the transition to secure if not enabled */ > + if (!kvm->arch.svm_enabled) > + return H_AUTHORITY; > + > srcu_idx = srcu_read_lock(&kvm->srcu); > slots = kvm_memslots(kvm); > kvm_for_each_memslot(memslot, slots) { > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 62ee66d..c32e6cc2 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -670,6 +670,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > (hv_enabled && cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)); > break; > #endif > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > + case KVM_CAP_PPC_SECURE_GUEST: > + r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR); > + break; > +#endif > default: > r = 0; > break; > @@ -2170,6 +2175,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, > r = kvm->arch.kvm_ops->enable_nested(kvm); > break; > #endif > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > + case KVM_CAP_PPC_SECURE_GUEST: > + r = -EINVAL; > + if (!is_kvmppc_hv_enabled(kvm) || !kvm->arch.kvm_ops->enable_svm) > + break; > + r = kvm->arch.kvm_ops->enable_svm(kvm); > + break; > +#endif > default: > r = -EINVAL; > break; > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 5e6234c..428c7dd 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1016,6 +1016,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_ARM_INJECT_EXT_DABT 178 > #define KVM_CAP_S390_VCPU_RESETS 179 > #define KVM_CAP_S390_PROTECTED 180 > +#define KVM_CAP_PPC_SECURE_GUEST 181 > > #ifdef KVM_CAP_IRQ_ROUTING > >
On Thu, Mar 19, 2020 at 03:33:01PM +1100, Paul Mackerras wrote: > At present, on Power systems with Protected Execution Facility > hardware and an ultravisor, a KVM guest can transition to being a > secure guest at will. Userspace (QEMU) has no way of knowing > whether a host system is capable of running secure guests. This > will present a problem in future when the ultravisor is capable of > migrating secure guests from one host to another, because > virtualization management software will have no way to ensure that > secure guests only run in domains where all of the hosts can > support secure guests. > > This adds a VM capability which has two functions: (a) userspace > can query it to find out whether the host can support secure guests, > and (b) userspace can enable it for a guest, which allows that > guest to become a secure guest. If userspace does not enable it, > KVM will return an error when the ultravisor does the hypercall > that indicates that the guest is starting to transition to a > secure guest. The ultravisor will then abort the transition and > the guest will terminate. > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> > --- > Note, only compile-tested. Ram, please test. > > Documentation/virt/kvm/api.rst | 17 +++++++++++++++++ > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/include/asm/kvm_ppc.h | 1 + > arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++ > arch/powerpc/kvm/book3s_hv_uvmem.c | 4 ++++ > arch/powerpc/kvm/powerpc.c | 13 +++++++++++++ > include/uapi/linux/kvm.h | 1 + > 7 files changed, 50 insertions(+) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 158d118..a925500 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -5779,6 +5779,23 @@ it hard or impossible to use it correctly. The availability of > KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 signals that those bugs are fixed. > Userspace should not try to use KVM_CAP_MANUAL_DIRTY_LOG_PROTECT. > > +7.19 KVM_CAP_PPC_SECURE_GUEST > +------------------------------ > + > +:Architectures: ppc > + > +This capability indicates that KVM is running on a host that has > +ultravisor firmware and thus can support a secure guest. On such a > +system, a guest can ask the ultravisor to make it a secure guest, > +one whose memory is inaccessible to the host except for pages which > +are explicitly requested to be shared with the host. The ultravisor > +notifies KVM when a guest requests to become a secure guest, and KVM > +has the opportunity to veto the transition. > + > +If present, this capability can be enabled for a VM, meaning that KVM > +will allow the transition to secure guest mode. Otherwise KVM will > +veto the transition. > + > 8. Other capabilities. > ====================== > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 6e8b8ff..f99b433 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -303,6 +303,7 @@ struct kvm_arch { > u8 radix; > u8 fwnmi_enabled; > u8 secure_guest; > + u8 svm_enabled; > bool threads_indep; > bool nested_enable; > pgd_t *pgtable; > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 406ec46..0733618 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -316,6 +316,7 @@ struct kvmppc_ops { > int size); > int (*store_to_eaddr)(struct kvm_vcpu *vcpu, ulong *eaddr, void *ptr, > int size); > + int (*enable_svm)(struct kvm *kvm); > int (*svm_off)(struct kvm *kvm); > }; > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index fbc55a1..36da720 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -5423,6 +5423,18 @@ static void unpin_vpa_reset(struct kvm *kvm, struct kvmppc_vpa *vpa) > } > > /* > + * Enable a guest to become a secure VM. > + * Called when the KVM_CAP_PPC_SECURE_GUEST capability is enabled. > + */ > +static int kvmhv_enable_svm(struct kvm *kvm) > +{ > + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) > + return -EINVAL; > + kvm->arch.svm_enabled = 1; > + return 0; > +} > + > +/* > * IOCTL handler to turn off secure mode of guest > * > * - Release all device pages > @@ -5543,6 +5555,7 @@ static struct kvmppc_ops kvm_ops_hv = { > .enable_nested = kvmhv_enable_nested, > .load_from_eaddr = kvmhv_load_from_eaddr, > .store_to_eaddr = kvmhv_store_to_eaddr, > + .enable_svm = kvmhv_enable_svm, > .svm_off = kvmhv_svm_off, > }; > > diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c > index 79b1202..2ad999f 100644 > --- a/arch/powerpc/kvm/book3s_hv_uvmem.c > +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c > @@ -216,6 +216,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) > if (!kvm_is_radix(kvm)) > return H_UNSUPPORTED; > > + /* NAK the transition to secure if not enabled */ > + if (!kvm->arch.svm_enabled) > + return H_AUTHORITY; > + > srcu_idx = srcu_read_lock(&kvm->srcu); > slots = kvm_memslots(kvm); > kvm_for_each_memslot(memslot, slots) { > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 62ee66d..c32e6cc2 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -670,6 +670,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > (hv_enabled && cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)); > break; > #endif > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > + case KVM_CAP_PPC_SECURE_GUEST: > + r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR); We also need to check if the kvmppc_uvmem_init() has been successfully called and initialized. r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR) && kvmppc_uvmem_bitmap; RP
On Thu, Mar 19, 2020 at 12:41:08PM -0700, Ram Pai wrote: > On Thu, Mar 19, 2020 at 03:33:01PM +1100, Paul Mackerras wrote: [snip] > > --- a/arch/powerpc/kvm/powerpc.c > > +++ b/arch/powerpc/kvm/powerpc.c > > @@ -670,6 +670,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > > (hv_enabled && cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)); > > break; > > #endif > > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > > + case KVM_CAP_PPC_SECURE_GUEST: > > + r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR); > > We also need to check if the kvmppc_uvmem_init() has been successfully > called and initialized. > > r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR) > && kvmppc_uvmem_bitmap; Well I can't do that exactly because kvmppc_uvmem_bitmap is in a different module (the kvm_hv module, whereas this code is in the kvm module), and I wouldn't want to depend on kvmppc_uvmem_bitmap, since that's an internal implementation detail. The firmware_has_feature(FW_FEATURE_ULTRAVISOR) test ultimately depends on there being a device tree node with "ibm,ultravisor" in its compatible property (see early_init_dt_scan_ultravisor()). So that means there is an ultravisor there. The cases where that test would pass but kvmppc_uvmem_bitmap == NULL would be those where the device tree nodes are present but not right, or where the host is so short of memory that it couldn't allocate the kvmppc_uvmem_bitmap. If you think those cases are worth worrying about then I will have to devise a way to do the test without depending on any symbols from the kvm-hv module. Paul.
On Fri, Mar 20, 2020 at 10:17:13AM +1100, Paul Mackerras wrote: > On Thu, Mar 19, 2020 at 12:41:08PM -0700, Ram Pai wrote: > > On Thu, Mar 19, 2020 at 03:33:01PM +1100, Paul Mackerras wrote: > [snip] > > > --- a/arch/powerpc/kvm/powerpc.c > > > +++ b/arch/powerpc/kvm/powerpc.c > > > @@ -670,6 +670,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > > > (hv_enabled && cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)); > > > break; > > > #endif > > > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > > > + case KVM_CAP_PPC_SECURE_GUEST: > > > + r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR); > > > > We also need to check if the kvmppc_uvmem_init() has been successfully > > called and initialized. > > > > r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR) > > && kvmppc_uvmem_bitmap; > > Well I can't do that exactly because kvmppc_uvmem_bitmap is in a > different module (the kvm_hv module, whereas this code is in the kvm > module), and I wouldn't want to depend on kvmppc_uvmem_bitmap, since > that's an internal implementation detail. yes. checking for kvmppc_uvmem_bitmap depends on internal implementation detail. Its also a loose approximation. There has to be something better which can tell, if everything needed to support secure guests, is available and initialized. > > The firmware_has_feature(FW_FEATURE_ULTRAVISOR) test ultimately > depends on there being a device tree node with "ibm,ultravisor" in its > compatible property (see early_init_dt_scan_ultravisor()). So that > means there is an ultravisor there. The cases where that test would > pass but kvmppc_uvmem_bitmap == NULL would be those where the device > tree nodes are present but not right, or where the host is so short of > memory that it couldn't allocate the kvmppc_uvmem_bitmap. If you > think those cases are worth worrying about then I will have to devise > a way to do the test without depending on any symbols from the kvm-hv > module. the cases, where incorrect behavior can happen; if we do not have this additional check, are -- a) zero secure memory in the system. b) "kvmppc_uvmem" memory region is not defined. c) the memory region fails to map. d) kvmppc_uvmem_bitmap allocation failed. All these are possible to varying level of certainity. I do not know we should be concerned about these possibilities. But if we do, than will a patch like this help? compile tested. ------------------ diff --git a/arch/powerpc/include/asm/kvm_book3s_uvmem.h b/arch/powerpc/include/asm/kvm_book3s_uvmem.h index 5a9834e..643c497 100644 --- a/arch/powerpc/include/asm/kvm_book3s_uvmem.h +++ b/arch/powerpc/include/asm/kvm_book3s_uvmem.h @@ -4,6 +4,7 @@ #ifdef CONFIG_PPC_UV int kvmppc_uvmem_init(void); +int kvmppc_uv_enabled(void); void kvmppc_uvmem_free(void); int kvmppc_uvmem_slot_init(struct kvm *kvm, const struct kvm_memory_slot *slot); void kvmppc_uvmem_slot_free(struct kvm *kvm, @@ -28,6 +29,11 @@ static inline int kvmppc_uvmem_init(void) return 0; } +static inline int kvmppc_uv_enabled(void) +{ + return 0; +} + static inline void kvmppc_uvmem_free(void) { } static inline int diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c index 79b1202..3331ac5 100644 --- a/arch/powerpc/kvm/book3s_hv_uvmem.c +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c @@ -804,6 +804,11 @@ int kvmppc_uvmem_init(void) return ret; } +int kvmppc_uv_enabled(void) +{ + return !kvmppc_uvmem_bitmap; +} + void kvmppc_uvmem_free(void) { memunmap_pages(&kvmppc_uvmem_pgmap); ------------------ > > Paul.
On Thu, Mar 19, 2020 at 03:33:01PM +1100, Paul Mackerras wrote: > At present, on Power systems with Protected Execution Facility > hardware and an ultravisor, a KVM guest can transition to being a > secure guest at will. Userspace (QEMU) has no way of knowing > whether a host system is capable of running secure guests. This > will present a problem in future when the ultravisor is capable of > migrating secure guests from one host to another, because > virtualization management software will have no way to ensure that > secure guests only run in domains where all of the hosts can > support secure guests. > > This adds a VM capability which has two functions: (a) userspace > can query it to find out whether the host can support secure guests, > and (b) userspace can enable it for a guest, which allows that > guest to become a secure guest. If userspace does not enable it, > KVM will return an error when the ultravisor does the hypercall > that indicates that the guest is starting to transition to a > secure guest. The ultravisor will then abort the transition and > the guest will terminate. > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > Note, only compile-tested. Ram, please test. > > Documentation/virt/kvm/api.rst | 17 +++++++++++++++++ > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/include/asm/kvm_ppc.h | 1 + > arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++ > arch/powerpc/kvm/book3s_hv_uvmem.c | 4 ++++ > arch/powerpc/kvm/powerpc.c | 13 +++++++++++++ > include/uapi/linux/kvm.h | 1 + > 7 files changed, 50 insertions(+) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 158d118..a925500 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -5779,6 +5779,23 @@ it hard or impossible to use it correctly. The availability of > KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 signals that those bugs are fixed. > Userspace should not try to use KVM_CAP_MANUAL_DIRTY_LOG_PROTECT. > > +7.19 KVM_CAP_PPC_SECURE_GUEST > +------------------------------ > + > +:Architectures: ppc > + > +This capability indicates that KVM is running on a host that has > +ultravisor firmware and thus can support a secure guest. On such a > +system, a guest can ask the ultravisor to make it a secure guest, > +one whose memory is inaccessible to the host except for pages which > +are explicitly requested to be shared with the host. The ultravisor > +notifies KVM when a guest requests to become a secure guest, and KVM > +has the opportunity to veto the transition. > + > +If present, this capability can be enabled for a VM, meaning that KVM > +will allow the transition to secure guest mode. Otherwise KVM will > +veto the transition. > + > 8. Other capabilities. > ====================== > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 6e8b8ff..f99b433 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -303,6 +303,7 @@ struct kvm_arch { > u8 radix; > u8 fwnmi_enabled; > u8 secure_guest; > + u8 svm_enabled; > bool threads_indep; > bool nested_enable; > pgd_t *pgtable; > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 406ec46..0733618 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -316,6 +316,7 @@ struct kvmppc_ops { > int size); > int (*store_to_eaddr)(struct kvm_vcpu *vcpu, ulong *eaddr, void *ptr, > int size); > + int (*enable_svm)(struct kvm *kvm); > int (*svm_off)(struct kvm *kvm); > }; > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index fbc55a1..36da720 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -5423,6 +5423,18 @@ static void unpin_vpa_reset(struct kvm *kvm, struct kvmppc_vpa *vpa) > } > > /* > + * Enable a guest to become a secure VM. > + * Called when the KVM_CAP_PPC_SECURE_GUEST capability is enabled. > + */ > +static int kvmhv_enable_svm(struct kvm *kvm) > +{ > + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) > + return -EINVAL; > + kvm->arch.svm_enabled = 1; > + return 0; > +} > + > +/* > * IOCTL handler to turn off secure mode of guest > * > * - Release all device pages > @@ -5543,6 +5555,7 @@ static struct kvmppc_ops kvm_ops_hv = { > .enable_nested = kvmhv_enable_nested, > .load_from_eaddr = kvmhv_load_from_eaddr, > .store_to_eaddr = kvmhv_store_to_eaddr, > + .enable_svm = kvmhv_enable_svm, > .svm_off = kvmhv_svm_off, > }; > > diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c > index 79b1202..2ad999f 100644 > --- a/arch/powerpc/kvm/book3s_hv_uvmem.c > +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c > @@ -216,6 +216,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) > if (!kvm_is_radix(kvm)) > return H_UNSUPPORTED; > > + /* NAK the transition to secure if not enabled */ > + if (!kvm->arch.svm_enabled) > + return H_AUTHORITY; > + > srcu_idx = srcu_read_lock(&kvm->srcu); > slots = kvm_memslots(kvm); > kvm_for_each_memslot(memslot, slots) { > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 62ee66d..c32e6cc2 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -670,6 +670,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > (hv_enabled && cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)); > break; > #endif > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > + case KVM_CAP_PPC_SECURE_GUEST: > + r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR); > + break; > +#endif > default: > r = 0; > break; > @@ -2170,6 +2175,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, > r = kvm->arch.kvm_ops->enable_nested(kvm); > break; > #endif > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) > + case KVM_CAP_PPC_SECURE_GUEST: > + r = -EINVAL; > + if (!is_kvmppc_hv_enabled(kvm) || !kvm->arch.kvm_ops->enable_svm) > + break; > + r = kvm->arch.kvm_ops->enable_svm(kvm); > + break; > +#endif > default: > r = -EINVAL; > break; > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 5e6234c..428c7dd 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1016,6 +1016,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_ARM_INJECT_EXT_DABT 178 > #define KVM_CAP_S390_VCPU_RESETS 179 > #define KVM_CAP_S390_PROTECTED 180 > +#define KVM_CAP_PPC_SECURE_GUEST 181 > > #ifdef KVM_CAP_IRQ_ROUTING >
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 158d118..a925500 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -5779,6 +5779,23 @@ it hard or impossible to use it correctly. The availability of KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 signals that those bugs are fixed. Userspace should not try to use KVM_CAP_MANUAL_DIRTY_LOG_PROTECT. +7.19 KVM_CAP_PPC_SECURE_GUEST +------------------------------ + +:Architectures: ppc + +This capability indicates that KVM is running on a host that has +ultravisor firmware and thus can support a secure guest. On such a +system, a guest can ask the ultravisor to make it a secure guest, +one whose memory is inaccessible to the host except for pages which +are explicitly requested to be shared with the host. The ultravisor +notifies KVM when a guest requests to become a secure guest, and KVM +has the opportunity to veto the transition. + +If present, this capability can be enabled for a VM, meaning that KVM +will allow the transition to secure guest mode. Otherwise KVM will +veto the transition. + 8. Other capabilities. ====================== diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 6e8b8ff..f99b433 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -303,6 +303,7 @@ struct kvm_arch { u8 radix; u8 fwnmi_enabled; u8 secure_guest; + u8 svm_enabled; bool threads_indep; bool nested_enable; pgd_t *pgtable; diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 406ec46..0733618 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -316,6 +316,7 @@ struct kvmppc_ops { int size); int (*store_to_eaddr)(struct kvm_vcpu *vcpu, ulong *eaddr, void *ptr, int size); + int (*enable_svm)(struct kvm *kvm); int (*svm_off)(struct kvm *kvm); }; diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index fbc55a1..36da720 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -5423,6 +5423,18 @@ static void unpin_vpa_reset(struct kvm *kvm, struct kvmppc_vpa *vpa) } /* + * Enable a guest to become a secure VM. + * Called when the KVM_CAP_PPC_SECURE_GUEST capability is enabled. + */ +static int kvmhv_enable_svm(struct kvm *kvm) +{ + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) + return -EINVAL; + kvm->arch.svm_enabled = 1; + return 0; +} + +/* * IOCTL handler to turn off secure mode of guest * * - Release all device pages @@ -5543,6 +5555,7 @@ static struct kvmppc_ops kvm_ops_hv = { .enable_nested = kvmhv_enable_nested, .load_from_eaddr = kvmhv_load_from_eaddr, .store_to_eaddr = kvmhv_store_to_eaddr, + .enable_svm = kvmhv_enable_svm, .svm_off = kvmhv_svm_off, }; diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c index 79b1202..2ad999f 100644 --- a/arch/powerpc/kvm/book3s_hv_uvmem.c +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c @@ -216,6 +216,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) if (!kvm_is_radix(kvm)) return H_UNSUPPORTED; + /* NAK the transition to secure if not enabled */ + if (!kvm->arch.svm_enabled) + return H_AUTHORITY; + srcu_idx = srcu_read_lock(&kvm->srcu); slots = kvm_memslots(kvm); kvm_for_each_memslot(memslot, slots) { diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 62ee66d..c32e6cc2 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -670,6 +670,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) (hv_enabled && cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)); break; #endif +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) + case KVM_CAP_PPC_SECURE_GUEST: + r = hv_enabled && !!firmware_has_feature(FW_FEATURE_ULTRAVISOR); + break; +#endif default: r = 0; break; @@ -2170,6 +2175,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, r = kvm->arch.kvm_ops->enable_nested(kvm); break; #endif +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_UV) + case KVM_CAP_PPC_SECURE_GUEST: + r = -EINVAL; + if (!is_kvmppc_hv_enabled(kvm) || !kvm->arch.kvm_ops->enable_svm) + break; + r = kvm->arch.kvm_ops->enable_svm(kvm); + break; +#endif default: r = -EINVAL; break; diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 5e6234c..428c7dd 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1016,6 +1016,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_ARM_INJECT_EXT_DABT 178 #define KVM_CAP_S390_VCPU_RESETS 179 #define KVM_CAP_S390_PROTECTED 180 +#define KVM_CAP_PPC_SECURE_GUEST 181 #ifdef KVM_CAP_IRQ_ROUTING
At present, on Power systems with Protected Execution Facility hardware and an ultravisor, a KVM guest can transition to being a secure guest at will. Userspace (QEMU) has no way of knowing whether a host system is capable of running secure guests. This will present a problem in future when the ultravisor is capable of migrating secure guests from one host to another, because virtualization management software will have no way to ensure that secure guests only run in domains where all of the hosts can support secure guests. This adds a VM capability which has two functions: (a) userspace can query it to find out whether the host can support secure guests, and (b) userspace can enable it for a guest, which allows that guest to become a secure guest. If userspace does not enable it, KVM will return an error when the ultravisor does the hypercall that indicates that the guest is starting to transition to a secure guest. The ultravisor will then abort the transition and the guest will terminate. Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- Note, only compile-tested. Ram, please test. Documentation/virt/kvm/api.rst | 17 +++++++++++++++++ arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/include/asm/kvm_ppc.h | 1 + arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++ arch/powerpc/kvm/book3s_hv_uvmem.c | 4 ++++ arch/powerpc/kvm/powerpc.c | 13 +++++++++++++ include/uapi/linux/kvm.h | 1 + 7 files changed, 50 insertions(+)