| Message ID | 20191024114059.102802-30-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Add support for protected VMs | expand |
On 24/10/2019 13.40, Janosch Frank wrote: > Indicate via register sync if the VM is in secure mode. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/include/uapi/asm/kvm.h | 5 ++++- > arch/s390/kvm/kvm-s390.c | 7 ++++++- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h > index 436ec7636927..b44c02426c2e 100644 > --- a/arch/s390/include/uapi/asm/kvm.h > +++ b/arch/s390/include/uapi/asm/kvm.h > @@ -231,11 +231,13 @@ struct kvm_guest_debug_arch { > #define KVM_SYNC_GSCB (1UL << 9) > #define KVM_SYNC_BPBC (1UL << 10) > #define KVM_SYNC_ETOKEN (1UL << 11) > +#define KVM_SYNC_PV (1UL << 12) > > #define KVM_SYNC_S390_VALID_FIELDS \ > (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \ > KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \ > - KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN) > + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN | \ > + KVM_SYNC_PV) > > /* length and alignment of the sdnx as a power of two */ > #define SDNXC 8 > @@ -261,6 +263,7 @@ struct kvm_sync_regs { > __u8 reserved[512]; /* for future vector expansion */ > __u32 fpc; /* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */ > __u8 bpbc : 1; /* bp mode */ > + __u8 pv : 1; /* pv mode */ > __u8 reserved2 : 7; Don't you want to decrease the reserved2 bits to 6 ? ... > __u8 padding1[51]; /* riccb needs to be 64byte aligned */ ... otherwise you might mess up the alignment here! > __u8 riccb[64]; /* runtime instrumentation controls block */ > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index f623c64aeade..500972a1f742 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -2856,6 +2856,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB; > if (test_kvm_facility(vcpu->kvm, 156)) > vcpu->run->kvm_valid_regs |= KVM_SYNC_ETOKEN; > + if (test_kvm_facility(vcpu->kvm, 161)) > + vcpu->run->kvm_valid_regs |= KVM_SYNC_PV; > /* fprs can be synchronized via vrs, even if the guest has no vx. With > * MACHINE_HAS_VX, (load|store)_fpu_regs() will work with vrs format. > */ > @@ -4136,6 +4138,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > { > kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea; > kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC; > + kvm_run->s.regs.pv = 0; > if (MACHINE_HAS_GS) { > __ctl_set_bit(2, 4); > if (vcpu->arch.gs_enabled) > @@ -4172,8 +4175,10 @@ static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > /* Restore will be done lazily at return */ > current->thread.fpu.fpc = vcpu->arch.host_fpregs.fpc; > current->thread.fpu.regs = vcpu->arch.host_fpregs.regs; > - if (likely(!kvm_s390_pv_is_protected(vcpu->kvm))) > + if (likely(!kvm_s390_pv_handle_cpu(vcpu))) Why change the if-statement now? Should this maybe rather be squashed into the patch that introduced the if-statement? > store_regs_fmt2(vcpu, kvm_run); > + else > + kvm_run->s.regs.pv = 1; > } > > int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > Thomas
On 11/15/19 10:36 AM, Thomas Huth wrote: > On 24/10/2019 13.40, Janosch Frank wrote: >> Indicate via register sync if the VM is in secure mode. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> arch/s390/include/uapi/asm/kvm.h | 5 ++++- >> arch/s390/kvm/kvm-s390.c | 7 ++++++- >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h >> index 436ec7636927..b44c02426c2e 100644 >> --- a/arch/s390/include/uapi/asm/kvm.h >> +++ b/arch/s390/include/uapi/asm/kvm.h >> @@ -231,11 +231,13 @@ struct kvm_guest_debug_arch { >> #define KVM_SYNC_GSCB (1UL << 9) >> #define KVM_SYNC_BPBC (1UL << 10) >> #define KVM_SYNC_ETOKEN (1UL << 11) >> +#define KVM_SYNC_PV (1UL << 12) >> >> #define KVM_SYNC_S390_VALID_FIELDS \ >> (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \ >> KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \ >> - KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN) >> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN | \ >> + KVM_SYNC_PV) >> >> /* length and alignment of the sdnx as a power of two */ >> #define SDNXC 8 >> @@ -261,6 +263,7 @@ struct kvm_sync_regs { >> __u8 reserved[512]; /* for future vector expansion */ >> __u32 fpc; /* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */ >> __u8 bpbc : 1; /* bp mode */ >> + __u8 pv : 1; /* pv mode */ >> __u8 reserved2 : 7; > > Don't you want to decrease the reserved2 bits to 6 ? ... Ups > >> __u8 padding1[51]; /* riccb needs to be 64byte aligned */ > > ... otherwise you might mess up the alignment here! > >> __u8 riccb[64]; /* runtime instrumentation controls block */ >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index f623c64aeade..500972a1f742 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -2856,6 +2856,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) >> vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB; >> if (test_kvm_facility(vcpu->kvm, 156)) >> vcpu->run->kvm_valid_regs |= KVM_SYNC_ETOKEN; >> + if (test_kvm_facility(vcpu->kvm, 161)) >> + vcpu->run->kvm_valid_regs |= KVM_SYNC_PV; >> /* fprs can be synchronized via vrs, even if the guest has no vx. With >> * MACHINE_HAS_VX, (load|store)_fpu_regs() will work with vrs format. >> */ >> @@ -4136,6 +4138,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >> { >> kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea; >> kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC; >> + kvm_run->s.regs.pv = 0; >> if (MACHINE_HAS_GS) { >> __ctl_set_bit(2, 4); >> if (vcpu->arch.gs_enabled) >> @@ -4172,8 +4175,10 @@ static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >> /* Restore will be done lazily at return */ >> current->thread.fpu.fpc = vcpu->arch.host_fpregs.fpc; >> current->thread.fpu.regs = vcpu->arch.host_fpregs.regs; >> - if (likely(!kvm_s390_pv_is_protected(vcpu->kvm))) >> + if (likely(!kvm_s390_pv_handle_cpu(vcpu))) > > Why change the if-statement now? Should this maybe rather be squashed > into the patch that introduced the if-statement? That was part of a cleanup that should have been done in other patches. > >> store_regs_fmt2(vcpu, kvm_run); >> + else >> + kvm_run->s.regs.pv = 1; >> } >> >> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >> > > Thomas >
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h index 436ec7636927..b44c02426c2e 100644 --- a/arch/s390/include/uapi/asm/kvm.h +++ b/arch/s390/include/uapi/asm/kvm.h @@ -231,11 +231,13 @@ struct kvm_guest_debug_arch { #define KVM_SYNC_GSCB (1UL << 9) #define KVM_SYNC_BPBC (1UL << 10) #define KVM_SYNC_ETOKEN (1UL << 11) +#define KVM_SYNC_PV (1UL << 12) #define KVM_SYNC_S390_VALID_FIELDS \ (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \ KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \ - KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN) + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN | \ + KVM_SYNC_PV) /* length and alignment of the sdnx as a power of two */ #define SDNXC 8 @@ -261,6 +263,7 @@ struct kvm_sync_regs { __u8 reserved[512]; /* for future vector expansion */ __u32 fpc; /* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */ __u8 bpbc : 1; /* bp mode */ + __u8 pv : 1; /* pv mode */ __u8 reserved2 : 7; __u8 padding1[51]; /* riccb needs to be 64byte aligned */ __u8 riccb[64]; /* runtime instrumentation controls block */ diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index f623c64aeade..500972a1f742 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2856,6 +2856,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB; if (test_kvm_facility(vcpu->kvm, 156)) vcpu->run->kvm_valid_regs |= KVM_SYNC_ETOKEN; + if (test_kvm_facility(vcpu->kvm, 161)) + vcpu->run->kvm_valid_regs |= KVM_SYNC_PV; /* fprs can be synchronized via vrs, even if the guest has no vx. With * MACHINE_HAS_VX, (load|store)_fpu_regs() will work with vrs format. */ @@ -4136,6 +4138,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) { kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea; kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC; + kvm_run->s.regs.pv = 0; if (MACHINE_HAS_GS) { __ctl_set_bit(2, 4); if (vcpu->arch.gs_enabled) @@ -4172,8 +4175,10 @@ static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) /* Restore will be done lazily at return */ current->thread.fpu.fpc = vcpu->arch.host_fpregs.fpc; current->thread.fpu.regs = vcpu->arch.host_fpregs.regs; - if (likely(!kvm_s390_pv_is_protected(vcpu->kvm))) + if (likely(!kvm_s390_pv_handle_cpu(vcpu))) store_regs_fmt2(vcpu, kvm_run); + else + kvm_run->s.regs.pv = 1; } int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
Indicate via register sync if the VM is in secure mode. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- arch/s390/include/uapi/asm/kvm.h | 5 ++++- arch/s390/kvm/kvm-s390.c | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-)