Use DIAG318 to set Control Program Name & Version Codes
v8 cover Use DIAG318 to set Control Program Name & Version Codes
Changelog:
v8
• Reset is handled in KVM during initial and clear resets
• Sync/Store register handling
• Removed device IOCTL code
• Added KVM Capability for DIAG318
- this is for determining if the CPU model can enable this feature
• Reverted changes introduced by bullet 3 in v7
• Unshadowing CPNC again, as it makes sense if the guest executing in VSIE sets a unique name/version code; this data should be preserved
- reverts bullet 4 in v3
• Diag318 is no longer reported via VM or CPU events
- no place to put this such that the messages aren't flooding the logs
- not necessary, as this data is primarily for IBM hardware/firmware service events, and is observable via such events (e.g. CPU ring dump)
- was nice for testing purposes
• A copy of the diag318 info (name & version code) is now stored in the kvm_vcpu_arch struct, as opposed to the kvm_arch struct
v7
• Removed diag handler, as it will now take place within userspace
• Removed KVM_S390_VM_MISC_ENABLE_DIAG318 (undoes first bullet in v6)
• Misc clean ups and fixes [removed in v8]
- introduced a new patch to s/diag318/diag_318 and s/byte_134/fac134 to keep things consistent with the rest of the code
v6
• KVM disables diag318 get/set by default [removed in v7]
• added new IOCTL to tell KVM to enable diag318 [removed in v7]
• removed VCPU event message in favor of VM_EVENT only [removed in v8]
v5
• s/cpc/diag318_info in order to make the relevant data more clear
• removed mutex locks for diag318_info get/set
v4
• removed setup.c changes introduced in bullet 1 of v3
• kept diag318_info struct cleanup
• analogous QEMU patches:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00164.html
v3
• kernel patch for diag 0x318 instruction call fixup [removed in v4]
• removed CPU model code
• cleaned up diag318_info struct
• cpnc is no longer unshadowed as it was not needed [removed in v8]
• rebased on 5.1.0-rc3
-------------------------------------------------------------------------------
This instruction call is executed once-and-only-once during Kernel setup. The availability of this instruction depends on Read Info byte 134 (aka fac134), bit 0.
DIAG 0x318's is handled by userspace and may be enabled for a guest even if the host kernel cannot support it.
The diag318_info is composed of a Control Program Name Code (CPNC) and a Control Program Version Code (CPVC). The CPNC is stored in the SIE block, and the CPNC & CPVC pair is stored in the kvm_vcpu_arch struct.
These values are used for problem diagnosis and allows IBM to identify control program information by answering the following question:
"What environment is this guest running in?" (CPNC) "What are more details regarding the OS?" (CPVC)
In the future, we will implement the CPVC to convey more information about the OS (such as Linux version and potentially some value denoting a specific distro + release). For now, we set this field to 0 until we come up with a solid plan.
Collin Walling (2):
s390/setup: diag 318: refactor struct
s390/kvm: diagnose 0x318 sync and reset
arch/s390/include/asm/diag.h | 6 ++---- arch/s390/include/asm/kvm_host.h | 4 +++- arch/s390/include/uapi/asm/kvm.h | 5 ++++- arch/s390/kernel/setup.c | 3 +-- arch/s390/kvm/kvm-s390.c | 11 ++++++++++- arch/s390/kvm/vsie.c | 3 +++ include/uapi/linux/kvm.h | 1 + 7 files changed, 24 insertions(+), 9 deletions(-)
Signature · 1 line
2.21.3
PATCH v8 1/2 s390/setup: diag 318: refactor struct
The diag 318 struct introduced in include/asm/diag.h can be reused in KVM, so let's condense the version code fields in the diag318_info struct for easier usage and simplify it until we can determine how the data should be formatted.
--- arch/s390/include/asm/diag.h | 6 ++---- arch/s390/kernel/setup.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)
arch/s390/include/asm/diag.h +2 -4
@@ -298,10 +298,8 @@ struct diag26c_mac_resp {union diag318_info {unsigned long val;struct { -unsigned int cpnc :8 ; -unsigned int cpvc_linux :24 ; -unsigned char cpvc_distro[3 ]; -unsigned char zero; +unsigned long cpnc :8 ; +unsigned long cpvc :56 ; }; };
arch/s390/kernel/setup.c +1 -2
@@ -1021,8 +1021,7 @@ static void __init setup_control_program_code(void) {union diag318_info diag318_info = { .cpnc =CPNC_LINUX , - .cpvc_linux =0 , - .cpvc_distro = {0 }, + .cpvc =0 , };if (!sclp.has_diag318)
Signature · 1 line
2.21.3
Re: [PATCH v8 1/2] s390/setup: diag 318: refactor struct
Quoted 10 lines · Collin Walling
The diag 318 struct introduced in
include/asm/diag.hcan be reused in KVM, so let's condense the version code fields in thediag318_infostruct for easier usage and simplify it until we can determine how the data should be formatted.Signed-off-by Collin Walling <[email protected]>Reviewed-by David Hildenbrand <[email protected]>Reviewed-by Cornelia Huck <[email protected]>Reviewed-by Thomas Huth <[email protected]>Reviewed-by Janosch Frank <[email protected]>
Series looks good to me. Can you respin the 2nd patch regarding the VSIE things and I can then apply it.
Quoted 37 lines
--- arch/s390/include/asm/diag.h | 6 ++---- arch/s390/kernel/setup.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)arch/s390/include/asm/diag.h +2
-4@@ -298,10 +298,8 @@ struct diag26c_mac_resp {union diag318_info {unsigned long val;struct { -unsigned int cpnc :8 ; -unsigned int cpvc_linux :24 ; -unsigned char cpvc_distro[3 ]; -unsigned char zero; +unsigned long cpnc :8 ; +unsigned long cpvc :56 ; }; };arch/s390/kernel/setup.c +1
-2@@ -1021,8 +1021,7 @@ static void __init setup_control_program_code(void) {union diag318_info diag318_info = { .cpnc =CPNC_LINUX , - .cpvc_linux =0 , - .cpvc_distro = {0 }, + .cpvc =0 , };if (!sclp.has_diag318)
Re: [PATCH v8 1/2] s390/setup: diag 318: refactor struct
Quoted 7 lines · Christian Borntraeger
Quoted 5 lines · Collin Walling
The diag 318 struct introduced in
include/asm/diag.hcan be reused in KVM, so let's condense the version code fields in thediag318_infostruct for easier usage and simplify it until we can determine how the data should be formatted.
Quoted 10 lines
Quoted 5 lines
Signed-off-by Collin Walling <[email protected]>Reviewed-by David Hildenbrand <[email protected]>Reviewed-by Cornelia Huck <[email protected]>Reviewed-by Thomas Huth <[email protected]>Reviewed-by Janosch Frank <[email protected]>Series looks good to me. Can you respin the 2nd patch regarding the VSIE things and I can then apply it.
Will do. Thanks.
Quoted 37 lines
Quoted 37 lines
--- arch/s390/include/asm/diag.h | 6 ++---- arch/s390/kernel/setup.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)arch/s390/include/asm/diag.h +2
-4@@ -298,10 +298,8 @@ struct diag26c_mac_resp {union diag318_info {unsigned long val;struct { -unsigned int cpnc :8 ; -unsigned int cpvc_linux :24 ; -unsigned char cpvc_distro[3 ]; -unsigned char zero; +unsigned long cpnc :8 ; +unsigned long cpvc :56 ; }; };arch/s390/kernel/setup.c +1
-2@@ -1021,8 +1021,7 @@ static void __init setup_control_program_code(void) {union diag318_info diag318_info = { .cpnc =CPNC_LINUX , - .cpvc_linux =0 , - .cpvc_distro = {0 }, + .cpvc =0 , };if (!sclp.has_diag318)
Signature · 4 lines
Regards, Collin Stay safe and stay healthy
PATCH v8 2/2 s390/kvm: diagnose 0x318 sync and reset
In reply to Collin WallingDIAGNOSE 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.
--- 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(-)
arch/s390/include/asm/kvm_host.h +3 -1
@@ -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 {
arch/s390/include/uapi/asm/kvm.h +4 -1
@@ -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) */
arch/s390/kvm/kvm-s390.c +10 -1
@@ -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)
arch/s390/kvm/vsie.c +3 -0
@@ -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);
include/uapi/linux/kvm.h +1 -0
@@ -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
Signature · 1 line
2.21.3
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 12 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.
Quoted 5 lines
The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
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.
Quoted 148 lines
--- 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(-)arch/s390/include/asm/kvm_host.h +3
-1@@ -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 {arch/s390/include/uapi/asm/kvm.h +4
-1@@ -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) */ arch/s390/kvm/kvm-s390.c +10
-1@@ -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)arch/s390/kvm/vsie.c +3
-0@@ -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);include/uapi/linux/kvm.h +1
-0@@ -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
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 15 lines · Janosch Frank
Quoted 14 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
Quoted 8 lines
Quoted 3 lines
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>Reviewed-by Janosch Frank <[email protected]>Could you extend the s390 kvm selftests
sync_regstest with diag318 please?
Can do!
Quoted 155 lines
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.
Quoted 148 lines
--- 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(-)arch/s390/include/asm/kvm_host.h +3
-1@@ -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 {arch/s390/include/uapi/asm/kvm.h +4
-1@@ -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) */ arch/s390/kvm/kvm-s390.c +10
-1@@ -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)arch/s390/kvm/vsie.c +3
-0@@ -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);include/uapi/linux/kvm.h +1
-0@@ -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
Signature · 4 lines
Regards, Collin Stay safe and stay healthy
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
In reply to Collin WallingQuoted 14 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int
kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)arch/s390/kvm/kvm-s390.c +5
-0@@ -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)arch/s390/kvm/vsie.c +0
-0
arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1
-0@@ -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.
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 47 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 29 lines
int
kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)arch/s390/kvm/kvm-s390.c +5
-0@@ -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)arch/s390/kvm/vsie.c +0
-0@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)break ; }
Quoted 7 lines
arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 1 line
arch/s390/kvm/vsie.c +1
-0+ 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?
Signature · 4 lines
Regards, Collin Stay safe and stay healthy
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 53 lines · Collin Walling
Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int
kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)arch/s390/kvm/kvm-s390.c +5
-0@@ -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)arch/s390/kvm/vsie.c +0
-0arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1
-0@@ -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?
Quoted 7 lines
Quoted 2 lines
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.
Quoted 2 lines
* Execute diag318 (under SIE)
2. Here the SIE runs using the shadow SCB.
Quoted 3 lines
* 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?
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 81 lines · David Hildenbrand
Quoted 53 lines · Collin Walling
Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int
kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)arch/s390/kvm/kvm-s390.c +5
-0@@ -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)arch/s390/kvm/vsie.c +0
-0arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1
-0@@ -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?
Quoted 7 lines
Quoted 2 lines
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.
Quoted 2 lines
* Execute diag318 (under SIE)
2. Here the SIE runs using the shadow SCB.
Quoted 3 lines
* 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.
Quoted 8 lines
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?
Signature · 4 lines
Regards, Collin Stay safe and stay healthy
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 90 lines
Am 19.06.2020 um 19:56 schrieb Collin Walling <[email protected]>:
Quoted 85 lines · David Hildenbrand
Quoted 54 lines · Collin Walling
Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int
kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)arch/s390/kvm/kvm-s390.c +5
-0@@ -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)arch/s390/kvm/vsie.c +0
-0arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1
-0@@ -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?
Quoted 7 lines
Quoted 2 lines
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.
Quoted 2 lines
* Execute diag318 (under SIE)
2. Here the SIE runs using the shadow SCB.
Quoted 3 lines
* 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.
Quoted 8 lines
Quoted 4 lines
(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.
Quoted 4 lines
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!
Quoted 7 lines
Signature · 5 lines
-- Regards, Collin Stay safe and stay healthy
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 101 lines · David Hildenbrand
Quoted 90 lines
Am 19.06.2020 um 19:56 schrieb Collin Walling <[email protected]>:
Quoted 85 lines · David Hildenbrand
Quoted 54 lines · Collin Walling
Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int
kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)arch/s390/kvm/kvm-s390.c +5
-0@@ -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)arch/s390/kvm/vsie.c +0
-0arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1
-0@@ -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?
Quoted 7 lines
Quoted 2 lines
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.
Quoted 2 lines
* Execute diag318 (under SIE)
2. Here the SIE runs using the shadow SCB.
Quoted 3 lines
* 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.
Quoted 8 lines
Quoted 4 lines
(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.
Quoted 9 lines
Quoted 4 lines
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.
Quoted 8 lines
Quoted 7 lines
Signature · 5 lines
-- Regards, Collin Stay safe and stay healthy
Signature · 4 lines
Regards, Collin Stay safe and stay healthy
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
In reply to Collin WallingQuoted 25 lines · On Thu, 18 Jun 2020 18:22:22 -0400 Collin Walling <[email protected]> wrote:
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>--- 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(-)
(...)
include/uapi/linux/kvm.h struct kvm_ppc_resize_hpt { · line 1034
Quoted 9 lines
include/uapi/linux/kvm.h +1
-0@@ -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?
Quoted 3 lines
include/uapi/linux/kvm.h +0
-0#ifdef KVM_CAP_IRQ_ROUTING
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 31 lines · Cornelia Huck
Quoted 25 lines · On Thu, 18 Jun 2020 18:22:22 -0400 Collin Walling <[email protected]> wrote:
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>--- 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(-)(...)
Quoted 12 lines
include/uapi/linux/kvm.h struct kvm_ppc_resize_hpt { · line 1034
Quoted 9 lines
include/uapi/linux/kvm.h +1
-0@@ -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 184Do 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?
Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset
Quoted 45 lines · On Mon, 22 Jun 2020 16:50:41 +0200 Christian Borntraeger…
Quoted 31 lines · Cornelia Huck
Quoted 25 lines · On Thu, 18 Jun 2020 18:22:22 -0400 Collin Walling <[email protected]> wrote:
DIAGNOSE
0x318(diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed viafirmware/serviceevents.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_archstruct.The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>--- 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(-)(...)
Quoted 12 lines
include/uapi/linux/kvm.h struct kvm_ppc_resize_hpt { · line 1034
Quoted 9 lines
include/uapi/linux/kvm.h +1
-0@@ -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 184Do we strictly need this new cap, or would checking against the sync regs capabilities be enough?
Quoted 7 lines
We could check the
sync_regsvalid field to decide about the sync. We do that for ETOKEN as well and QEMU also uses it inhandle_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_regvalidity seems wrong. So better keep the CAP?
Ok, makes sense.