Message ID | 20200618222222.23175-3-walling@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Use DIAG318 to set Control Program Name & Version Codes | expand |
On 6/19/20 12:22 AM, Collin Walling wrote: > DIAGNOSE 0x318 (diag318) sets information regarding the environment > the VM is running in (Linux, z/VM, etc) and is observed via > firmware/service events. > > This is a privileged s390x instruction that must be intercepted by > SIE. Userspace handles the instruction as well as migration. Data > is communicated via VCPU register synchronization. > > The Control Program Name Code (CPNC) is stored in the SIE block. The > CPNC along with the Control Program Version Code (CPVC) are stored > in the kvm_vcpu_arch struct. > > The CPNC is shadowed/unshadowed in VSIE. > > This data is reset on load normal and clear resets. > > Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Could you extend the s390 kvm selftests sync_regs test with diag318 please? I'd also like to have it added to the kvm unit tests. You can either do that yourself or I'll add it when I go over my pending patches. Since we can't retrieve these values from the VM, a simple check for the sclp feature bit and an execution of the instruction would be enough. > --- > arch/s390/include/asm/kvm_host.h | 4 +++- > arch/s390/include/uapi/asm/kvm.h | 5 ++++- > arch/s390/kvm/kvm-s390.c | 11 ++++++++++- > arch/s390/kvm/vsie.c | 3 +++ > include/uapi/linux/kvm.h | 1 + > 5 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h > index 3d554887794e..8bdf6f1607ca 100644 > --- a/arch/s390/include/asm/kvm_host.h > +++ b/arch/s390/include/asm/kvm_host.h > @@ -260,7 +260,8 @@ struct kvm_s390_sie_block { > __u32 scaol; /* 0x0064 */ > __u8 sdf; /* 0x0068 */ > __u8 epdx; /* 0x0069 */ > - __u8 reserved6a[2]; /* 0x006a */ > + __u8 cpnc; /* 0x006a */ > + __u8 reserved6b; /* 0x006b */ > __u32 todpr; /* 0x006c */ > #define GISA_FORMAT1 0x00000001 > __u32 gd; /* 0x0070 */ > @@ -745,6 +746,7 @@ struct kvm_vcpu_arch { > bool gs_enabled; > bool skey_enabled; > struct kvm_s390_pv_vcpu pv; > + union diag318_info diag318_info; > }; > > struct kvm_vm_stat { > diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h > index 436ec7636927..2ae1b660086c 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_DIAG318 (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_DIAG318) > > /* length and alignment of the sdnx as a power of two */ > #define SDNXC 8 > @@ -254,6 +256,7 @@ struct kvm_sync_regs { > __u64 pft; /* pfault token [PFAULT] */ > __u64 pfs; /* pfault select [PFAULT] */ > __u64 pfc; /* pfault compare [PFAULT] */ > + __u64 diag318; /* diagnose 0x318 info */ > union { > __u64 vrs[32][2]; /* vector registers (KVM_SYNC_VRS) */ > __u64 fprs[16]; /* fp registers (KVM_SYNC_FPRS) */ > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index d0ff26d157bc..b05ad718b64b 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -545,6 +545,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > case KVM_CAP_S390_AIS_MIGRATION: > case KVM_CAP_S390_VCPU_RESETS: > case KVM_CAP_SET_GUEST_DEBUG: > + case KVM_CAP_S390_DIAG318: > r = 1; > break; > case KVM_CAP_S390_HPAGE_1M: > @@ -3267,7 +3268,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > KVM_SYNC_ACRS | > KVM_SYNC_CRS | > KVM_SYNC_ARCH0 | > - KVM_SYNC_PFAULT; > + KVM_SYNC_PFAULT | > + KVM_SYNC_DIAG318; > kvm_s390_set_prefix(vcpu, 0); > if (test_kvm_facility(vcpu->kvm, 64)) > vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB; > @@ -3562,6 +3564,7 @@ static void kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu) > vcpu->arch.sie_block->pp = 0; > vcpu->arch.sie_block->fpf &= ~FPF_BPBC; > vcpu->arch.sie_block->todpr = 0; > + vcpu->arch.sie_block->cpnc = 0; > } > } > > @@ -3579,6 +3582,7 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu) > > regs->etoken = 0; > regs->etoken_extension = 0; > + regs->diag318 = 0; > } > > int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) > @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) > kvm_clear_async_pf_completion_queue(vcpu); > } > + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { > + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; > + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; > + } > /* > * If userspace sets the riccb (e.g. after migration) to a valid state, > * we should enable RI here instead of doing the lazy enablement. > @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; > 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.diag318 = vcpu->arch.diag318_info.val; > if (MACHINE_HAS_GS) { > __ctl_set_bit(2, 4); > if (vcpu->arch.gs_enabled) > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index 9e9056cebfcf..ba83d0568bc7 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > break; > } > > + scb_o->cpnc = scb_s->cpnc; > + > if (scb_s->ihcpu != 0xffffU) > scb_o->ihcpu = scb_s->ihcpu; > } > @@ -548,6 +550,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > scb_s->ecd |= scb_o->ecd & ECD_ETOKENF; > > scb_s->hpid = HPID_VSIE; > + scb_s->cpnc = scb_o->cpnc; > > prepare_ibc(vcpu, vsie_page); > rc = shadow_crycb(vcpu, vsie_page); > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 4fdf30316582..35cdb4307904 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_PPC_SECURE_GUEST 181 > #define KVM_CAP_HALT_POLL 182 > #define KVM_CAP_ASYNC_PF_INT 183 > +#define KVM_CAP_S390_DIAG318 184 > > #ifdef KVM_CAP_IRQ_ROUTING > >
On 6/19/20 7:02 AM, Janosch Frank wrote: > On 6/19/20 12:22 AM, Collin Walling wrote: >> DIAGNOSE 0x318 (diag318) sets information regarding the environment >> the VM is running in (Linux, z/VM, etc) and is observed via >> firmware/service events. >> >> This is a privileged s390x instruction that must be intercepted by >> SIE. Userspace handles the instruction as well as migration. Data >> is communicated via VCPU register synchronization. >> >> The Control Program Name Code (CPNC) is stored in the SIE block. The >> CPNC along with the Control Program Version Code (CPVC) are stored >> in the kvm_vcpu_arch struct. >> >> The CPNC is shadowed/unshadowed in VSIE. >> >> This data is reset on load normal and clear resets. >> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> > > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > > Could you extend the s390 kvm selftests sync_regs test with diag318 please? > Can do! > I'd also like to have it added to the kvm unit tests. You can either do > that yourself or I'll add it when I go over my pending patches. Since we > can't retrieve these values from the VM, a simple check for the sclp > feature bit and an execution of the instruction would be enough. > >> --- >> arch/s390/include/asm/kvm_host.h | 4 +++- >> arch/s390/include/uapi/asm/kvm.h | 5 ++++- >> arch/s390/kvm/kvm-s390.c | 11 ++++++++++- >> arch/s390/kvm/vsie.c | 3 +++ >> include/uapi/linux/kvm.h | 1 + >> 5 files changed, 21 insertions(+), 3 deletions(-) >> >> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >> index 3d554887794e..8bdf6f1607ca 100644 >> --- a/arch/s390/include/asm/kvm_host.h >> +++ b/arch/s390/include/asm/kvm_host.h >> @@ -260,7 +260,8 @@ struct kvm_s390_sie_block { >> __u32 scaol; /* 0x0064 */ >> __u8 sdf; /* 0x0068 */ >> __u8 epdx; /* 0x0069 */ >> - __u8 reserved6a[2]; /* 0x006a */ >> + __u8 cpnc; /* 0x006a */ >> + __u8 reserved6b; /* 0x006b */ >> __u32 todpr; /* 0x006c */ >> #define GISA_FORMAT1 0x00000001 >> __u32 gd; /* 0x0070 */ >> @@ -745,6 +746,7 @@ struct kvm_vcpu_arch { >> bool gs_enabled; >> bool skey_enabled; >> struct kvm_s390_pv_vcpu pv; >> + union diag318_info diag318_info; >> }; >> >> struct kvm_vm_stat { >> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h >> index 436ec7636927..2ae1b660086c 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_DIAG318 (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_DIAG318) >> >> /* length and alignment of the sdnx as a power of two */ >> #define SDNXC 8 >> @@ -254,6 +256,7 @@ struct kvm_sync_regs { >> __u64 pft; /* pfault token [PFAULT] */ >> __u64 pfs; /* pfault select [PFAULT] */ >> __u64 pfc; /* pfault compare [PFAULT] */ >> + __u64 diag318; /* diagnose 0x318 info */ >> union { >> __u64 vrs[32][2]; /* vector registers (KVM_SYNC_VRS) */ >> __u64 fprs[16]; /* fp registers (KVM_SYNC_FPRS) */ >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index d0ff26d157bc..b05ad718b64b 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -545,6 +545,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) >> case KVM_CAP_S390_AIS_MIGRATION: >> case KVM_CAP_S390_VCPU_RESETS: >> case KVM_CAP_SET_GUEST_DEBUG: >> + case KVM_CAP_S390_DIAG318: >> r = 1; >> break; >> case KVM_CAP_S390_HPAGE_1M: >> @@ -3267,7 +3268,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) >> KVM_SYNC_ACRS | >> KVM_SYNC_CRS | >> KVM_SYNC_ARCH0 | >> - KVM_SYNC_PFAULT; >> + KVM_SYNC_PFAULT | >> + KVM_SYNC_DIAG318; >> kvm_s390_set_prefix(vcpu, 0); >> if (test_kvm_facility(vcpu->kvm, 64)) >> vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB; >> @@ -3562,6 +3564,7 @@ static void kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu) >> vcpu->arch.sie_block->pp = 0; >> vcpu->arch.sie_block->fpf &= ~FPF_BPBC; >> vcpu->arch.sie_block->todpr = 0; >> + vcpu->arch.sie_block->cpnc = 0; >> } >> } >> >> @@ -3579,6 +3582,7 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu) >> >> regs->etoken = 0; >> regs->etoken_extension = 0; >> + regs->diag318 = 0; >> } >> >> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) >> @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) >> kvm_clear_async_pf_completion_queue(vcpu); >> } >> + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { >> + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; >> + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; >> + } >> /* >> * If userspace sets the riccb (e.g. after migration) to a valid state, >> * we should enable RI here instead of doing the lazy enablement. >> @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >> kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; >> 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.diag318 = vcpu->arch.diag318_info.val; >> if (MACHINE_HAS_GS) { >> __ctl_set_bit(2, 4); >> if (vcpu->arch.gs_enabled) >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index 9e9056cebfcf..ba83d0568bc7 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> break; >> } >> >> + scb_o->cpnc = scb_s->cpnc; >> + >> if (scb_s->ihcpu != 0xffffU) >> scb_o->ihcpu = scb_s->ihcpu; >> } >> @@ -548,6 +550,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> scb_s->ecd |= scb_o->ecd & ECD_ETOKENF; >> >> scb_s->hpid = HPID_VSIE; >> + scb_s->cpnc = scb_o->cpnc; >> >> prepare_ibc(vcpu, vsie_page); >> rc = shadow_crycb(vcpu, vsie_page); >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index 4fdf30316582..35cdb4307904 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt { >> #define KVM_CAP_PPC_SECURE_GUEST 181 >> #define KVM_CAP_HALT_POLL 182 >> #define KVM_CAP_ASYNC_PF_INT 183 >> +#define KVM_CAP_S390_DIAG318 184 >> >> #ifdef KVM_CAP_IRQ_ROUTING >> >> > >
On 19.06.20 00:22, Collin Walling wrote: > DIAGNOSE 0x318 (diag318) sets information regarding the environment > the VM is running in (Linux, z/VM, etc) and is observed via > firmware/service events. > > This is a privileged s390x instruction that must be intercepted by > SIE. Userspace handles the instruction as well as migration. Data > is communicated via VCPU register synchronization. > > The Control Program Name Code (CPNC) is stored in the SIE block. The > CPNC along with the Control Program Version Code (CPVC) are stored > in the kvm_vcpu_arch struct. > > The CPNC is shadowed/unshadowed in VSIE. > [...] > > int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) > @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) > kvm_clear_async_pf_completion_queue(vcpu); > } > + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { > + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; > + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; > + } > /* > * If userspace sets the riccb (e.g. after migration) to a valid state, > * we should enable RI here instead of doing the lazy enablement. > @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; > 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.diag318 = vcpu->arch.diag318_info.val; > if (MACHINE_HAS_GS) { > __ctl_set_bit(2, 4); > if (vcpu->arch.gs_enabled) > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index 9e9056cebfcf..ba83d0568bc7 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > break; > } > > + scb_o->cpnc = scb_s->cpnc; "This is a privileged s390x instruction that must be intercepted", how can the cpnc change, then, while in SIE? Apart from that LGTM.
On 6/19/20 10:52 AM, David Hildenbrand wrote: > On 19.06.20 00:22, Collin Walling wrote: >> DIAGNOSE 0x318 (diag318) sets information regarding the environment >> the VM is running in (Linux, z/VM, etc) and is observed via >> firmware/service events. >> >> This is a privileged s390x instruction that must be intercepted by >> SIE. Userspace handles the instruction as well as migration. Data >> is communicated via VCPU register synchronization. >> >> The Control Program Name Code (CPNC) is stored in the SIE block. The >> CPNC along with the Control Program Version Code (CPVC) are stored >> in the kvm_vcpu_arch struct. >> >> The CPNC is shadowed/unshadowed in VSIE. >> > > [...] > >> >> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) >> @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) >> kvm_clear_async_pf_completion_queue(vcpu); >> } >> + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { >> + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; >> + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; >> + } >> /* >> * If userspace sets the riccb (e.g. after migration) to a valid state, >> * we should enable RI here instead of doing the lazy enablement. >> @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >> kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; >> 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.diag318 = vcpu->arch.diag318_info.val; >> if (MACHINE_HAS_GS) { >> __ctl_set_bit(2, 4); >> if (vcpu->arch.gs_enabled) >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index 9e9056cebfcf..ba83d0568bc7 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> break; >> } >> >> + scb_o->cpnc = scb_s->cpnc; > > "This is a privileged s390x instruction that must be intercepted", how > can the cpnc change, then, while in SIE? > > Apart from that LGTM. > I thought shadow/unshadow was a load/store (respectively) when executing in SIE for a level 3+ guest (where LPAR is level 1)? * Shadow SCB (load shadow VSIE page; originally CPNC is 0) * Execute diag318 (under SIE) * Unshadow SCB (store in original VSIE page; CPNC is whatever code the guest decided to set) Don't we need to preserve the CPNC for the level 3+ guest somehow?
On 19.06.20 17:47, Collin Walling wrote: > On 6/19/20 10:52 AM, David Hildenbrand wrote: >> On 19.06.20 00:22, Collin Walling wrote: >>> DIAGNOSE 0x318 (diag318) sets information regarding the environment >>> the VM is running in (Linux, z/VM, etc) and is observed via >>> firmware/service events. >>> >>> This is a privileged s390x instruction that must be intercepted by >>> SIE. Userspace handles the instruction as well as migration. Data >>> is communicated via VCPU register synchronization. >>> >>> The Control Program Name Code (CPNC) is stored in the SIE block. The >>> CPNC along with the Control Program Version Code (CPVC) are stored >>> in the kvm_vcpu_arch struct. >>> >>> The CPNC is shadowed/unshadowed in VSIE. >>> >> >> [...] >> >>> >>> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) >>> @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) >>> kvm_clear_async_pf_completion_queue(vcpu); >>> } >>> + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { >>> + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; >>> + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; >>> + } >>> /* >>> * If userspace sets the riccb (e.g. after migration) to a valid state, >>> * we should enable RI here instead of doing the lazy enablement. >>> @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>> kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; >>> 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.diag318 = vcpu->arch.diag318_info.val; >>> if (MACHINE_HAS_GS) { >>> __ctl_set_bit(2, 4); >>> if (vcpu->arch.gs_enabled) >>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>> index 9e9056cebfcf..ba83d0568bc7 100644 >>> --- a/arch/s390/kvm/vsie.c >>> +++ b/arch/s390/kvm/vsie.c >>> @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>> break; >>> } >>> >>> + scb_o->cpnc = scb_s->cpnc; >> >> "This is a privileged s390x instruction that must be intercepted", how >> can the cpnc change, then, while in SIE? >> >> Apart from that LGTM. >> > > I thought shadow/unshadow was a load/store (respectively) when executing > in SIE for a level 3+ guest (where LPAR is level 1)? > > * Shadow SCB (load shadow VSIE page; originally CPNC is 0) 1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB. > > * Execute diag318 (under SIE) 2. Here the SIE runs using the shadow SCB. > > * Unshadow SCB (store in original VSIE page; CPNC is whatever code the > guest decided to set) 3. Here you copy back the cpnc from the shadow SCB to the pinned (original) SCB. If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be dropped, because the values will always match. If guest3 tries to modify the cpnc (via diag 318), we exit the SIE (intercept) in 2., return to our guest 2. guest 2 will perform the change and adapt the original SCB. (yep, it's confusing) Or did I miss anything?
On 6/19/20 1:17 PM, David Hildenbrand wrote: > On 19.06.20 17:47, Collin Walling wrote: >> On 6/19/20 10:52 AM, David Hildenbrand wrote: >>> On 19.06.20 00:22, Collin Walling wrote: >>>> DIAGNOSE 0x318 (diag318) sets information regarding the environment >>>> the VM is running in (Linux, z/VM, etc) and is observed via >>>> firmware/service events. >>>> >>>> This is a privileged s390x instruction that must be intercepted by >>>> SIE. Userspace handles the instruction as well as migration. Data >>>> is communicated via VCPU register synchronization. >>>> >>>> The Control Program Name Code (CPNC) is stored in the SIE block. The >>>> CPNC along with the Control Program Version Code (CPVC) are stored >>>> in the kvm_vcpu_arch struct. >>>> >>>> The CPNC is shadowed/unshadowed in VSIE. >>>> >>> >>> [...] >>> >>>> >>>> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) >>>> @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>>> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) >>>> kvm_clear_async_pf_completion_queue(vcpu); >>>> } >>>> + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { >>>> + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; >>>> + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; >>>> + } >>>> /* >>>> * If userspace sets the riccb (e.g. after migration) to a valid state, >>>> * we should enable RI here instead of doing the lazy enablement. >>>> @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>>> kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; >>>> 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.diag318 = vcpu->arch.diag318_info.val; >>>> if (MACHINE_HAS_GS) { >>>> __ctl_set_bit(2, 4); >>>> if (vcpu->arch.gs_enabled) >>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>>> index 9e9056cebfcf..ba83d0568bc7 100644 >>>> --- a/arch/s390/kvm/vsie.c >>>> +++ b/arch/s390/kvm/vsie.c >>>> @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>>> break; >>>> } >>>> >>>> + scb_o->cpnc = scb_s->cpnc; >>> >>> "This is a privileged s390x instruction that must be intercepted", how >>> can the cpnc change, then, while in SIE? >>> >>> Apart from that LGTM. >>> >> >> I thought shadow/unshadow was a load/store (respectively) when executing >> in SIE for a level 3+ guest (where LPAR is level 1)? >> >> * Shadow SCB (load shadow VSIE page; originally CPNC is 0) > > 1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB. > >> >> * Execute diag318 (under SIE) > > 2. Here the SIE runs using the shadow SCB. > >> >> * Unshadow SCB (store in original VSIE page; CPNC is whatever code the >> guest decided to set) > > 3. Here you copy back the cpnc from the shadow SCB to the pinned > (original) SCB. > > > If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be > dropped, because the values will always match. > > > If guest3 tries to modify the cpnc (via diag 318), we exit the SIE > (intercept) in 2., return to our guest 2. guest 2 will perform the > change and adapt the original SCB. > > (yep, it's confusing) > > Or did I miss anything? > Ah, I see. So the shadowing isn't necessarily for SIE block values, but for storing the register / PSW / clock states, as well as facility bits for the level 3+ guests? Looking at what the vsie code does, that seems to make sense. So we don't need to shadow OR unshadow the CPNC, then?
> Am 19.06.2020 um 19:56 schrieb Collin Walling <walling@linux.ibm.com>: > > On 6/19/20 1:17 PM, David Hildenbrand wrote: >>> On 19.06.20 17:47, Collin Walling wrote: >>> On 6/19/20 10:52 AM, David Hildenbrand wrote: >>>> On 19.06.20 00:22, Collin Walling wrote: >>>>> DIAGNOSE 0x318 (diag318) sets information regarding the environment >>>>> the VM is running in (Linux, z/VM, etc) and is observed via >>>>> firmware/service events. >>>>> >>>>> This is a privileged s390x instruction that must be intercepted by >>>>> SIE. Userspace handles the instruction as well as migration. Data >>>>> is communicated via VCPU register synchronization. >>>>> >>>>> The Control Program Name Code (CPNC) is stored in the SIE block. The >>>>> CPNC along with the Control Program Version Code (CPVC) are stored >>>>> in the kvm_vcpu_arch struct. >>>>> >>>>> The CPNC is shadowed/unshadowed in VSIE. >>>>> >>>> >>>> [...] >>>> >>>>> >>>>> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) >>>>> @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>>>> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) >>>>> kvm_clear_async_pf_completion_queue(vcpu); >>>>> } >>>>> + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { >>>>> + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; >>>>> + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; >>>>> + } >>>>> /* >>>>> * If userspace sets the riccb (e.g. after migration) to a valid state, >>>>> * we should enable RI here instead of doing the lazy enablement. >>>>> @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>>>> kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; >>>>> 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.diag318 = vcpu->arch.diag318_info.val; >>>>> if (MACHINE_HAS_GS) { >>>>> __ctl_set_bit(2, 4); >>>>> if (vcpu->arch.gs_enabled) >>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>>>> index 9e9056cebfcf..ba83d0568bc7 100644 >>>>> --- a/arch/s390/kvm/vsie.c >>>>> +++ b/arch/s390/kvm/vsie.c >>>>> @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>>>> break; >>>>> } >>>>> >>>>> + scb_o->cpnc = scb_s->cpnc; >>>> >>>> "This is a privileged s390x instruction that must be intercepted", how >>>> can the cpnc change, then, while in SIE? >>>> >>>> Apart from that LGTM. >>>> >>> >>> I thought shadow/unshadow was a load/store (respectively) when executing >>> in SIE for a level 3+ guest (where LPAR is level 1)? >>> >>> * Shadow SCB (load shadow VSIE page; originally CPNC is 0) >> >> 1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB. >> >>> >>> * Execute diag318 (under SIE) >> >> 2. Here the SIE runs using the shadow SCB. >> >>> >>> * Unshadow SCB (store in original VSIE page; CPNC is whatever code the >>> guest decided to set) >> >> 3. Here you copy back the cpnc from the shadow SCB to the pinned >> (original) SCB. >> >> >> If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be >> dropped, because the values will always match. >> >> >> If guest3 tries to modify the cpnc (via diag 318), we exit the SIE >> (intercept) in 2., return to our guest 2. guest 2 will perform the >> change and adapt the original SCB. >> >> (yep, it's confusing) >> >> Or did I miss anything? >> > > Ah, I see. So the shadowing isn't necessarily for SIE block values, but > for storing the register / PSW / clock states, as well as facility bits > for the level 3+ guests? Looking at what the We have to forward all values the SIE has to see and copy back only what could have been changed by the SIE. > vsie code does, that seems > to make sense. > > So we don't need to shadow OR unshadow the CPNC, then? I think you have to shadow (forward the value) but not unshadow (value cannot change). Cheers! > > -- > Regards, > Collin > > Stay safe and stay healthy >
On 6/19/20 2:13 PM, David Hildenbrand wrote: > > >> Am 19.06.2020 um 19:56 schrieb Collin Walling <walling@linux.ibm.com>: >> >> On 6/19/20 1:17 PM, David Hildenbrand wrote: >>>> On 19.06.20 17:47, Collin Walling wrote: >>>> On 6/19/20 10:52 AM, David Hildenbrand wrote: >>>>> On 19.06.20 00:22, Collin Walling wrote: >>>>>> DIAGNOSE 0x318 (diag318) sets information regarding the environment >>>>>> the VM is running in (Linux, z/VM, etc) and is observed via >>>>>> firmware/service events. >>>>>> >>>>>> This is a privileged s390x instruction that must be intercepted by >>>>>> SIE. Userspace handles the instruction as well as migration. Data >>>>>> is communicated via VCPU register synchronization. >>>>>> >>>>>> The Control Program Name Code (CPNC) is stored in the SIE block. The >>>>>> CPNC along with the Control Program Version Code (CPVC) are stored >>>>>> in the kvm_vcpu_arch struct. >>>>>> >>>>>> The CPNC is shadowed/unshadowed in VSIE. >>>>>> >>>>> >>>>> [...] >>>>> >>>>>> >>>>>> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) >>>>>> @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>>>>> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) >>>>>> kvm_clear_async_pf_completion_queue(vcpu); >>>>>> } >>>>>> + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { >>>>>> + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; >>>>>> + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; >>>>>> + } >>>>>> /* >>>>>> * If userspace sets the riccb (e.g. after migration) to a valid state, >>>>>> * we should enable RI here instead of doing the lazy enablement. >>>>>> @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) >>>>>> kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; >>>>>> 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.diag318 = vcpu->arch.diag318_info.val; >>>>>> if (MACHINE_HAS_GS) { >>>>>> __ctl_set_bit(2, 4); >>>>>> if (vcpu->arch.gs_enabled) >>>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>>>>> index 9e9056cebfcf..ba83d0568bc7 100644 >>>>>> --- a/arch/s390/kvm/vsie.c >>>>>> +++ b/arch/s390/kvm/vsie.c >>>>>> @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>>>>> break; >>>>>> } >>>>>> >>>>>> + scb_o->cpnc = scb_s->cpnc; >>>>> >>>>> "This is a privileged s390x instruction that must be intercepted", how >>>>> can the cpnc change, then, while in SIE? >>>>> >>>>> Apart from that LGTM. >>>>> >>>> >>>> I thought shadow/unshadow was a load/store (respectively) when executing >>>> in SIE for a level 3+ guest (where LPAR is level 1)? >>>> >>>> * Shadow SCB (load shadow VSIE page; originally CPNC is 0) >>> >>> 1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB. >>> >>>> >>>> * Execute diag318 (under SIE) >>> >>> 2. Here the SIE runs using the shadow SCB. >>> >>>> >>>> * Unshadow SCB (store in original VSIE page; CPNC is whatever code the >>>> guest decided to set) >>> >>> 3. Here you copy back the cpnc from the shadow SCB to the pinned >>> (original) SCB. >>> >>> >>> If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be >>> dropped, because the values will always match. >>> >>> >>> If guest3 tries to modify the cpnc (via diag 318), we exit the SIE >>> (intercept) in 2., return to our guest 2. guest 2 will perform the >>> change and adapt the original SCB. >>> >>> (yep, it's confusing) >>> >>> Or did I miss anything? >>> >> >> Ah, I see. So the shadowing isn't necessarily for SIE block values, but >> for storing the register / PSW / clock states, as well as facility bits >> for the level 3+ guests? Looking at what the > > We have to forward all values the SIE has to see and copy back only what could have been changed by the SIE. > >> vsie code does, that seems >> to make sense. >> >> So we don't need to shadow OR unshadow the CPNC, then? > > I think you have to shadow (forward the value) but not unshadow (value cannot change). > > Cheers! > Gotcha. Very tricky. I'll have to study on it some more. Thanks for the info! Take care. >> >> -- >> Regards, >> Collin >> >> Stay safe and stay healthy >> >
On Thu, 18 Jun 2020 18:22:22 -0400 Collin Walling <walling@linux.ibm.com> wrote: > DIAGNOSE 0x318 (diag318) sets information regarding the environment > the VM is running in (Linux, z/VM, etc) and is observed via > firmware/service events. > > This is a privileged s390x instruction that must be intercepted by > SIE. Userspace handles the instruction as well as migration. Data > is communicated via VCPU register synchronization. > > The Control Program Name Code (CPNC) is stored in the SIE block. The > CPNC along with the Control Program Version Code (CPVC) are stored > in the kvm_vcpu_arch struct. > > The CPNC is shadowed/unshadowed in VSIE. > > This data is reset on load normal and clear resets. > > Signed-off-by: Collin Walling <walling@linux.ibm.com> > --- > arch/s390/include/asm/kvm_host.h | 4 +++- > arch/s390/include/uapi/asm/kvm.h | 5 ++++- > arch/s390/kvm/kvm-s390.c | 11 ++++++++++- > arch/s390/kvm/vsie.c | 3 +++ > include/uapi/linux/kvm.h | 1 + > 5 files changed, 21 insertions(+), 3 deletions(-) > (...) > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 4fdf30316582..35cdb4307904 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_PPC_SECURE_GUEST 181 > #define KVM_CAP_HALT_POLL 182 > #define KVM_CAP_ASYNC_PF_INT 183 > +#define KVM_CAP_S390_DIAG318 184 Do we strictly need this new cap, or would checking against the sync regs capabilities be enough? > > #ifdef KVM_CAP_IRQ_ROUTING >
On 22.06.20 12:24, Cornelia Huck wrote: > On Thu, 18 Jun 2020 18:22:22 -0400 > Collin Walling <walling@linux.ibm.com> wrote: > >> DIAGNOSE 0x318 (diag318) sets information regarding the environment >> the VM is running in (Linux, z/VM, etc) and is observed via >> firmware/service events. >> >> This is a privileged s390x instruction that must be intercepted by >> SIE. Userspace handles the instruction as well as migration. Data >> is communicated via VCPU register synchronization. >> >> The Control Program Name Code (CPNC) is stored in the SIE block. The >> CPNC along with the Control Program Version Code (CPVC) are stored >> in the kvm_vcpu_arch struct. >> >> The CPNC is shadowed/unshadowed in VSIE. >> >> This data is reset on load normal and clear resets. >> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> --- >> arch/s390/include/asm/kvm_host.h | 4 +++- >> arch/s390/include/uapi/asm/kvm.h | 5 ++++- >> arch/s390/kvm/kvm-s390.c | 11 ++++++++++- >> arch/s390/kvm/vsie.c | 3 +++ >> include/uapi/linux/kvm.h | 1 + >> 5 files changed, 21 insertions(+), 3 deletions(-) >> > > (...) > >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index 4fdf30316582..35cdb4307904 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt { >> #define KVM_CAP_PPC_SECURE_GUEST 181 >> #define KVM_CAP_HALT_POLL 182 >> #define KVM_CAP_ASYNC_PF_INT 183 >> +#define KVM_CAP_S390_DIAG318 184 > > Do we strictly need this new cap, or would checking against the sync > regs capabilities be enough? We could check the sync_regs valid field to decide about the sync. We do that for ETOKEN as well and QEMU also uses it in handle_diag_318. I think what this is used for is actually to tell the QEMU CPU model if this is there. And for that the sync_reg validity seems wrong. So better keep the CAP?
On Mon, 22 Jun 2020 16:50:41 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 22.06.20 12:24, Cornelia Huck wrote: > > On Thu, 18 Jun 2020 18:22:22 -0400 > > Collin Walling <walling@linux.ibm.com> wrote: > > > >> DIAGNOSE 0x318 (diag318) sets information regarding the environment > >> the VM is running in (Linux, z/VM, etc) and is observed via > >> firmware/service events. > >> > >> This is a privileged s390x instruction that must be intercepted by > >> SIE. Userspace handles the instruction as well as migration. Data > >> is communicated via VCPU register synchronization. > >> > >> The Control Program Name Code (CPNC) is stored in the SIE block. The > >> CPNC along with the Control Program Version Code (CPVC) are stored > >> in the kvm_vcpu_arch struct. > >> > >> The CPNC is shadowed/unshadowed in VSIE. > >> > >> This data is reset on load normal and clear resets. > >> > >> Signed-off-by: Collin Walling <walling@linux.ibm.com> > >> --- > >> arch/s390/include/asm/kvm_host.h | 4 +++- > >> arch/s390/include/uapi/asm/kvm.h | 5 ++++- > >> arch/s390/kvm/kvm-s390.c | 11 ++++++++++- > >> arch/s390/kvm/vsie.c | 3 +++ > >> include/uapi/linux/kvm.h | 1 + > >> 5 files changed, 21 insertions(+), 3 deletions(-) > >> > > > > (...) > > > >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > >> index 4fdf30316582..35cdb4307904 100644 > >> --- a/include/uapi/linux/kvm.h > >> +++ b/include/uapi/linux/kvm.h > >> @@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt { > >> #define KVM_CAP_PPC_SECURE_GUEST 181 > >> #define KVM_CAP_HALT_POLL 182 > >> #define KVM_CAP_ASYNC_PF_INT 183 > >> +#define KVM_CAP_S390_DIAG318 184 > > > > Do we strictly need this new cap, or would checking against the sync > > regs capabilities be enough? > > We could check the sync_regs valid field to decide about the sync. We do > that for ETOKEN as well and QEMU also uses it in handle_diag_318. > > I think what this is used for is actually to tell the QEMU CPU model > if this is there. And for that the sync_reg validity seems wrong. So better > keep the CAP? > Ok, makes sense.
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index 3d554887794e..8bdf6f1607ca 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -260,7 +260,8 @@ struct kvm_s390_sie_block { __u32 scaol; /* 0x0064 */ __u8 sdf; /* 0x0068 */ __u8 epdx; /* 0x0069 */ - __u8 reserved6a[2]; /* 0x006a */ + __u8 cpnc; /* 0x006a */ + __u8 reserved6b; /* 0x006b */ __u32 todpr; /* 0x006c */ #define GISA_FORMAT1 0x00000001 __u32 gd; /* 0x0070 */ @@ -745,6 +746,7 @@ struct kvm_vcpu_arch { bool gs_enabled; bool skey_enabled; struct kvm_s390_pv_vcpu pv; + union diag318_info diag318_info; }; struct kvm_vm_stat { diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h index 436ec7636927..2ae1b660086c 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_DIAG318 (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_DIAG318) /* length and alignment of the sdnx as a power of two */ #define SDNXC 8 @@ -254,6 +256,7 @@ struct kvm_sync_regs { __u64 pft; /* pfault token [PFAULT] */ __u64 pfs; /* pfault select [PFAULT] */ __u64 pfc; /* pfault compare [PFAULT] */ + __u64 diag318; /* diagnose 0x318 info */ union { __u64 vrs[32][2]; /* vector registers (KVM_SYNC_VRS) */ __u64 fprs[16]; /* fp registers (KVM_SYNC_FPRS) */ diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index d0ff26d157bc..b05ad718b64b 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -545,6 +545,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_S390_AIS_MIGRATION: case KVM_CAP_S390_VCPU_RESETS: case KVM_CAP_SET_GUEST_DEBUG: + case KVM_CAP_S390_DIAG318: r = 1; break; case KVM_CAP_S390_HPAGE_1M: @@ -3267,7 +3268,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) KVM_SYNC_ACRS | KVM_SYNC_CRS | KVM_SYNC_ARCH0 | - KVM_SYNC_PFAULT; + KVM_SYNC_PFAULT | + KVM_SYNC_DIAG318; kvm_s390_set_prefix(vcpu, 0); if (test_kvm_facility(vcpu->kvm, 64)) vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB; @@ -3562,6 +3564,7 @@ static void kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu) vcpu->arch.sie_block->pp = 0; vcpu->arch.sie_block->fpf &= ~FPF_BPBC; vcpu->arch.sie_block->todpr = 0; + vcpu->arch.sie_block->cpnc = 0; } } @@ -3579,6 +3582,7 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu) regs->etoken = 0; regs->etoken_extension = 0; + regs->diag318 = 0; } int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) @@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) kvm_clear_async_pf_completion_queue(vcpu); } + if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) { + vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318; + vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc; + } /* * If userspace sets the riccb (e.g. after migration) to a valid state, * we should enable RI here instead of doing the lazy enablement. @@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; 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.diag318 = vcpu->arch.diag318_info.val; if (MACHINE_HAS_GS) { __ctl_set_bit(2, 4); if (vcpu->arch.gs_enabled) diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index 9e9056cebfcf..ba83d0568bc7 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) break; } + scb_o->cpnc = scb_s->cpnc; + if (scb_s->ihcpu != 0xffffU) scb_o->ihcpu = scb_s->ihcpu; } @@ -548,6 +550,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) scb_s->ecd |= scb_o->ecd & ECD_ETOKENF; scb_s->hpid = HPID_VSIE; + scb_s->cpnc = scb_o->cpnc; prepare_ibc(vcpu, vsie_page); rc = shadow_crycb(vcpu, vsie_page); diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 4fdf30316582..35cdb4307904 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_PPC_SECURE_GUEST 181 #define KVM_CAP_HALT_POLL 182 #define KVM_CAP_ASYNC_PF_INT 183 +#define KVM_CAP_S390_DIAG318 184 #ifdef KVM_CAP_IRQ_ROUTING
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events. This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization. The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct. The CPNC is shadowed/unshadowed in VSIE. This data is reset on load normal and clear resets. Signed-off-by: Collin Walling <walling@linux.ibm.com> --- arch/s390/include/asm/kvm_host.h | 4 +++- arch/s390/include/uapi/asm/kvm.h | 5 ++++- arch/s390/kvm/kvm-s390.c | 11 ++++++++++- arch/s390/kvm/vsie.c | 3 +++ include/uapi/linux/kvm.h | 1 + 5 files changed, 21 insertions(+), 3 deletions(-)