| Message ID | 20200622154636.5499-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 Mon, 22 Jun 2020 11:46:36 -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. > > This data is reset on load normal and clear resets. Looks good to me AFAICS without access to the architecture. Acked-by: Cornelia Huck <cohuck@redhat.com> One small thing below. > > Signed-off-by: Collin Walling <walling@linux.ibm.com> > Reviewed-by: Janosch Frank <frankja@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 | 1 + > include/uapi/linux/kvm.h | 1 + > 5 files changed, 19 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 Should we document this in Documentation/virt/kvm/api.rst? (Documentation of KVM caps generally seems to be a bit of a hit-and-miss, though. But we could at least document the s390 ones :) I also noticed that the new entries for the vcpu resets and pv do not seem to be in proper rst format. Maybe fix that and add the new doc in an add-on series? > > #ifdef KVM_CAP_IRQ_ROUTING >
On 6/22/20 12:04 PM, Cornelia Huck wrote: > On Mon, 22 Jun 2020 11:46:36 -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. >> >> This data is reset on load normal and clear resets. > > Looks good to me AFAICS without access to the architecture. > > Acked-by: Cornelia Huck <cohuck@redhat.com> > > One small thing below. > >> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> Reviewed-by: Janosch Frank <frankja@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 | 1 + >> include/uapi/linux/kvm.h | 1 + >> 5 files changed, 19 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 > > Should we document this in Documentation/virt/kvm/api.rst? > > (Documentation of KVM caps generally seems to be a bit of a > hit-and-miss, though. But we could at least document the s390 ones :) > > I also noticed that the new entries for the vcpu resets and pv do not > seem to be in proper rst format. Maybe fix that and add the new doc in > an add-on series? > Sure thing. I'll fix up the rst and add docs for the new DIAG cap. >> >> #ifdef KVM_CAP_IRQ_ROUTING >> >
On 6/22/20 12:13 PM, Collin Walling wrote: > On 6/22/20 12:04 PM, Cornelia Huck wrote: >> On Mon, 22 Jun 2020 11:46:36 -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. >>> >>> This data is reset on load normal and clear resets. >> >> Looks good to me AFAICS without access to the architecture. >> >> Acked-by: Cornelia Huck <cohuck@redhat.com> >> >> One small thing below. >> >>> >>> Signed-off-by: Collin Walling <walling@linux.ibm.com> >>> Reviewed-by: Janosch Frank <frankja@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 | 1 + >>> include/uapi/linux/kvm.h | 1 + >>> 5 files changed, 19 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 >> >> Should we document this in Documentation/virt/kvm/api.rst? >> >> (Documentation of KVM caps generally seems to be a bit of a >> hit-and-miss, though. But we could at least document the s390 ones :) >> >> I also noticed that the new entries for the vcpu resets and pv do not >> seem to be in proper rst format. Maybe fix that and add the new doc in >> an add-on series? >> > > Sure thing. I'll fix up the rst and add docs for the new DIAG cap. > >>> >>> #ifdef KVM_CAP_IRQ_ROUTING >>> >> > > Mind if I get some early feedback for the first run? How does this sound: 8.24 KVM_CAP_S390_DIAG318 ------------------------- :Architecture: s390 This capability allows for information regarding the control program that may be observed via system/firmware service events. The availability of this capability indicates that KVM handling of the register synchronization, reset, and VSIE shadowing of the DIAGNOSE 0x318 related information is present. The information associated with the instruction is an 8-byte value consisting of a one-byte Control Program Name Code (CPNC), and a 7-byte Control Program Version Code (CPVC). The CPNC determines what environment the control program is running in (e.g. Linux, z/VM...), and the CPVC is used for extraneous information specific to OS (e.g. Linux version, Linux distribution...) The CPNC must be stored in the SIE block for the CPU that executes the diag instruction, which is communicated from userspace to KVM via register synchronization using the KVM_SYNC_DIAG318 flag. Both codes are stored together in the kvm_vcpu_arch struct.
On Mon, 22 Jun 2020 12:23:45 -0400 Collin Walling <walling@linux.ibm.com> wrote: > Mind if I get some early feedback for the first run? How does this sound: > > 8.24 KVM_CAP_S390_DIAG318 > ------------------------- > > :Architecture: s390 > > This capability allows for information regarding the control program > that may be observed via system/firmware service events. The > availability of this capability indicates that KVM handling of the > register synchronization, reset, and VSIE shadowing of the DIAGNOSE > 0x318 related information is present. > > The information associated with the instruction is an 8-byte value > consisting of a one-byte Control Program Name Code (CPNC), and a 7-byte > Control Program Version Code (CPVC). The CPNC determines what > environment the control program is running in (e.g. Linux, z/VM...), and > the CPVC is used for extraneous information specific to OS (e.g. Linux > version, Linux distribution...) > > The CPNC must be stored in the SIE block for the CPU that executes the > diag instruction, which is communicated from userspace to KVM via > register synchronization using the KVM_SYNC_DIAG318 flag. Both codes are > stored together in the kvm_vcpu_arch struct. Hm... what about replacing that last paragraph with "If this capability is available, the CPNC and CPVC are available for synchronization between KVM and userspace via the sync regs mechanism (KVM_SYNC_DIAG318)." ?
On 6/22/20 12:35 PM, Cornelia Huck wrote: > On Mon, 22 Jun 2020 12:23:45 -0400 > Collin Walling <walling@linux.ibm.com> wrote: > >> Mind if I get some early feedback for the first run? How does this sound: >> >> 8.24 KVM_CAP_S390_DIAG318 >> ------------------------- >> >> :Architecture: s390 >> >> This capability allows for information regarding the control program >> that may be observed via system/firmware service events. The >> availability of this capability indicates that KVM handling of the >> register synchronization, reset, and VSIE shadowing of the DIAGNOSE >> 0x318 related information is present. >> >> The information associated with the instruction is an 8-byte value >> consisting of a one-byte Control Program Name Code (CPNC), and a 7-byte >> Control Program Version Code (CPVC). The CPNC determines what >> environment the control program is running in (e.g. Linux, z/VM...), and >> the CPVC is used for extraneous information specific to OS (e.g. Linux >> version, Linux distribution...) >> >> The CPNC must be stored in the SIE block for the CPU that executes the >> diag instruction, which is communicated from userspace to KVM via >> register synchronization using the KVM_SYNC_DIAG318 flag. Both codes are >> stored together in the kvm_vcpu_arch struct. > > Hm... what about replacing that last paragraph with > > "If this capability is available, the CPNC and CPVC are available for > synchronization between KVM and userspace via the sync regs mechanism > (KVM_SYNC_DIAG318)." > > ? > I like it!
On 22.06.20 17:46, 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. > > 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> LGTM Reviewed-by: David Hildenbrand <david@redhat.com>
On 22/06/2020 17.46, 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. > > 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> > --- > 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 | 1 + > include/uapi/linux/kvm.h | 1 + > 5 files changed, 19 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) */ It's been a while since I touched kvm_sync_regs the last time ... but can your really extend this structure right in the middle without breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I think you rather have to add this add the end or e.g. put it into the padding2 region or something like that...? Or do I miss something? Thomas
On 23.06.20 10:42, Thomas Huth wrote: > On 22/06/2020 17.46, 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. >> >> 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> >> --- >> 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 | 1 + >> include/uapi/linux/kvm.h | 1 + >> 5 files changed, 19 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) */ > > It's been a while since I touched kvm_sync_regs the last time ... but > can your really extend this structure right in the middle without > breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I > think you rather have to add this add the end or e.g. put it into the > padding2 region or something like that...? Or do I miss something? Argh. You are right. It should go to the end and not in the middle. Will fixup.
On 23.06.20 10:45, Christian Borntraeger wrote: > > > On 23.06.20 10:42, Thomas Huth wrote: >> On 22/06/2020 17.46, 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. >>> >>> 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> >>> --- >>> 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 | 1 + >>> include/uapi/linux/kvm.h | 1 + >>> 5 files changed, 19 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) */ >> >> It's been a while since I touched kvm_sync_regs the last time ... but >> can your really extend this structure right in the middle without >> breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I >> think you rather have to add this add the end or e.g. put it into the >> padding2 region or something like that...? Or do I miss something? > > Argh. You are right. It should go to the end and not in the middle. Will fixup. > Something like this on top. diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h index 2ae1b660086c..7a6b14874d65 100644 --- a/arch/s390/include/uapi/asm/kvm.h +++ b/arch/s390/include/uapi/asm/kvm.h @@ -256,7 +256,6 @@ 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) */ @@ -267,7 +266,8 @@ struct kvm_sync_regs { __u8 reserved2 : 7; __u8 padding1[51]; /* riccb needs to be 64byte aligned */ __u8 riccb[64]; /* runtime instrumentation controls block */ - __u8 padding2[192]; /* sdnx needs to be 256byte aligned */ + __u64 diag318; /* diagnose 0x318 info */ + __u8 padding2[184]; /* sdnx needs to be 256byte aligned */ union { __u8 sdnx[SDNXL]; /* state description annex */ struct {
On 23/06/2020 10.47, Christian Borntraeger wrote: > > > On 23.06.20 10:45, Christian Borntraeger wrote: >> >> >> On 23.06.20 10:42, Thomas Huth wrote: >>> On 22/06/2020 17.46, 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. >>>> >>>> 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> >>>> --- >>>> 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 | 1 + >>>> include/uapi/linux/kvm.h | 1 + >>>> 5 files changed, 19 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) */ >>> >>> It's been a while since I touched kvm_sync_regs the last time ... but >>> can your really extend this structure right in the middle without >>> breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I >>> think you rather have to add this add the end or e.g. put it into the >>> padding2 region or something like that...? Or do I miss something? >> >> Argh. You are right. It should go to the end and not in the middle. Will fixup. >> > > Something like this on top. > > diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h > index 2ae1b660086c..7a6b14874d65 100644 > --- a/arch/s390/include/uapi/asm/kvm.h > +++ b/arch/s390/include/uapi/asm/kvm.h > @@ -256,7 +256,6 @@ 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) */ > @@ -267,7 +266,8 @@ struct kvm_sync_regs { > __u8 reserved2 : 7; > __u8 padding1[51]; /* riccb needs to be 64byte aligned */ > __u8 riccb[64]; /* runtime instrumentation controls block */ > - __u8 padding2[192]; /* sdnx needs to be 256byte aligned */ > + __u64 diag318; /* diagnose 0x318 info */ > + __u8 padding2[184]; /* sdnx needs to be 256byte aligned */ > union { > __u8 sdnx[SDNXL]; /* state description annex */ > struct { Ack! Thomas
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..4f3cbf6003a9 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -548,6 +548,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