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@xxxxxxxxxxxxx>
>> ---
>> 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
>
Attachment:
signature.asc
Description: OpenPGP digital signature
