| Message ID | 1521156300-19296-7-git-send-email-akrowiak@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: vfio-ap: guest dedicated crypto adapters | expand |
On 16/03/2018 00:24, Tony Krowiak wrote: > If the CPU model indicates that AP facility is installed on > the guest (i.e., -cpu xxxx,ap=on), then the expectation is that > the AP bus running in the guest will initialize; however, if the > AP instructions are not being interpreted by the firmware, then > they will be intercepted and routed back to QEMU for handling. > If a handler is not defined to process the intercepted instruciton, > then an operation exception will be injected into the > guest, in which case the AP bus will not initialize. > > There are two situations where AP instructions will not be > interpreted: > > 1. The guest is not configured with a vfio-ap device (i.e., > -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for > the vfio-ap device enables interpretive execution of AP > instructions. > > 2. The guest is a second level guest but the first level guest has > not enabled interpretive execution. > > This patch introduces AP instruction handlers to ensure the AP bus > module initializes on the guest when the AP facility is installed > on the guest but AP instructions are not being interpreted. The logic > incorporated is: > > * If the CPU model indicates AP instructions are > installed > > * Set the status response code for the instruction to indicate that > the APQN contained in the instruction is not valid. This is > a valid response because there will be no devices configured for > the guest in any of the above scenarios. > > * Else return an error from the handler. This will result in an > operation being injected into the guest and the AP bus will not > initialize on the guest. That is commensurate with how things work > today. > > Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> > --- > hw/vfio/ap.c | 45 ++++++++++++++++++++++++++++++++++++++++++ > include/hw/s390x/ap-device.h | 6 +++++ > target/s390x/kvm.c | 14 +++++++++++++ > 3 files changed, 65 insertions(+), 0 deletions(-) > > diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c > index b397bb1..88e744d 100644 > --- a/hw/vfio/ap.c > +++ b/hw/vfio/ap.c > @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error **errp) > kvm_s390_set_interpret_ap(0); > } > > +int ap_device_handle_nqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + > + if (s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > +int ap_device_handle_dqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + > + if (s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > +int ap_device_handle_pqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + int fc = 4 & (env->regs[0] >> 24); > + > + /* > + * The Query Configuration Information (QCI) function (fc == 4) does not > + * set a response code in reg 1, so check for that along with the > + * AP feature. > + */ > + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > static Property vfio_ap_properties[] = { > DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), > DEFINE_PROP_END_OF_LIST(), > diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h > index 693df90..d45ae38 100644 > --- a/include/hw/s390x/ap-device.h > +++ b/include/hw/s390x/ap-device.h > @@ -11,6 +11,8 @@ > #ifndef HW_S390X_AP_DEVICE_H > #define HW_S390X_AP_DEVICE_H > > +#include "cpu.h" > + > #define AP_DEVICE_TYPE "ap-device" > > typedef struct APDevice { > @@ -35,4 +37,8 @@ static inline APDevice *to_ap_dev(DeviceState *dev) > #define AP_DEVICE_CLASS(klass) \ > OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE) > > +int ap_device_handle_nqap(S390CPU *cpu); > +int ap_device_handle_dqap(S390CPU *cpu); > +int ap_device_handle_pqap(S390CPU *cpu); > + > #endif /* HW_S390X_AP_DEVICE_H */ > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index 2812e28..a636394 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -50,6 +50,7 @@ > #include "exec/memattrs.h" > #include "hw/s390x/s390-virtio-ccw.h" > #include "hw/s390x/s390-virtio-hcall.h" > +#include "hw/s390x/ap-device.h" > > #ifndef DEBUG_KVM > #define DEBUG_KVM 0 > @@ -88,6 +89,9 @@ > #define PRIV_B2_CHSC 0x5f > #define PRIV_B2_SIGA 0x74 > #define PRIV_B2_XSCH 0x76 > +#define PRIV_B2_NQAP 0xad > +#define PRIV_B2_DQAP 0xae > +#define PRIV_B2_PQAP 0xaf > > #define PRIV_EB_SQBS 0x8a > #define PRIV_EB_PCISTB 0xd0 > @@ -1245,6 +1249,16 @@ static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) > case PRIV_B2_SCLP_CALL: > rc = kvm_sclp_service_call(cpu, run, ipbh0); > break; > + case PRIV_B2_NQAP: > + rc = ap_device_handle_nqap(cpu); > + break; > + case PRIV_B2_DQAP: > + rc = ap_device_handle_dqap(cpu); > + break; > + case PRIV_B2_PQAP: > + rc = ap_device_handle_pqap(cpu); > + break; > + break; one too much > default: > rc = -1; > DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1);
On 03/16/2018 04:03 AM, Pierre Morel wrote: > On 16/03/2018 00:24, Tony Krowiak wrote: >> If the CPU model indicates that AP facility is installed on >> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >> the AP bus running in the guest will initialize; however, if the >> AP instructions are not being interpreted by the firmware, then >> they will be intercepted and routed back to QEMU for handling. >> If a handler is not defined to process the intercepted instruciton, >> then an operation exception will be injected into the >> guest, in which case the AP bus will not initialize. >> >> There are two situations where AP instructions will not be >> interpreted: >> >> 1. The guest is not configured with a vfio-ap device (i.e., >> -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for >> the vfio-ap device enables interpretive execution of AP >> instructions. >> >> 2. The guest is a second level guest but the first level guest has >> not enabled interpretive execution. >> >> This patch introduces AP instruction handlers to ensure the AP bus >> module initializes on the guest when the AP facility is installed >> on the guest but AP instructions are not being interpreted. The logic >> incorporated is: >> >> * If the CPU model indicates AP instructions are >> installed >> >> * Set the status response code for the instruction to indicate that >> the APQN contained in the instruction is not valid. This is >> a valid response because there will be no devices configured for >> the guest in any of the above scenarios. >> >> * Else return an error from the handler. This will result in an >> operation being injected into the guest and the AP bus will not >> initialize on the guest. That is commensurate with how things work >> today. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> >> --- >> hw/vfio/ap.c | 45 >> ++++++++++++++++++++++++++++++++++++++++++ >> include/hw/s390x/ap-device.h | 6 +++++ >> target/s390x/kvm.c | 14 +++++++++++++ >> 3 files changed, 65 insertions(+), 0 deletions(-) >> >> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c >> index b397bb1..88e744d 100644 >> --- a/hw/vfio/ap.c >> +++ b/hw/vfio/ap.c >> @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, >> Error **errp) >> kvm_s390_set_interpret_ap(0); >> } >> >> +int ap_device_handle_nqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_dqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_pqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + int fc = 4 & (env->regs[0] >> 24); >> + >> + /* >> + * The Query Configuration Information (QCI) function (fc == 4) >> does not >> + * set a response code in reg 1, so check for that along with the >> + * AP feature. >> + */ >> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> static Property vfio_ap_properties[] = { >> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >> DEFINE_PROP_END_OF_LIST(), >> diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h >> index 693df90..d45ae38 100644 >> --- a/include/hw/s390x/ap-device.h >> +++ b/include/hw/s390x/ap-device.h >> @@ -11,6 +11,8 @@ >> #ifndef HW_S390X_AP_DEVICE_H >> #define HW_S390X_AP_DEVICE_H >> >> +#include "cpu.h" >> + >> #define AP_DEVICE_TYPE "ap-device" >> >> typedef struct APDevice { >> @@ -35,4 +37,8 @@ static inline APDevice *to_ap_dev(DeviceState *dev) >> #define AP_DEVICE_CLASS(klass) \ >> OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE) >> >> +int ap_device_handle_nqap(S390CPU *cpu); >> +int ap_device_handle_dqap(S390CPU *cpu); >> +int ap_device_handle_pqap(S390CPU *cpu); >> + >> #endif /* HW_S390X_AP_DEVICE_H */ >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index 2812e28..a636394 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -50,6 +50,7 @@ >> #include "exec/memattrs.h" >> #include "hw/s390x/s390-virtio-ccw.h" >> #include "hw/s390x/s390-virtio-hcall.h" >> +#include "hw/s390x/ap-device.h" >> >> #ifndef DEBUG_KVM >> #define DEBUG_KVM 0 >> @@ -88,6 +89,9 @@ >> #define PRIV_B2_CHSC 0x5f >> #define PRIV_B2_SIGA 0x74 >> #define PRIV_B2_XSCH 0x76 >> +#define PRIV_B2_NQAP 0xad >> +#define PRIV_B2_DQAP 0xae >> +#define PRIV_B2_PQAP 0xaf >> >> #define PRIV_EB_SQBS 0x8a >> #define PRIV_EB_PCISTB 0xd0 >> @@ -1245,6 +1249,16 @@ static int handle_b2(S390CPU *cpu, struct >> kvm_run *run, uint8_t ipa1) >> case PRIV_B2_SCLP_CALL: >> rc = kvm_sclp_service_call(cpu, run, ipbh0); >> break; >> + case PRIV_B2_NQAP: >> + rc = ap_device_handle_nqap(cpu); >> + break; >> + case PRIV_B2_DQAP: >> + rc = ap_device_handle_dqap(cpu); >> + break; >> + case PRIV_B2_PQAP: >> + rc = ap_device_handle_pqap(cpu); >> + break; >> + break; > > one too much Yes there is, how on earth did I miss that? I will fix it. > >> default: >> rc = -1; >> DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1); > >
On 16.03.2018 00:24, Tony Krowiak wrote: > If the CPU model indicates that AP facility is installed on > the guest (i.e., -cpu xxxx,ap=on), then the expectation is that > the AP bus running in the guest will initialize; however, if the > AP instructions are not being interpreted by the firmware, then > they will be intercepted and routed back to QEMU for handling. > If a handler is not defined to process the intercepted instruciton, > then an operation exception will be injected into the > guest, in which case the AP bus will not initialize. > > There are two situations where AP instructions will not be > interpreted: > > 1. The guest is not configured with a vfio-ap device (i.e., > -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for > the vfio-ap device enables interpretive execution of AP > instructions. > > 2. The guest is a second level guest but the first level guest has > not enabled interpretive execution. > > This patch introduces AP instruction handlers to ensure the AP bus > module initializes on the guest when the AP facility is installed > on the guest but AP instructions are not being interpreted. The logic > incorporated is: > > * If the CPU model indicates AP instructions are > installed > > * Set the status response code for the instruction to indicate that > the APQN contained in the instruction is not valid. This is > a valid response because there will be no devices configured for > the guest in any of the above scenarios. > > * Else return an error from the handler. This will result in an > operation being injected into the guest and the AP bus will not s/operation/operation exception/ > initialize on the guest. That is commensurate with how things work > today. > > Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> > --- > hw/vfio/ap.c | 45 ++++++++++++++++++++++++++++++++++++++++++ > include/hw/s390x/ap-device.h | 6 +++++ > target/s390x/kvm.c | 14 +++++++++++++ > 3 files changed, 65 insertions(+), 0 deletions(-) > > diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c > index b397bb1..88e744d 100644 > --- a/hw/vfio/ap.c > +++ b/hw/vfio/ap.c > @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error **errp) > kvm_s390_set_interpret_ap(0); > } > I think you are missing cpu_synchronize_state(CPU(cpu)) in all of these functions. > +int ap_device_handle_nqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + > + if (s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > +int ap_device_handle_dqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + > + if (s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > +int ap_device_handle_pqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + int fc = 4 & (env->regs[0] >> 24); > + > + /* > + * The Query Configuration Information (QCI) function (fc == 4) does not > + * set a response code in reg 1, so check for that along with the > + * AP feature. > + */ > + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } This would imply an operation exception in case fc==4, which sounds very wrong. Hard to review without access to documentation. (hard to understand why such stuff is to be kept confidential for decades) > + > + return -EOPNOTSUPP; > +} > + > static Property vfio_ap_properties[] = { > DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), > DEFINE_PROP_END_OF_LIST(), > diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h > index 693df90..d45ae38 100644 > --- a/include/hw/s390x/ap-device.h > +++ b/include/hw/s390x/ap-device.h > @@ -11,6 +11,8 @@ > #ifndef HW_S390X_AP_DEVICE_H > #define HW_S390X_AP_DEVICE_H >
On 26.03.2018 10:32, David Hildenbrand wrote: > On 16.03.2018 00:24, Tony Krowiak wrote: >> If the CPU model indicates that AP facility is installed on >> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >> the AP bus running in the guest will initialize; however, if the >> AP instructions are not being interpreted by the firmware, then >> they will be intercepted and routed back to QEMU for handling. >> If a handler is not defined to process the intercepted instruciton, >> then an operation exception will be injected into the >> guest, in which case the AP bus will not initialize. >> >> There are two situations where AP instructions will not be >> interpreted: >> >> 1. The guest is not configured with a vfio-ap device (i.e., >> -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for >> the vfio-ap device enables interpretive execution of AP >> instructions. >> >> 2. The guest is a second level guest but the first level guest has >> not enabled interpretive execution. >> >> This patch introduces AP instruction handlers to ensure the AP bus >> module initializes on the guest when the AP facility is installed >> on the guest but AP instructions are not being interpreted. The logic >> incorporated is: >> >> * If the CPU model indicates AP instructions are >> installed >> >> * Set the status response code for the instruction to indicate that >> the APQN contained in the instruction is not valid. This is >> a valid response because there will be no devices configured for >> the guest in any of the above scenarios. >> >> * Else return an error from the handler. This will result in an >> operation being injected into the guest and the AP bus will not > > s/operation/operation exception/ > >> initialize on the guest. That is commensurate with how things work >> today. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> >> --- >> hw/vfio/ap.c | 45 ++++++++++++++++++++++++++++++++++++++++++ >> include/hw/s390x/ap-device.h | 6 +++++ >> target/s390x/kvm.c | 14 +++++++++++++ >> 3 files changed, 65 insertions(+), 0 deletions(-) >> >> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c >> index b397bb1..88e744d 100644 >> --- a/hw/vfio/ap.c >> +++ b/hw/vfio/ap.c >> @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error **errp) >> kvm_s390_set_interpret_ap(0); >> } >> > > I think you are missing cpu_synchronize_state(CPU(cpu)) in all of these > functions. > No you don't. There is just a duplicate cpu_synchronize_state(CPU(cpu)) in kvm_sclp_service_call() which confused me.
On 26/03/2018 10:32, David Hildenbrand wrote: > On 16.03.2018 00:24, Tony Krowiak wrote: >> If the CPU model indicates that AP facility is installed on >> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >> the AP bus running in the guest will initialize; however, if the >> AP instructions are not being interpreted by the firmware, then >> they will be intercepted and routed back to QEMU for handling. >> If a handler is not defined to process the intercepted instruciton, >> t ...snip... > >> +int ap_device_handle_nqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_dqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_pqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + int fc = 4 & (env->regs[0] >> 24); >> + >> + /* >> + * The Query Configuration Information (QCI) function (fc == 4) does not >> + * set a response code in reg 1, so check for that along with the >> + * AP feature. >> + */ >> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } > This would imply an operation exception in case fc==4, which sounds very > wrong. It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be tested to know what to answer. If the feature is there, QCI must be answered correctly. there are also some error situations to handle in all three functions. > > Hard to review without access to documentation. (hard to understand why > such stuff is to be kept confidential for decades) > >> + >> + return -EOPNOTSUPP; >> +} >> + >> static Property vfio_ap_properties[] = { >> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >> DEFINE_PROP_END_OF_LIST(), >> diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h >> index 693df90..d45ae38 100644 >> --- a/include/hw/s390x/ap-device.h >> +++ b/include/hw/s390x/ap-device.h >> @@ -11,6 +11,8 @@ >> #ifndef HW_S390X_AP_DEVICE_H >> #define HW_S390X_AP_DEVICE_H >> >
On 03/26/2018 11:03 AM, Pierre Morel wrote: >>> +int ap_device_handle_pqap(S390CPU *cpu) >>> +{ >>> + CPUS390XState *env = &cpu->env; >>> + int fc = 4 & (env->regs[0] >> 24); >>> + >>> + /* >>> + * The Query Configuration Information (QCI) function (fc == 4) does not >>> + * set a response code in reg 1, so check for that along with the >>> + * AP feature. >>> + */ >>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>> + env->regs[1] = 0x10000; >>> + >>> + return 0; >>> + } >> This would imply an operation exception in case fc==4, which sounds very >> wrong. Yes, operation exception is a wrong response under the condition (fc == 4) && s390_has_feat(S390_FEAT_AP). @David: FYI Tony is likely to respond after Wednesday as he is on vacation right now. > > It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be tested > to know what to answer. > If the feature is there, QCI must be answered correctly. > > there are also some error situations to handle in all three functions. > I agree. Regards, Halil
On 03/26/2018 04:32 AM, David Hildenbrand wrote: > On 16.03.2018 00:24, Tony Krowiak wrote: >> If the CPU model indicates that AP facility is installed on >> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >> the AP bus running in the guest will initialize; however, if the >> AP instructions are not being interpreted by the firmware, then >> they will be intercepted and routed back to QEMU for handling. >> If a handler is not defined to process the intercepted instruciton, >> then an operation exception will be injected into the >> guest, in which case the AP bus will not initialize. >> >> There are two situations where AP instructions will not be >> interpreted: >> >> 1. The guest is not configured with a vfio-ap device (i.e., >> -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for >> the vfio-ap device enables interpretive execution of AP >> instructions. >> >> 2. The guest is a second level guest but the first level guest has >> not enabled interpretive execution. >> >> This patch introduces AP instruction handlers to ensure the AP bus >> module initializes on the guest when the AP facility is installed >> on the guest but AP instructions are not being interpreted. The logic >> incorporated is: >> >> * If the CPU model indicates AP instructions are >> installed >> >> * Set the status response code for the instruction to indicate that >> the APQN contained in the instruction is not valid. This is >> a valid response because there will be no devices configured for >> the guest in any of the above scenarios. >> >> * Else return an error from the handler. This will result in an >> operation being injected into the guest and the AP bus will not > s/operation/operation exception/ thanks > >> initialize on the guest. That is commensurate with how things work >> today. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> >> --- >> hw/vfio/ap.c | 45 ++++++++++++++++++++++++++++++++++++++++++ >> include/hw/s390x/ap-device.h | 6 +++++ >> target/s390x/kvm.c | 14 +++++++++++++ >> 3 files changed, 65 insertions(+), 0 deletions(-) >> >> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c >> index b397bb1..88e744d 100644 >> --- a/hw/vfio/ap.c >> +++ b/hw/vfio/ap.c >> @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error **errp) >> kvm_s390_set_interpret_ap(0); >> } >> > I think you are missing cpu_synchronize_state(CPU(cpu)) in all of these > functions. I see you negated this in your next response. > >> +int ap_device_handle_nqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_dqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_pqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + int fc = 4 & (env->regs[0] >> 24); >> + >> + /* >> + * The Query Configuration Information (QCI) function (fc == 4) does not >> + * set a response code in reg 1, so check for that along with the >> + * AP feature. >> + */ >> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } > This would imply an operation exception in case fc==4, which sounds very > wrong. Take a look at my response to Pierre for an answer to this one. > > Hard to review without access to documentation. (hard to understand why > such stuff is to be kept confidential for decades) You'll have to talk to the powers that be at IBM about that one :) > >> + >> + return -EOPNOTSUPP; >> +} >> + >> static Property vfio_ap_properties[] = { >> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >> DEFINE_PROP_END_OF_LIST(), >> diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h >> index 693df90..d45ae38 100644 >> --- a/include/hw/s390x/ap-device.h >> +++ b/include/hw/s390x/ap-device.h >> @@ -11,6 +11,8 @@ >> #ifndef HW_S390X_AP_DEVICE_H >> #define HW_S390X_AP_DEVICE_H >> >
On 03/26/2018 05:03 AM, Pierre Morel wrote: > On 26/03/2018 10:32, David Hildenbrand wrote: >> On 16.03.2018 00:24, Tony Krowiak wrote: >>> If the CPU model indicates that AP facility is installed on >>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>> the AP bus running in the guest will initialize; however, if the >>> AP instructions are not being interpreted by the firmware, then >>> they will be intercepted and routed back to QEMU for handling. >>> If a handler is not defined to process the intercepted instruciton, >>> t > ...snip... >> >>> +int ap_device_handle_nqap(S390CPU *cpu) >>> +{ >>> + CPUS390XState *env = &cpu->env; >>> + >>> + if (s390_has_feat(S390_FEAT_AP)) { >>> + env->regs[1] = 0x10000; >>> + >>> + return 0; >>> + } >>> + >>> + return -EOPNOTSUPP; >>> +} >>> + >>> +int ap_device_handle_dqap(S390CPU *cpu) >>> +{ >>> + CPUS390XState *env = &cpu->env; >>> + >>> + if (s390_has_feat(S390_FEAT_AP)) { >>> + env->regs[1] = 0x10000; >>> + >>> + return 0; >>> + } >>> + >>> + return -EOPNOTSUPP; >>> +} >>> + >>> +int ap_device_handle_pqap(S390CPU *cpu) >>> +{ >>> + CPUS390XState *env = &cpu->env; >>> + int fc = 4 & (env->regs[0] >> 24); >>> + >>> + /* >>> + * The Query Configuration Information (QCI) function (fc == 4) >>> does not >>> + * set a response code in reg 1, so check for that along with the >>> + * AP feature. >>> + */ >>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>> + env->regs[1] = 0x10000; >>> + >>> + return 0; >>> + } >> This would imply an operation exception in case fc==4, which sounds very >> wrong. > > It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be > tested > to know what to answer. > If the feature is there, QCI must be answered correctly. This is an interesting proposition which raises several issues that will need to be addressed. The only time the PQAP(QCI) instruction is intercepted is when: * A vfio-ap device is NOT defined for the guest because the vfio_ap device driver will set ECA.28 and the PQAP(QCI) instruction will be interpreted * STFLE.12 is set for the guest You say that the QCI must be answered correctly, but what is the correct response? If we return the matrix - i.e., APM, ADM and AQM - configured via the mediated matrix device's sysfs attributes files, then if any APQNs are defined in the matrix, we will have to handle *ALL* AP instructions by executing them on behalf of the guest. I suppose we could return an empty matrix in which case the AP bus will come up without any devices on the guest, but what is the expectation of an admin who deliberately configures the mediated matrix device? Should we forego handling interception of AP instructions and consider a guest configuration that turns on S390_FEAT_AP but does not define a vfio-ap device to be erroneous and terminate starting of the guest? Anybody have any thoughts? > > > there are also some error situations to handle in all three functions. To what error situations are you referring? > > > >> >> Hard to review without access to documentation. (hard to understand why >> such stuff is to be kept confidential for decades) >> >>> + >>> + return -EOPNOTSUPP; >>> +} >>> + >>> static Property vfio_ap_properties[] = { >>> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >>> DEFINE_PROP_END_OF_LIST(), >>> diff --git a/include/hw/s390x/ap-device.h >>> b/include/hw/s390x/ap-device.h >>> index 693df90..d45ae38 100644 >>> --- a/include/hw/s390x/ap-device.h >>> +++ b/include/hw/s390x/ap-device.h >>> @@ -11,6 +11,8 @@ >>> #ifndef HW_S390X_AP_DEVICE_H >>> #define HW_S390X_AP_DEVICE_H >> >
On 03/26/2018 08:01 AM, Halil Pasic wrote: > > On 03/26/2018 11:03 AM, Pierre Morel wrote: >>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>> +{ >>>> + CPUS390XState *env = &cpu->env; >>>> + int fc = 4 & (env->regs[0] >> 24); >>>> + >>>> + /* >>>> + * The Query Configuration Information (QCI) function (fc == 4) does not >>>> + * set a response code in reg 1, so check for that along with the >>>> + * AP feature. >>>> + */ >>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>> + env->regs[1] = 0x10000; >>>> + >>>> + return 0; >>>> + } >>> This would imply an operation exception in case fc==4, which sounds very >>> wrong. > Yes, operation exception is a wrong response under the condition > (fc == 4) && s390_has_feat(S390_FEAT_AP). See my response to Pierre: Message ID: <0b957a5c-1a87-7952-292d-f65052bb6c5a@linux.vnet.ibm.com> > > @David: > FYI Tony is likely to respond after Wednesday as he is on vacation right > now. > >> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be tested >> to know what to answer. >> If the feature is there, QCI must be answered correctly. >> >> there are also some error situations to handle in all three functions. >> > I agree. > > Regards, > Halil
On Mon, 2 Apr 2018 12:36:27 -0400 Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: > On 03/26/2018 05:03 AM, Pierre Morel wrote: > > On 26/03/2018 10:32, David Hildenbrand wrote: > >> On 16.03.2018 00:24, Tony Krowiak wrote: > >>> + /* > >>> + * The Query Configuration Information (QCI) function (fc == 4) > >>> does not > >>> + * set a response code in reg 1, so check for that along with the > >>> + * AP feature. > >>> + */ > >>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { > >>> + env->regs[1] = 0x10000; > >>> + > >>> + return 0; > >>> + } > >> This would imply an operation exception in case fc==4, which sounds very > >> wrong. > > > > It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be > > tested > > to know what to answer. > > If the feature is there, QCI must be answered correctly. > This is an interesting proposition which raises several issues that will > need to > be addressed. The only time the PQAP(QCI) instruction is intercepted is > when: > * A vfio-ap device is NOT defined for the guest because the vfio_ap > device driver > will set ECA.28 and the PQAP(QCI) instruction will be interpreted > * STFLE.12 is set for the guest > > You say that the QCI must be answered correctly, but what is the correct > response? > If we return the matrix - i.e., APM, ADM and AQM - configured via the > mediated > matrix device's sysfs attributes files, then if any APQNs are defined in > the matrix, > we will have to handle *ALL* AP instructions by executing them on behalf > of the > guest. I suppose we could return an empty matrix in which case the AP > bus will come > up without any devices on the guest, but what is the expectation of an > admin who > deliberately configures the mediated matrix device? Should we forego > handling interception > of AP instructions and consider a guest configuration that turns on > S390_FEAT_AP but > does not define a vfio-ap device to be erroneous and terminate starting > of the guest? > Anybody have any thoughts? Hard to really give good advice without access to the documentation, but: - If we tell the guest that the feature is available, but it does not get any cards to use, returning an empty matrix makes the most sense to me. - I would not tie starting the guest to the presence of a vfio-ap device. Having the feature available in theory but without any devices actually being usable by the guest does not really sound wrong (can we hotplug this later?)
On 03/04/2018 11:36, Cornelia Huck wrote: > On Mon, 2 Apr 2018 12:36:27 -0400 > Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: > >> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>> + /* >>>>> + * The Query Configuration Information (QCI) function (fc == 4) >>>>> does not >>>>> + * set a response code in reg 1, so check for that along with the >>>>> + * AP feature. >>>>> + */ >>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>> This would imply an operation exception in case fc==4, which sounds very >>>> wrong. >>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be >>> tested >>> to know what to answer. >>> If the feature is there, QCI must be answered correctly. >> This is an interesting proposition which raises several issues that will >> need to >> be addressed. The only time the PQAP(QCI) instruction is intercepted is >> when: >> * A vfio-ap device is NOT defined for the guest because the vfio_ap >> device driver >> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >> * STFLE.12 is set for the guest >> >> You say that the QCI must be answered correctly, but what is the correct >> response? >> If we return the matrix - i.e., APM, ADM and AQM - configured via the >> mediated >> matrix device's sysfs attributes files, then if any APQNs are defined in >> the matrix, >> we will have to handle *ALL* AP instructions by executing them on behalf >> of the >> guest. I suppose we could return an empty matrix in which case the AP >> bus will come >> up without any devices on the guest, but what is the expectation of an >> admin who >> deliberately configures the mediated matrix device? Should we forego >> handling interception >> of AP instructions and consider a guest configuration that turns on >> S390_FEAT_AP but >> does not define a vfio-ap device to be erroneous and terminate starting >> of the guest? >> Anybody have any thoughts? > Hard to really give good advice without access to the documentation, but: > - If we tell the guest that the feature is available, but it does not > get any cards to use, returning an empty matrix makes the most sense > to me. +1 > - I would not tie starting the guest to the presence of a vfio-ap > device. Having the feature available in theory but without any > devices actually being usable by the guest does not really sound > wrong (can we hotplug this later?) > +1
On 02/04/2018 18:36, Tony Krowiak wrote: > On 03/26/2018 05:03 AM, Pierre Morel wrote: >> On 26/03/2018 10:32, David Hildenbrand wrote: >>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>> If the CPU model indicates that AP facility is installed on >>>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>>> the AP bus running in the guest will initialize; however, if the >>>> AP instructions are not being interpreted by the firmware, then >>>> they will be intercepted and routed back to QEMU for handling. >>>> If a handler is not defined to process the intercepted instruciton, >>>> t >> ...snip... >>> >>>> +int ap_device_handle_nqap(S390CPU *cpu) >>>> +{ >>>> + CPUS390XState *env = &cpu->env; >>>> + >>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>> + env->regs[1] = 0x10000; >>>> + >>>> + return 0; >>>> + } >>>> + >>>> + return -EOPNOTSUPP; >>>> +} >>>> + >>>> +int ap_device_handle_dqap(S390CPU *cpu) >>>> +{ >>>> + CPUS390XState *env = &cpu->env; >>>> + >>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>> + env->regs[1] = 0x10000; >>>> + >>>> + return 0; >>>> + } >>>> + >>>> + return -EOPNOTSUPP; >>>> +} >>>> + >>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>> +{ >>>> + CPUS390XState *env = &cpu->env; >>>> + int fc = 4 & (env->regs[0] >> 24); >>>> + >>>> + /* >>>> + * The Query Configuration Information (QCI) function (fc == >>>> 4) does not >>>> + * set a response code in reg 1, so check for that along with the >>>> + * AP feature. >>>> + */ >>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>> + env->regs[1] = 0x10000; >>>> + >>>> + return 0; >>>> + } >>> This would imply an operation exception in case fc==4, which sounds >>> very >>> wrong. >> >> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must >> be tested >> to know what to answer. >> If the feature is there, QCI must be answered correctly. > This is an interesting proposition which raises several issues that > will need to > be addressed. The only time the PQAP(QCI) instruction is intercepted > is when: > * A vfio-ap device is NOT defined for the guest because the vfio_ap > device driver > will set ECA.28 and the PQAP(QCI) instruction will be interpreted > * STFLE.12 is set for the guest > > You say that the QCI must be answered correctly, but what is the > correct response? > If we return the matrix - i.e., APM, ADM and AQM - configured via the > mediated > matrix device's sysfs attributes files, then if any APQNs are defined > in the matrix, > we will have to handle *ALL* AP instructions by executing them on > behalf of the > guest. I suppose we could return an empty matrix in which case the AP > bus will come > up without any devices on the guest, but what is the expectation of an > admin who > deliberately configures the mediated matrix device? Should we forego > handling interception > of AP instructions and consider a guest configuration that turns on > S390_FEAT_AP but > does not define a vfio-ap device to be erroneous and terminate > starting of the guest? > Anybody have any thoughts? >> >> >> there are also some error situations to handle in all three functions. > To what error situations are you referring? program exceptions, like access, privilege or specification exceptions. >> >> >> >>> >>> Hard to review without access to documentation. (hard to understand why >>> such stuff is to be kept confidential for decades) >>> >>>> + >>>> + return -EOPNOTSUPP; >>>> +} >>>> + >>>> static Property vfio_ap_properties[] = { >>>> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >>>> DEFINE_PROP_END_OF_LIST(), >>>> diff --git a/include/hw/s390x/ap-device.h >>>> b/include/hw/s390x/ap-device.h >>>> index 693df90..d45ae38 100644 >>>> --- a/include/hw/s390x/ap-device.h >>>> +++ b/include/hw/s390x/ap-device.h >>>> @@ -11,6 +11,8 @@ >>>> #ifndef HW_S390X_AP_DEVICE_H >>>> #define HW_S390X_AP_DEVICE_H >>> >> > >
On 04/04/2018 07:09 AM, Pierre Morel wrote: > On 02/04/2018 18:36, Tony Krowiak wrote: >> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>> If the CPU model indicates that AP facility is installed on >>>>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>>>> the AP bus running in the guest will initialize; however, if the >>>>> AP instructions are not being interpreted by the firmware, then >>>>> they will be intercepted and routed back to QEMU for handling. >>>>> If a handler is not defined to process the intercepted instruciton, >>>>> t >>> ...snip... >>>> >>>>> +int ap_device_handle_nqap(S390CPU *cpu) >>>>> +{ >>>>> + CPUS390XState *env = &cpu->env; >>>>> + >>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>>> + >>>>> + return -EOPNOTSUPP; >>>>> +} >>>>> + >>>>> +int ap_device_handle_dqap(S390CPU *cpu) >>>>> +{ >>>>> + CPUS390XState *env = &cpu->env; >>>>> + >>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>>> + >>>>> + return -EOPNOTSUPP; >>>>> +} >>>>> + >>>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>>> +{ >>>>> + CPUS390XState *env = &cpu->env; >>>>> + int fc = 4 & (env->regs[0] >> 24); >>>>> + >>>>> + /* >>>>> + * The Query Configuration Information (QCI) function (fc == >>>>> 4) does not >>>>> + * set a response code in reg 1, so check for that along with >>>>> the >>>>> + * AP feature. >>>>> + */ >>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>> This would imply an operation exception in case fc==4, which sounds >>>> very >>>> wrong. >>> >>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must >>> be tested >>> to know what to answer. >>> If the feature is there, QCI must be answered correctly. >> This is an interesting proposition which raises several issues that >> will need to >> be addressed. The only time the PQAP(QCI) instruction is intercepted >> is when: >> * A vfio-ap device is NOT defined for the guest because the vfio_ap >> device driver >> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >> * STFLE.12 is set for the guest >> >> You say that the QCI must be answered correctly, but what is the >> correct response? >> If we return the matrix - i.e., APM, ADM and AQM - configured via the >> mediated >> matrix device's sysfs attributes files, then if any APQNs are defined >> in the matrix, >> we will have to handle *ALL* AP instructions by executing them on >> behalf of the >> guest. I suppose we could return an empty matrix in which case the AP >> bus will come >> up without any devices on the guest, but what is the expectation of >> an admin who >> deliberately configures the mediated matrix device? Should we forego >> handling interception >> of AP instructions and consider a guest configuration that turns on >> S390_FEAT_AP but >> does not define a vfio-ap device to be erroneous and terminate >> starting of the guest? >> Anybody have any thoughts? >>> >>> >>> there are also some error situations to handle in all three functions. >> To what error situations are you referring? > > program exceptions, like access, privilege or specification exceptions. I'm sorry, I still don't know what you are referring to, can you be more specific? > > > >>> >>> >>> >>>> >>>> Hard to review without access to documentation. (hard to understand >>>> why >>>> such stuff is to be kept confidential for decades) >>>> >>>>> + >>>>> + return -EOPNOTSUPP; >>>>> +} >>>>> + >>>>> static Property vfio_ap_properties[] = { >>>>> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >>>>> DEFINE_PROP_END_OF_LIST(), >>>>> diff --git a/include/hw/s390x/ap-device.h >>>>> b/include/hw/s390x/ap-device.h >>>>> index 693df90..d45ae38 100644 >>>>> --- a/include/hw/s390x/ap-device.h >>>>> +++ b/include/hw/s390x/ap-device.h >>>>> @@ -11,6 +11,8 @@ >>>>> #ifndef HW_S390X_AP_DEVICE_H >>>>> #define HW_S390X_AP_DEVICE_H >>>> >>> >> >> >
On 04/04/2018 07:09 AM, Pierre Morel wrote: > On 02/04/2018 18:36, Tony Krowiak wrote: >> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>> If the CPU model indicates that AP facility is installed on >>>>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>>>> the AP bus running in the guest will initialize; however, if the >>>>> AP instructions are not being interpreted by the firmware, then >>>>> they will be intercepted and routed back to QEMU for handling. >>>>> If a handler is not defined to process the intercepted instruciton, >>>>> t >>> ...snip... >>>> >>>>> +int ap_device_handle_nqap(S390CPU *cpu) >>>>> +{ >>>>> + CPUS390XState *env = &cpu->env; >>>>> + >>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>>> + >>>>> + return -EOPNOTSUPP; >>>>> +} >>>>> + >>>>> +int ap_device_handle_dqap(S390CPU *cpu) >>>>> +{ >>>>> + CPUS390XState *env = &cpu->env; >>>>> + >>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>>> + >>>>> + return -EOPNOTSUPP; >>>>> +} >>>>> + >>>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>>> +{ >>>>> + CPUS390XState *env = &cpu->env; >>>>> + int fc = 4 & (env->regs[0] >> 24); >>>>> + >>>>> + /* >>>>> + * The Query Configuration Information (QCI) function (fc == >>>>> 4) does not >>>>> + * set a response code in reg 1, so check for that along with >>>>> the >>>>> + * AP feature. >>>>> + */ >>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>> This would imply an operation exception in case fc==4, which sounds >>>> very >>>> wrong. >>> >>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must >>> be tested >>> to know what to answer. >>> If the feature is there, QCI must be answered correctly. >> This is an interesting proposition which raises several issues that >> will need to >> be addressed. The only time the PQAP(QCI) instruction is intercepted >> is when: >> * A vfio-ap device is NOT defined for the guest because the vfio_ap >> device driver >> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >> * STFLE.12 is set for the guest >> >> You say that the QCI must be answered correctly, but what is the >> correct response? >> If we return the matrix - i.e., APM, ADM and AQM - configured via the >> mediated >> matrix device's sysfs attributes files, then if any APQNs are defined >> in the matrix, >> we will have to handle *ALL* AP instructions by executing them on >> behalf of the >> guest. I suppose we could return an empty matrix in which case the AP >> bus will come >> up without any devices on the guest, but what is the expectation of >> an admin who >> deliberately configures the mediated matrix device? Should we forego >> handling interception >> of AP instructions and consider a guest configuration that turns on >> S390_FEAT_AP but >> does not define a vfio-ap device to be erroneous and terminate >> starting of the guest? >> Anybody have any thoughts? >>> >>> >>> there are also some error situations to handle in all three functions. >> To what error situations are you referring? > > program exceptions, like access, privilege or specification exceptions. Are you suggesting that these handlers need to verify that the instruction specification is correct? For example, the NQAP instruction - NQAP r1,r2 - expects an even-odd pair of general registers in the r1 and r2 and must designate an even register; otherwise a specification exception is recognized. Are you saying that the handler for NQAP must verify this requirement and inject a specification exception if it is not met? If not, then can you provide a specific example of what you are talking about? > > > >>> >>> >>> >>>> >>>> Hard to review without access to documentation. (hard to understand >>>> why >>>> such stuff is to be kept confidential for decades) >>>> >>>>> + >>>>> + return -EOPNOTSUPP; >>>>> +} >>>>> + >>>>> static Property vfio_ap_properties[] = { >>>>> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >>>>> DEFINE_PROP_END_OF_LIST(), >>>>> diff --git a/include/hw/s390x/ap-device.h >>>>> b/include/hw/s390x/ap-device.h >>>>> index 693df90..d45ae38 100644 >>>>> --- a/include/hw/s390x/ap-device.h >>>>> +++ b/include/hw/s390x/ap-device.h >>>>> @@ -11,6 +11,8 @@ >>>>> #ifndef HW_S390X_AP_DEVICE_H >>>>> #define HW_S390X_AP_DEVICE_H >>>> >>> >> >> >
On 04/04/2018 14:59, Tony Krowiak wrote: > On 04/04/2018 07:09 AM, Pierre Morel wrote: >> On 02/04/2018 18:36, Tony Krowiak wrote: >>> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>>> If the CPU model indicates that AP facility is installed on >>>>>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>>>>> the AP bus running in the guest will initialize; however, if the >>>>>> AP instructions are not being interpreted by the firmware, then >>>>>> they will be intercepted and routed back to QEMU for handling. >>>>>> If a handler is not defined to process the intercepted instruciton, >>>>>> t >>>> ...snip... >>>>> >>>>>> +int ap_device_handle_nqap(S390CPU *cpu) >>>>>> +{ >>>>>> + CPUS390XState *env = &cpu->env; >>>>>> + >>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>> + env->regs[1] = 0x10000; >>>>>> + >>>>>> + return 0; >>>>>> + } >>>>>> + >>>>>> + return -EOPNOTSUPP; >>>>>> +} >>>>>> + >>>>>> +int ap_device_handle_dqap(S390CPU *cpu) >>>>>> +{ >>>>>> + CPUS390XState *env = &cpu->env; >>>>>> + >>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>> + env->regs[1] = 0x10000; >>>>>> + >>>>>> + return 0; >>>>>> + } >>>>>> + >>>>>> + return -EOPNOTSUPP; >>>>>> +} >>>>>> + >>>>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>>>> +{ >>>>>> + CPUS390XState *env = &cpu->env; >>>>>> + int fc = 4 & (env->regs[0] >> 24); >>>>>> + >>>>>> + /* >>>>>> + * The Query Configuration Information (QCI) function (fc == >>>>>> 4) does not >>>>>> + * set a response code in reg 1, so check for that along >>>>>> with the >>>>>> + * AP feature. >>>>>> + */ >>>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>>> + env->regs[1] = 0x10000; >>>>>> + >>>>>> + return 0; >>>>>> + } >>>>> This would imply an operation exception in case fc==4, which >>>>> sounds very >>>>> wrong. >>>> >>>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must >>>> be tested >>>> to know what to answer. >>>> If the feature is there, QCI must be answered correctly. >>> This is an interesting proposition which raises several issues that >>> will need to >>> be addressed. The only time the PQAP(QCI) instruction is intercepted >>> is when: >>> * A vfio-ap device is NOT defined for the guest because the vfio_ap >>> device driver >>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >>> * STFLE.12 is set for the guest >>> >>> You say that the QCI must be answered correctly, but what is the >>> correct response? >>> If we return the matrix - i.e., APM, ADM and AQM - configured via >>> the mediated >>> matrix device's sysfs attributes files, then if any APQNs are >>> defined in the matrix, >>> we will have to handle *ALL* AP instructions by executing them on >>> behalf of the >>> guest. I suppose we could return an empty matrix in which case the >>> AP bus will come >>> up without any devices on the guest, but what is the expectation of >>> an admin who >>> deliberately configures the mediated matrix device? Should we forego >>> handling interception >>> of AP instructions and consider a guest configuration that turns on >>> S390_FEAT_AP but >>> does not define a vfio-ap device to be erroneous and terminate >>> starting of the guest? >>> Anybody have any thoughts? >>>> >>>> >>>> there are also some error situations to handle in all three functions. >>> To what error situations are you referring? >> >> program exceptions, like access, privilege or specification exceptions. > I'm sorry, I still don't know what you are referring to, can you be > more specific? You do not handle the program exceptions. The xQAP are priviledged instruction, you must test that it is called from the kernel with if (env->psw.mask & PSW_MASK_PSTATE) { s390_program_interrupt(env, PGM_PRIVILEGED, 4, ra); return 0; } Specification exceptions are recognized if the R1 or R2 specifies a odd number register ...etc. more in the documentation. Since to know the queue you are working on you need the content of the registers you must handle the exceptions. >> >> >> >>>> >>>> >>>> >>>>> >>>>> Hard to review without access to documentation. (hard to >>>>> understand why >>>>> such stuff is to be kept confidential for decades) >>>>> >>>>>> + >>>>>> + return -EOPNOTSUPP; >>>>>> +} >>>>>> + >>>>>> static Property vfio_ap_properties[] = { >>>>>> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >>>>>> DEFINE_PROP_END_OF_LIST(), >>>>>> diff --git a/include/hw/s390x/ap-device.h >>>>>> b/include/hw/s390x/ap-device.h >>>>>> index 693df90..d45ae38 100644 >>>>>> --- a/include/hw/s390x/ap-device.h >>>>>> +++ b/include/hw/s390x/ap-device.h >>>>>> @@ -11,6 +11,8 @@ >>>>>> #ifndef HW_S390X_AP_DEVICE_H >>>>>> #define HW_S390X_AP_DEVICE_H >>>>> >>>> >>> >>> >> >
On 04/03/2018 05:36 AM, Cornelia Huck wrote: > On Mon, 2 Apr 2018 12:36:27 -0400 > Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: > >> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>> + /* >>>>> + * The Query Configuration Information (QCI) function (fc == 4) >>>>> does not >>>>> + * set a response code in reg 1, so check for that along with the >>>>> + * AP feature. >>>>> + */ >>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>> This would imply an operation exception in case fc==4, which sounds very >>>> wrong. >>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be >>> tested >>> to know what to answer. >>> If the feature is there, QCI must be answered correctly. >> This is an interesting proposition which raises several issues that will >> need to >> be addressed. The only time the PQAP(QCI) instruction is intercepted is >> when: >> * A vfio-ap device is NOT defined for the guest because the vfio_ap >> device driver >> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >> * STFLE.12 is set for the guest >> >> You say that the QCI must be answered correctly, but what is the correct >> response? >> If we return the matrix - i.e., APM, ADM and AQM - configured via the >> mediated >> matrix device's sysfs attributes files, then if any APQNs are defined in >> the matrix, >> we will have to handle *ALL* AP instructions by executing them on behalf >> of the >> guest. I suppose we could return an empty matrix in which case the AP >> bus will come >> up without any devices on the guest, but what is the expectation of an >> admin who >> deliberately configures the mediated matrix device? Should we forego >> handling interception >> of AP instructions and consider a guest configuration that turns on >> S390_FEAT_AP but >> does not define a vfio-ap device to be erroneous and terminate starting >> of the guest? >> Anybody have any thoughts? > Hard to really give good advice without access to the documentation, but: > - If we tell the guest that the feature is available, but it does not > get any cards to use, returning an empty matrix makes the most sense > to me. After some thought, I am inclined to agree with this. If the no vfio-ap device is defined for the guest, then anything configured for the mediated matrix device is not relevant and the guest would therefore have no AP devices. > - I would not tie starting the guest to the presence of a vfio-ap > device. Having the feature available in theory but without any > devices actually being usable by the guest does not really sound > wrong (can we hotplug this later?) >
On 04/04/2018 15:33, Tony Krowiak wrote: > On 04/04/2018 07:09 AM, Pierre Morel wrote: >> On 02/04/2018 18:36, Tony Krowiak wrote: >>> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>>> If the CPU model indicates that AP facility is installed on >>>>>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>>>>> the AP bus running in the guest will initialize; however, if the >>>>>> AP instructions are not being interpreted by the firmware, then >>>>>> they will be intercepted and routed back to QEMU for handling. >>>>>> If a handler is not defined to process the intercepted instruciton, >>>>>> t >>>> ...snip... >>>>> >>>>>> +int ap_device_handle_nqap(S390CPU *cpu) >>>>>> +{ >>>>>> + CPUS390XState *env = &cpu->env; >>>>>> + >>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>> + env->regs[1] = 0x10000; >>>>>> + >>>>>> + return 0; >>>>>> + } >>>>>> + >>>>>> + return -EOPNOTSUPP; >>>>>> +} >>>>>> + >>>>>> +int ap_device_handle_dqap(S390CPU *cpu) >>>>>> +{ >>>>>> + CPUS390XState *env = &cpu->env; >>>>>> + >>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>> + env->regs[1] = 0x10000; >>>>>> + >>>>>> + return 0; >>>>>> + } >>>>>> + >>>>>> + return -EOPNOTSUPP; >>>>>> +} >>>>>> + >>>>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>>>> +{ >>>>>> + CPUS390XState *env = &cpu->env; >>>>>> + int fc = 4 & (env->regs[0] >> 24); >>>>>> + >>>>>> + /* >>>>>> + * The Query Configuration Information (QCI) function (fc == >>>>>> 4) does not >>>>>> + * set a response code in reg 1, so check for that along >>>>>> with the >>>>>> + * AP feature. >>>>>> + */ >>>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>>> + env->regs[1] = 0x10000; >>>>>> + >>>>>> + return 0; >>>>>> + } >>>>> This would imply an operation exception in case fc==4, which >>>>> sounds very >>>>> wrong. >>>> >>>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must >>>> be tested >>>> to know what to answer. >>>> If the feature is there, QCI must be answered correctly. >>> This is an interesting proposition which raises several issues that >>> will need to >>> be addressed. The only time the PQAP(QCI) instruction is intercepted >>> is when: >>> * A vfio-ap device is NOT defined for the guest because the vfio_ap >>> device driver >>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >>> * STFLE.12 is set for the guest >>> >>> You say that the QCI must be answered correctly, but what is the >>> correct response? >>> If we return the matrix - i.e., APM, ADM and AQM - configured via >>> the mediated >>> matrix device's sysfs attributes files, then if any APQNs are >>> defined in the matrix, >>> we will have to handle *ALL* AP instructions by executing them on >>> behalf of the >>> guest. I suppose we could return an empty matrix in which case the >>> AP bus will come >>> up without any devices on the guest, but what is the expectation of >>> an admin who >>> deliberately configures the mediated matrix device? Should we forego >>> handling interception >>> of AP instructions and consider a guest configuration that turns on >>> S390_FEAT_AP but >>> does not define a vfio-ap device to be erroneous and terminate >>> starting of the guest? >>> Anybody have any thoughts? >>>> >>>> >>>> there are also some error situations to handle in all three functions. >>> To what error situations are you referring? >> >> program exceptions, like access, privilege or specification exceptions. > Are you suggesting that these handlers need to verify that the > instruction > specification is correct? For example, the NQAP instruction - NQAP > r1,r2 - > expects an even-odd pair of general registers in the r1 and r2 and must > designate an even register; otherwise a specification exception is > recognized. Are you saying that the handler for NQAP must verify this > requirement and inject a specification exception if it is not met? If > not, > then can you provide a specific example of what you are talking about? Yes I mean this. But one other thing. Returning code 0x01 seems wrong for me, this is right if no AP card are in the system but I do not thing that it is what we mean. I think we should return code 0x03 stating that the AP card is not in configured state The difference is that we can get out of a not-configured state with a SCLP command but we can not recover from an un-existing APQN. >> >> >> >>>> >>>> >>>> >>>>> >>>>> Hard to review without access to documentation. (hard to >>>>> understand why >>>>> such stuff is to be kept confidential for decades) >>>>> >>>>>> + >>>>>> + return -EOPNOTSUPP; >>>>>> +} >>>>>> + >>>>>> static Property vfio_ap_properties[] = { >>>>>> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >>>>>> DEFINE_PROP_END_OF_LIST(), >>>>>> diff --git a/include/hw/s390x/ap-device.h >>>>>> b/include/hw/s390x/ap-device.h >>>>>> index 693df90..d45ae38 100644 >>>>>> --- a/include/hw/s390x/ap-device.h >>>>>> +++ b/include/hw/s390x/ap-device.h >>>>>> @@ -11,6 +11,8 @@ >>>>>> #ifndef HW_S390X_AP_DEVICE_H >>>>>> #define HW_S390X_AP_DEVICE_H >>>>> >>>> >>> >>> >> >
On 04/04/2018 09:43 AM, Pierre Morel wrote: > On 04/04/2018 15:33, Tony Krowiak wrote: >> On 04/04/2018 07:09 AM, Pierre Morel wrote: >>> On 02/04/2018 18:36, Tony Krowiak wrote: >>>> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>>>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>>>> If the CPU model indicates that AP facility is installed on >>>>>>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>>>>>> the AP bus running in the guest will initialize; however, if the >>>>>>> AP instructions are not being interpreted by the firmware, then >>>>>>> they will be intercepted and routed back to QEMU for handling. >>>>>>> If a handler is not defined to process the intercepted instruciton, >>>>>>> t >>>>> ...snip... >>>>>> >>>>>>> +int ap_device_handle_nqap(S390CPU *cpu) >>>>>>> +{ >>>>>>> + CPUS390XState *env = &cpu->env; >>>>>>> + >>>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>>> + env->regs[1] = 0x10000; >>>>>>> + >>>>>>> + return 0; >>>>>>> + } >>>>>>> + >>>>>>> + return -EOPNOTSUPP; >>>>>>> +} >>>>>>> + >>>>>>> +int ap_device_handle_dqap(S390CPU *cpu) >>>>>>> +{ >>>>>>> + CPUS390XState *env = &cpu->env; >>>>>>> + >>>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>>> + env->regs[1] = 0x10000; >>>>>>> + >>>>>>> + return 0; >>>>>>> + } >>>>>>> + >>>>>>> + return -EOPNOTSUPP; >>>>>>> +} >>>>>>> + >>>>>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>>>>> +{ >>>>>>> + CPUS390XState *env = &cpu->env; >>>>>>> + int fc = 4 & (env->regs[0] >> 24); >>>>>>> + >>>>>>> + /* >>>>>>> + * The Query Configuration Information (QCI) function (fc >>>>>>> == 4) does not >>>>>>> + * set a response code in reg 1, so check for that along >>>>>>> with the >>>>>>> + * AP feature. >>>>>>> + */ >>>>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>>>> + env->regs[1] = 0x10000; >>>>>>> + >>>>>>> + return 0; >>>>>>> + } >>>>>> This would imply an operation exception in case fc==4, which >>>>>> sounds very >>>>>> wrong. >>>>> >>>>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO >>>>> must be tested >>>>> to know what to answer. >>>>> If the feature is there, QCI must be answered correctly. >>>> This is an interesting proposition which raises several issues that >>>> will need to >>>> be addressed. The only time the PQAP(QCI) instruction is >>>> intercepted is when: >>>> * A vfio-ap device is NOT defined for the guest because the vfio_ap >>>> device driver >>>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >>>> * STFLE.12 is set for the guest >>>> >>>> You say that the QCI must be answered correctly, but what is the >>>> correct response? >>>> If we return the matrix - i.e., APM, ADM and AQM - configured via >>>> the mediated >>>> matrix device's sysfs attributes files, then if any APQNs are >>>> defined in the matrix, >>>> we will have to handle *ALL* AP instructions by executing them on >>>> behalf of the >>>> guest. I suppose we could return an empty matrix in which case the >>>> AP bus will come >>>> up without any devices on the guest, but what is the expectation of >>>> an admin who >>>> deliberately configures the mediated matrix device? Should we >>>> forego handling interception >>>> of AP instructions and consider a guest configuration that turns on >>>> S390_FEAT_AP but >>>> does not define a vfio-ap device to be erroneous and terminate >>>> starting of the guest? >>>> Anybody have any thoughts? >>>>> >>>>> >>>>> there are also some error situations to handle in all three >>>>> functions. >>>> To what error situations are you referring? >>> >>> program exceptions, like access, privilege or specification exceptions. >> Are you suggesting that these handlers need to verify that the >> instruction >> specification is correct? For example, the NQAP instruction - NQAP >> r1,r2 - >> expects an even-odd pair of general registers in the r1 and r2 and must >> designate an even register; otherwise a specification exception is >> recognized. Are you saying that the handler for NQAP must verify this >> requirement and inject a specification exception if it is not met? If >> not, >> then can you provide a specific example of what you are talking about? > > Yes I mean this. > > But one other thing. > Returning code 0x01 seems wrong for me, this is right if no AP card > are in the system but I do not thing that it is what we mean. I disagree .... for the guest, this is exactly what we mean. In my opinion, assigning AP devices to the mediated matrix device is like assigning AP devices to an LPAR. If no vfio-ap device is defined for the guest, then it is effectively the same as starting a linux host in an LPAR without any AP devices assigned to it. > > I think we should return code 0x03 stating that the AP card is not in > configured state > > > The difference is that we can get out of a not-configured state with a > SCLP command but > we can not recover from an un-existing APQN. Remember, this state will occur only when the AP feature of the CPU model is turned on for the guest and no vfio-ap device is defined, so there is no configuration to recover for the guest because no AP devices will be configured for the guest. > > > >>> >>> >>> >>>>> >>>>> >>>>> >>>>>> >>>>>> Hard to review without access to documentation. (hard to >>>>>> understand why >>>>>> such stuff is to be kept confidential for decades) >>>>>> >>>>>>> + >>>>>>> + return -EOPNOTSUPP; >>>>>>> +} >>>>>>> + >>>>>>> static Property vfio_ap_properties[] = { >>>>>>> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >>>>>>> DEFINE_PROP_END_OF_LIST(), >>>>>>> diff --git a/include/hw/s390x/ap-device.h >>>>>>> b/include/hw/s390x/ap-device.h >>>>>>> index 693df90..d45ae38 100644 >>>>>>> --- a/include/hw/s390x/ap-device.h >>>>>>> +++ b/include/hw/s390x/ap-device.h >>>>>>> @@ -11,6 +11,8 @@ >>>>>>> #ifndef HW_S390X_AP_DEVICE_H >>>>>>> #define HW_S390X_AP_DEVICE_H >>>>>> >>>>> >>>> >>>> >>> >> >
On 04/04/2018 10:12 PM, Tony Krowiak wrote: > On 04/04/2018 09:43 AM, Pierre Morel wrote: >> On 04/04/2018 15:33, Tony Krowiak wrote: >>> On 04/04/2018 07:09 AM, Pierre Morel wrote: >>>> On 02/04/2018 18:36, Tony Krowiak wrote: >>>>> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>>>>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>>>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>>>>> If the CPU model indicates that AP facility is installed on >>>>>>>> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >>>>>>>> the AP bus running in the guest will initialize; however, if the >>>>>>>> AP instructions are not being interpreted by the firmware, then >>>>>>>> they will be intercepted and routed back to QEMU for handling. >>>>>>>> If a handler is not defined to process the intercepted instruciton, >>>>>>>> t >>>>>> ...snip... >>>>>>> >>>>>>>> +int ap_device_handle_nqap(S390CPU *cpu) >>>>>>>> +{ >>>>>>>> + CPUS390XState *env = &cpu->env; >>>>>>>> + >>>>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>>>> + env->regs[1] = 0x10000; >>>>>>>> + >>>>>>>> + return 0; >>>>>>>> + } >>>>>>>> + >>>>>>>> + return -EOPNOTSUPP; >>>>>>>> +} >>>>>>>> + >>>>>>>> +int ap_device_handle_dqap(S390CPU *cpu) >>>>>>>> +{ >>>>>>>> + CPUS390XState *env = &cpu->env; >>>>>>>> + >>>>>>>> + if (s390_has_feat(S390_FEAT_AP)) { >>>>>>>> + env->regs[1] = 0x10000; >>>>>>>> + >>>>>>>> + return 0; >>>>>>>> + } >>>>>>>> + >>>>>>>> + return -EOPNOTSUPP; >>>>>>>> +} >>>>>>>> + >>>>>>>> +int ap_device_handle_pqap(S390CPU *cpu) >>>>>>>> +{ >>>>>>>> + CPUS390XState *env = &cpu->env; >>>>>>>> + int fc = 4 & (env->regs[0] >> 24); >>>>>>>> + >>>>>>>> + /* >>>>>>>> + * The Query Configuration Information (QCI) function (fc == 4) does not >>>>>>>> + * set a response code in reg 1, so check for that along with the >>>>>>>> + * AP feature. >>>>>>>> + */ >>>>>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>>>>> + env->regs[1] = 0x10000; >>>>>>>> + >>>>>>>> + return 0; >>>>>>>> + } >>>>>>> This would imply an operation exception in case fc==4, which sounds very >>>>>>> wrong. >>>>>> >>>>>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be tested >>>>>> to know what to answer. >>>>>> If the feature is there, QCI must be answered correctly. >>>>> This is an interesting proposition which raises several issues that will need to >>>>> be addressed. The only time the PQAP(QCI) instruction is intercepted is when: >>>>> * A vfio-ap device is NOT defined for the guest because the vfio_ap device driver >>>>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >>>>> * STFLE.12 is set for the guest >>>>> >>>>> You say that the QCI must be answered correctly, but what is the correct response? >>>>> If we return the matrix - i.e., APM, ADM and AQM - configured via the mediated >>>>> matrix device's sysfs attributes files, then if any APQNs are defined in the matrix, >>>>> we will have to handle *ALL* AP instructions by executing them on behalf of the >>>>> guest. I suppose we could return an empty matrix in which case the AP bus will come >>>>> up without any devices on the guest, but what is the expectation of an admin who >>>>> deliberately configures the mediated matrix device? Should we forego handling interception >>>>> of AP instructions and consider a guest configuration that turns on S390_FEAT_AP but >>>>> does not define a vfio-ap device to be erroneous and terminate starting of the guest? >>>>> Anybody have any thoughts? >>>>>> >>>>>> >>>>>> there are also some error situations to handle in all three functions. >>>>> To what error situations are you referring? >>>> >>>> program exceptions, like access, privilege or specification exceptions. >>> Are you suggesting that these handlers need to verify that the instruction >>> specification is correct? For example, the NQAP instruction - NQAP r1,r2 - >>> expects an even-odd pair of general registers in the r1 and r2 and must >>> designate an even register; otherwise a specification exception is >>> recognized. Are you saying that the handler for NQAP must verify this >>> requirement and inject a specification exception if it is not met? If not, >>> then can you provide a specific example of what you are talking about? >> >> Yes I mean this. >> >> But one other thing. >> Returning code 0x01 seems wrong for me, this is right if no AP card >> are in the system but I do not thing that it is what we mean. > I disagree .... for the guest, this is exactly what we mean. In my opinion, assigning > AP devices to the mediated matrix device is like assigning AP devices to an LPAR. > If no vfio-ap device is defined for the guest, then it is effectively the same as > starting a linux host in an LPAR without any AP devices assigned to it. >> >> I think we should return code 0x03 stating that the AP card is not in configured state > >> >> >> The difference is that we can get out of a not-configured state with a SCLP command but >> we can not recover from an un-existing APQN. > Remember, this state will occur only when the AP feature of the CPU model is turned on for > the guest and no vfio-ap device is defined, so there is no configuration to recover for > the guest because no AP devices will be configured for the guest. Spoke with Pierre and we agreed that the response code 01 is what we need to do. Regards, Halil
On 04/03/2018 05:36 AM, Cornelia Huck wrote: > On Mon, 2 Apr 2018 12:36:27 -0400 > Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: > >> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>> + /* >>>>> + * The Query Configuration Information (QCI) function (fc == 4) >>>>> does not >>>>> + * set a response code in reg 1, so check for that along with the >>>>> + * AP feature. >>>>> + */ >>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>> + env->regs[1] = 0x10000; >>>>> + >>>>> + return 0; >>>>> + } >>>> This would imply an operation exception in case fc==4, which sounds very >>>> wrong. >>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be >>> tested >>> to know what to answer. >>> If the feature is there, QCI must be answered correctly. >> This is an interesting proposition which raises several issues that will >> need to >> be addressed. The only time the PQAP(QCI) instruction is intercepted is >> when: >> * A vfio-ap device is NOT defined for the guest because the vfio_ap >> device driver >> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >> * STFLE.12 is set for the guest >> >> You say that the QCI must be answered correctly, but what is the correct >> response? >> If we return the matrix - i.e., APM, ADM and AQM - configured via the >> mediated >> matrix device's sysfs attributes files, then if any APQNs are defined in >> the matrix, >> we will have to handle *ALL* AP instructions by executing them on behalf >> of the >> guest. I suppose we could return an empty matrix in which case the AP >> bus will come >> up without any devices on the guest, but what is the expectation of an >> admin who >> deliberately configures the mediated matrix device? Should we forego >> handling interception >> of AP instructions and consider a guest configuration that turns on >> S390_FEAT_AP but >> does not define a vfio-ap device to be erroneous and terminate starting >> of the guest? >> Anybody have any thoughts? > Hard to really give good advice without access to the documentation, but: > - If we tell the guest that the feature is available, but it does not > get any cards to use, returning an empty matrix makes the most sense > to me. > - I would not tie starting the guest to the presence of a vfio-ap > device. Having the feature available in theory but without any > devices actually being usable by the guest does not really sound > wrong (can we hotplug this later?) For this phase of development, it is my opinion that introducing AP instruction interception handlers is superfluous for the following reasons: 1. Interception handling was introduced solely to ensure an operation exception would not be injected into the guest when CPU model feature for AP (i.e., ap=on) is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) is not. 2. The implementation of guest dedicated crypto adapters uses AP instruction interpretation to virtualize AP devices for a guest. As such, the NQAP, DQAP and most variants of the PQAP instructions will not be intercepted. 3. Hot plugging AP devices is not being supported for this phase of development. It is my opinion that introducing these interception handlers at this time is unnecessary and risks opening a can of worms that would be better dealt with in a subsequent phase. For that reason and the reasons stated above, I think the better option is to terminate starting the guest if the CPU model feature for AP is enabled but an AP device is not defined for the guest. This restriction, of course, will be removed when hot plug is implemented in a subsequent development phase. >
On 04/05/2018 06:38 PM, Tony Krowiak wrote: >> Hard to really give good advice without access to the documentation, but: >> - If we tell the guest that the feature is available, but it does not >> get any cards to use, returning an empty matrix makes the most sense >> to me. >> - I would not tie starting the guest to the presence of a vfio-ap >> device. Having the feature available in theory but without any >> devices actually being usable by the guest does not really sound >> wrong (can we hotplug this later?) > For this phase of development, it is my opinion that introducing AP instruction > interception handlers is superfluous for the following reasons: > > 1. Interception handling was introduced solely to ensure an operation exception would > not be injected into the guest when CPU model feature for AP (i.e., ap=on) > is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) > is not. > > 2. The implementation of guest dedicated crypto adapters uses AP instruction > interpretation to virtualize AP devices for a guest. As such, the NQAP, > DQAP and most variants of the PQAP instructions will not be > intercepted. > > 3. Hot plugging AP devices is not being supported for this phase of development. > > It is my opinion that introducing these interception handlers at this time is > unnecessary and risks opening a can of worms that would be > better dealt with in a subsequent phase. For that reason and the reasons stated > above, I think the better option is to terminate starting the guest if the > CPU model feature for AP is enabled but an AP device is not defined for the > guest. This restriction, of course, will be removed when hot plug is implemented > in a subsequent development phase. I second that! I agree that having ap instructions but not having the possibility to actually do AP crypto is probably not what the user wants. Preventing such a guest form starting (with a nice message) sounds reasonable to me. I agree with Connie, the approach 'hold the line' (until future hotplugs) is the most reasonable thing to do *in the long run*. But I think it's better to limit ourselves to the simplest case for now, I don't see any problems with doing the hotplug support later. Regards, Halil
On Thu, 5 Apr 2018 19:17:47 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 04/05/2018 06:38 PM, Tony Krowiak wrote: > >> Hard to really give good advice without access to the documentation, but: > >> - If we tell the guest that the feature is available, but it does not > >> get any cards to use, returning an empty matrix makes the most sense > >> to me. > >> - I would not tie starting the guest to the presence of a vfio-ap > >> device. Having the feature available in theory but without any > >> devices actually being usable by the guest does not really sound > >> wrong (can we hotplug this later?) > > For this phase of development, it is my opinion that introducing AP instruction > > interception handlers is superfluous for the following reasons: > > > > 1. Interception handling was introduced solely to ensure an operation exception would > > not be injected into the guest when CPU model feature for AP (i.e., ap=on) > > is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) > > is not. > > > > 2. The implementation of guest dedicated crypto adapters uses AP instruction > > interpretation to virtualize AP devices for a guest. As such, the NQAP, > > DQAP and most variants of the PQAP instructions will not be > > intercepted. > > > > 3. Hot plugging AP devices is not being supported for this phase of development. > > > > It is my opinion that introducing these interception handlers at this time is > > unnecessary and risks opening a can of worms that would be > > better dealt with in a subsequent phase. For that reason and the reasons stated > > above, I think the better option is to terminate starting the guest if the > > CPU model feature for AP is enabled but an AP device is not defined for the > > guest. This restriction, of course, will be removed when hot plug is implemented > > in a subsequent development phase. > > I second that! I agree that having ap instructions but not having the > possibility to actually do AP crypto is probably not what the user wants. > Preventing such a guest form starting (with a nice message) sounds reasonable > to me. One problem I have with that is that it feels backwards to me. The situation "you cannot add this device unless $FEATURE is present" is quite common and thus easily understood. Now, this would introduce the situation "you cannot present $FEATURE unless this device is also present, and that right at the start". I'm not sure how you are supposed to correlate a cpu feature with the existence of a device. > I agree with Connie, the approach 'hold the line' (until future hotplugs) > is the most reasonable thing to do *in the long run*. But I think it's better > to limit ourselves to the simplest case for now, I don't see any problems > with doing the hotplug support later. Yes, having to add handlers that add very little benefit sucks, I agree. But I fear if we add the "feature needs device" dependency, we open another can of worms, including the question what happens if we want to support hotplug in the future (I'm not altogether sure how to handle the whole checking from qemu). Making sure that we have both the feature and the device when using a management software (e.g. libvirt) makes a lot of sense (and is probably also easier to implement), but it won't help us with the issue of the interception handlers, unfortunately.
On 06.04.2018 10:40, Cornelia Huck wrote: > On Thu, 5 Apr 2018 19:17:47 +0200 > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> On 04/05/2018 06:38 PM, Tony Krowiak wrote: >>>> Hard to really give good advice without access to the documentation, but: >>>> - If we tell the guest that the feature is available, but it does not >>>> get any cards to use, returning an empty matrix makes the most sense >>>> to me. >>>> - I would not tie starting the guest to the presence of a vfio-ap >>>> device. Having the feature available in theory but without any >>>> devices actually being usable by the guest does not really sound >>>> wrong (can we hotplug this later?) >>> For this phase of development, it is my opinion that introducing AP instruction >>> interception handlers is superfluous for the following reasons: >>> >>> 1. Interception handling was introduced solely to ensure an operation exception would >>> not be injected into the guest when CPU model feature for AP (i.e., ap=on) >>> is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) >>> is not. >>> >>> 2. The implementation of guest dedicated crypto adapters uses AP instruction >>> interpretation to virtualize AP devices for a guest. As such, the NQAP, >>> DQAP and most variants of the PQAP instructions will not be >>> intercepted. >>> >>> 3. Hot plugging AP devices is not being supported for this phase of development. >>> >>> It is my opinion that introducing these interception handlers at this time is >>> unnecessary and risks opening a can of worms that would be >>> better dealt with in a subsequent phase. For that reason and the reasons stated >>> above, I think the better option is to terminate starting the guest if the >>> CPU model feature for AP is enabled but an AP device is not defined for the >>> guest. This restriction, of course, will be removed when hot plug is implemented >>> in a subsequent development phase. >> >> I second that! I agree that having ap instructions but not having the >> possibility to actually do AP crypto is probably not what the user wants. >> Preventing such a guest form starting (with a nice message) sounds reasonable >> to me. > > One problem I have with that is that it feels backwards to me. > > The situation "you cannot add this device unless $FEATURE is present" > is quite common and thus easily understood. Now, this would introduce > the situation "you cannot present $FEATURE unless this device is also > present, and that right at the start". I'm not sure how you are > supposed to correlate a cpu feature with the existence of a device. I agree. Don't make things harder than they are. This smells like "cpu feature can only be provided if another magical QEMU command line option is present". Don't do that. Is it really that hard to implement a very simple interception handler that says t all instructions "yeah, I'm alive, but no, nothing to see here". > >> I agree with Connie, the approach 'hold the line' (until future hotplugs) >> is the most reasonable thing to do *in the long run*. But I think it's better >> to limit ourselves to the simplest case for now, I don't see any problems >> with doing the hotplug support later. > > Yes, having to add handlers that add very little benefit sucks, I > agree. But I fear if we add the "feature needs device" dependency, we > open another can of worms, including the question what happens if we > want to support hotplug in the future (I'm not altogether sure how to > handle the whole checking from qemu). > > Making sure that we have both the feature and the device when using a > management software (e.g. libvirt) makes a lot of sense (and is > probably also easier to implement), but it won't help us with the issue > of the interception handlers, unfortunately. > I agree.
On 04/06/2018 11:11 AM, David Hildenbrand wrote: > On 06.04.2018 10:40, Cornelia Huck wrote: >> On Thu, 5 Apr 2018 19:17:47 +0200 >> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >> >>> On 04/05/2018 06:38 PM, Tony Krowiak wrote: >>>>> Hard to really give good advice without access to the documentation, but: >>>>> - If we tell the guest that the feature is available, but it does not >>>>> get any cards to use, returning an empty matrix makes the most sense >>>>> to me. >>>>> - I would not tie starting the guest to the presence of a vfio-ap >>>>> device. Having the feature available in theory but without any >>>>> devices actually being usable by the guest does not really sound >>>>> wrong (can we hotplug this later?) >>>> For this phase of development, it is my opinion that introducing AP instruction >>>> interception handlers is superfluous for the following reasons: >>>> >>>> 1. Interception handling was introduced solely to ensure an operation exception would >>>> not be injected into the guest when CPU model feature for AP (i.e., ap=on) >>>> is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) >>>> is not. >>>> >>>> 2. The implementation of guest dedicated crypto adapters uses AP instruction >>>> interpretation to virtualize AP devices for a guest. As such, the NQAP, >>>> DQAP and most variants of the PQAP instructions will not be >>>> intercepted. >>>> >>>> 3. Hot plugging AP devices is not being supported for this phase of development. >>>> >>>> It is my opinion that introducing these interception handlers at this time is >>>> unnecessary and risks opening a can of worms that would be >>>> better dealt with in a subsequent phase. For that reason and the reasons stated >>>> above, I think the better option is to terminate starting the guest if the >>>> CPU model feature for AP is enabled but an AP device is not defined for the >>>> guest. This restriction, of course, will be removed when hot plug is implemented >>>> in a subsequent development phase. >>> >>> I second that! I agree that having ap instructions but not having the >>> possibility to actually do AP crypto is probably not what the user wants. >>> Preventing such a guest form starting (with a nice message) sounds reasonable >>> to me. >> >> One problem I have with that is that it feels backwards to me. >> >> The situation "you cannot add this device unless $FEATURE is present" >> is quite common and thus easily understood. Now, this would introduce >> the situation "you cannot present $FEATURE unless this device is also >> present, and that right at the start". I'm not sure how you are >> supposed to correlate a cpu feature with the existence of a device. I think it can be done straightforward and with less LOC than interception and emulation of 'nothing to see here' requires. > > I agree. Don't make things harder than they are. This smells like "cpu > feature can only be provided if another magical QEMU command line option > is present". Don't do that. Yes it is conceptually ugly. I'm 100% with you. That's why it should go away soon. From the practicality perspective however I would even argue that it's helpful to the user: tells 'oops you have forgotten something'. IMHO it's a shortcut of type make the problem smaller. Regarding what is harder and what is easier: the author is probably the most fit to decide that. If it is harder, it makes no sense, as this is all about cutting corners. > > Is it really that hard to implement a very simple interception handler > that says t all instructions "yeah, I'm alive, but no, nothing to see here". > I find it somewhat difficult to reason about what is static and what is dynamic in the AP architecture. To put something together that seems to work should be relatively easy. I could even say, I hope Tony tested the no device case with v3 and it apparently seemed to work -- as I don't see any does not work disclaimer. But getting all the stuff correct is IMHO a bit more of a challenge. >> >>> I agree with Connie, the approach 'hold the line' (until future hotplugs) >>> is the most reasonable thing to do *in the long run*. But I think it's better >>> to limit ourselves to the simplest case for now, I don't see any problems >>> with doing the hotplug support later. >> >> Yes, having to add handlers that add very little benefit sucks, I >> agree. But I fear if we add the "feature needs device" dependency, we >> open another can of worms, including the question what happens if we >> want to support hotplug in the future (I'm not altogether sure how to >> handle the whole checking from qemu). >> We do want hotplug in the future AFAIK. The idea was to just remove the limitation when everything is in place. Regarding the implementation, the idea was to use qemu_add_machine_init_done_notifier and only catch both of the following true * the cpu model has ap=on * on the ap bus (I think it would be nice to have a bus with max_dev = 1) there is no device When hotplug becomes available for vfio-ap we would just remove that code. For me it seemed legit. We have a precedence for not really complete stuff with vfio-ccw. So if it was OK to defer the stuff that was deferred there, I think, ap=on suddenly working without the strange device should be OK too. >> Making sure that we have both the feature and the device when using a >> management software (e.g. libvirt) makes a lot of sense (and is >> probably also easier to implement), but it won't help us with the issue >> of the interception handlers, unfortunately. >> I disagree. Conceptually hotplug of vfio-ap is perfectly legit. So doing something like this in management software sounds wrong. The idea here is cutting corners in order to have something that works reasonably well but with a couple of well defined limitations sooner. Regards, Halil
On 04/06/2018 02:09 PM, Halil Pasic wrote: > Yes it is conceptually ugly. I'm 100% with you. That's why it should go > away soon. From the practicality perspective however I would even argue that it's > helpful to the user: tells 'oops you have forgotten something'. IMHO > it's a shortcut of type make the problem smaller. Regarding what is > harder and what is easier: the author is probably the most fit to decide > that. If it is harder, it makes no sense, as this is all about cutting > corners. I've just realized, I have overlooked something. And that is using what libvirt calls host-model and host-passthrough mode. There the user does not explicitly ask for ap=on. So the user would get slapped in the face by this 'needs vfio-ap device' message (AFAIU) after upgrading stuff (without even knowing that AP was added) which is extremely ugly! I need to think about this some more.
On Fri, Apr 06, 2018 at 02:32:49PM +0200, Halil Pasic wrote: > > > On 04/06/2018 02:09 PM, Halil Pasic wrote: > > Yes it is conceptually ugly. I'm 100% with you. That's why it should go > > away soon. From the practicality perspective however I would even argue that it's > > helpful to the user: tells 'oops you have forgotten something'. IMHO > > it's a shortcut of type make the problem smaller. Regarding what is > > harder and what is easier: the author is probably the most fit to decide > > that. If it is harder, it makes no sense, as this is all about cutting > > corners. > > I've just realized, I have overlooked something. And that is using > what libvirt calls host-model and host-passthrough mode. There the > user does not explicitly ask for ap=on. So the user would get slapped > in the face by this 'needs vfio-ap device' message (AFAIU) after > upgrading stuff (without even knowing that AP was added) which is > extremely ugly! I need to think about this some more. Typically in this kind of scenario, enablement of the new feature is tied to QEMU machine type. IOW, existing machine types should not get the new feature, only the very latest machine type. That way existing guests are not exposed to it when upgrading QEMU, unless they also change their machine type. Regards, Daniel
On 16/03/2018 00:24, Tony Krowiak wrote: > If the CPU model indicates that AP facility is installed on > the guest (i.e., -cpu xxxx,ap=on), then the expectation is that > the AP bus running in the guest will initialize; however, if the > AP instructions are not being interpreted by the firmware, then > they will be intercepted and routed back to QEMU for handling. > If a handler is not defined to process the intercepted instruciton, > then an operation exception will be injected into the > guest, in which case the AP bus will not initialize. > > There are two situations where AP instructions will not be > interpreted: > > 1. The guest is not configured with a vfio-ap device (i.e., > -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for > the vfio-ap device enables interpretive execution of AP > instructions. > > 2. The guest is a second level guest but the first level guest has > not enabled interpretive execution. > > This patch introduces AP instruction handlers to ensure the AP bus > module initializes on the guest when the AP facility is installed > on the guest but AP instructions are not being interpreted. The logic > incorporated is: > > * If the CPU model indicates AP instructions are > installed > > * Set the status response code for the instruction to indicate that > the APQN contained in the instruction is not valid. This is > a valid response because there will be no devices configured for > the guest in any of the above scenarios. > > * Else return an error from the handler. This will result in an > operation being injected into the guest and the AP bus will not > initialize on the guest. That is commensurate with how things work > today. > > Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> > --- > hw/vfio/ap.c | 45 ++++++++++++++++++++++++++++++++++++++++++ > include/hw/s390x/ap-device.h | 6 +++++ > target/s390x/kvm.c | 14 +++++++++++++ > 3 files changed, 65 insertions(+), 0 deletions(-) > > diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c > index b397bb1..88e744d 100644 > --- a/hw/vfio/ap.c > +++ b/hw/vfio/ap.c > @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error **errp) > kvm_s390_set_interpret_ap(0); > } > > +int ap_device_handle_nqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + > + if (s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > +int ap_device_handle_dqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + > + if (s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > +int ap_device_handle_pqap(S390CPU *cpu) > +{ > + CPUS390XState *env = &cpu->env; > + int fc = 4 & (env->regs[0] >> 24); > + > + /* > + * The Query Configuration Information (QCI) function (fc == 4) does not > + * set a response code in reg 1, so check for that along with the > + * AP feature. > + */ APFT-t must be taken care of, depending on APQN and APFT feature: If APFT feature is enabled it should report the right information for the existing AP If it is not enabled it should report OPERATION_EXCEPTION > + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { > + env->regs[1] = 0x10000; > + > + return 0; > + } > + > + return -EOPNOTSUPP; > +} > + > static Property vfio_ap_properties[] = { > DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), > DEFINE_PROP_END_OF_LIST(), > diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h > index 693df90..d45ae38 100644 > --- a/include/hw/s390x/ap-device.h > +++ b/include/hw/s390x/ap-device.h > @@ -11,6 +11,8 @@ > #ifndef HW_S390X_AP_DEVICE_H > #define HW_S390X_AP_DEVICE_H > > +#include "cpu.h" > + > #define AP_DEVICE_TYPE "ap-device" > > typedef struct APDevice { > @@ -35,4 +37,8 @@ static inline APDevice *to_ap_dev(DeviceState *dev) > #define AP_DEVICE_CLASS(klass) \ > OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE) > > +int ap_device_handle_nqap(S390CPU *cpu); > +int ap_device_handle_dqap(S390CPU *cpu); > +int ap_device_handle_pqap(S390CPU *cpu); > + > #endif /* HW_S390X_AP_DEVICE_H */ > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index 2812e28..a636394 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -50,6 +50,7 @@ > #include "exec/memattrs.h" > #include "hw/s390x/s390-virtio-ccw.h" > #include "hw/s390x/s390-virtio-hcall.h" > +#include "hw/s390x/ap-device.h" > > #ifndef DEBUG_KVM > #define DEBUG_KVM 0 > @@ -88,6 +89,9 @@ > #define PRIV_B2_CHSC 0x5f > #define PRIV_B2_SIGA 0x74 > #define PRIV_B2_XSCH 0x76 > +#define PRIV_B2_NQAP 0xad > +#define PRIV_B2_DQAP 0xae > +#define PRIV_B2_PQAP 0xaf > > #define PRIV_EB_SQBS 0x8a > #define PRIV_EB_PCISTB 0xd0 > @@ -1245,6 +1249,16 @@ static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) > case PRIV_B2_SCLP_CALL: > rc = kvm_sclp_service_call(cpu, run, ipbh0); > break; > + case PRIV_B2_NQAP: > + rc = ap_device_handle_nqap(cpu); > + break; > + case PRIV_B2_DQAP: > + rc = ap_device_handle_dqap(cpu); > + break; > + case PRIV_B2_PQAP: > + rc = ap_device_handle_pqap(cpu); > + break; > + break; > default: > rc = -1; > DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1);
On 06/04/2018 16:08, Pierre Morel wrote: > On 16/03/2018 00:24, Tony Krowiak wrote: >> If the CPU model indicates that AP facility is installed on >> the guest (i.e., -cpu xxxx,ap=on), then the expectation is that >> the AP bus running in the guest will initialize; however, if the >> AP instructions are not being interpreted by the firmware, then >> they will be intercepted and routed back to QEMU for handling. >> If a handler is not defined to process the intercepted instruciton, >> then an operation exception will be injected into the >> guest, in which case the AP bus will not initialize. >> >> There are two situations where AP instructions will not be >> interpreted: >> >> 1. The guest is not configured with a vfio-ap device (i.e., >> -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for >> the vfio-ap device enables interpretive execution of AP >> instructions. >> >> 2. The guest is a second level guest but the first level guest has >> not enabled interpretive execution. >> >> This patch introduces AP instruction handlers to ensure the AP bus >> module initializes on the guest when the AP facility is installed >> on the guest but AP instructions are not being interpreted. The logic >> incorporated is: >> >> * If the CPU model indicates AP instructions are >> installed >> >> * Set the status response code for the instruction to indicate that >> the APQN contained in the instruction is not valid. This is >> a valid response because there will be no devices configured for >> the guest in any of the above scenarios. >> >> * Else return an error from the handler. This will result in an >> operation being injected into the guest and the AP bus will not >> initialize on the guest. That is commensurate with how things work >> today. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> >> --- >> hw/vfio/ap.c | 45 >> ++++++++++++++++++++++++++++++++++++++++++ >> include/hw/s390x/ap-device.h | 6 +++++ >> target/s390x/kvm.c | 14 +++++++++++++ >> 3 files changed, 65 insertions(+), 0 deletions(-) >> >> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c >> index b397bb1..88e744d 100644 >> --- a/hw/vfio/ap.c >> +++ b/hw/vfio/ap.c >> @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, >> Error **errp) >> kvm_s390_set_interpret_ap(0); >> } >> >> +int ap_device_handle_nqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_dqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + >> + if (s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> +int ap_device_handle_pqap(S390CPU *cpu) >> +{ >> + CPUS390XState *env = &cpu->env; >> + int fc = 4 & (env->regs[0] >> 24); >> + >> + /* >> + * The Query Configuration Information (QCI) function (fc == 4) >> does not >> + * set a response code in reg 1, so check for that along with the >> + * AP feature. >> + */ > > APFT-t must be taken care of, depending on APQN and APFT feature: > > If APFT feature is enabled it should report the right information for > the existing AP > If it is not enabled it should report OPERATION_EXCEPTION hum, sorry, only if interception of TAPQ-t is enabled and it is not. so forget it. Sorry for the noise. > >> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >> + env->regs[1] = 0x10000; >> + >> + return 0; >> + } >> + >> + return -EOPNOTSUPP; >> +} >> + >> static Property vfio_ap_properties[] = { >> DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), >> DEFINE_PROP_END_OF_LIST(), >> diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h >> index 693df90..d45ae38 100644 >> --- a/include/hw/s390x/ap-device.h >> +++ b/include/hw/s390x/ap-device.h >> @@ -11,6 +11,8 @@ >> #ifndef HW_S390X_AP_DEVICE_H >> #define HW_S390X_AP_DEVICE_H >> >> +#include "cpu.h" >> + >> #define AP_DEVICE_TYPE "ap-device" >> >> typedef struct APDevice { >> @@ -35,4 +37,8 @@ static inline APDevice *to_ap_dev(DeviceState *dev) >> #define AP_DEVICE_CLASS(klass) \ >> OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE) >> >> +int ap_device_handle_nqap(S390CPU *cpu); >> +int ap_device_handle_dqap(S390CPU *cpu); >> +int ap_device_handle_pqap(S390CPU *cpu); >> + >> #endif /* HW_S390X_AP_DEVICE_H */ >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index 2812e28..a636394 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -50,6 +50,7 @@ >> #include "exec/memattrs.h" >> #include "hw/s390x/s390-virtio-ccw.h" >> #include "hw/s390x/s390-virtio-hcall.h" >> +#include "hw/s390x/ap-device.h" >> >> #ifndef DEBUG_KVM >> #define DEBUG_KVM 0 >> @@ -88,6 +89,9 @@ >> #define PRIV_B2_CHSC 0x5f >> #define PRIV_B2_SIGA 0x74 >> #define PRIV_B2_XSCH 0x76 >> +#define PRIV_B2_NQAP 0xad >> +#define PRIV_B2_DQAP 0xae >> +#define PRIV_B2_PQAP 0xaf >> >> #define PRIV_EB_SQBS 0x8a >> #define PRIV_EB_PCISTB 0xd0 >> @@ -1245,6 +1249,16 @@ static int handle_b2(S390CPU *cpu, struct >> kvm_run *run, uint8_t ipa1) >> case PRIV_B2_SCLP_CALL: >> rc = kvm_sclp_service_call(cpu, run, ipbh0); >> break; >> + case PRIV_B2_NQAP: >> + rc = ap_device_handle_nqap(cpu); >> + break; >> + case PRIV_B2_DQAP: >> + rc = ap_device_handle_dqap(cpu); >> + break; >> + case PRIV_B2_PQAP: >> + rc = ap_device_handle_pqap(cpu); >> + break; >> + break; >> default: >> rc = -1; >> DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1); > >
On 04/05/2018 06:38 PM, Tony Krowiak wrote: > On 04/03/2018 05:36 AM, Cornelia Huck wrote: >> On Mon, 2 Apr 2018 12:36:27 -0400 >> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: >> >>> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>>> + /* >>>>>> + * The Query Configuration Information (QCI) function (fc == 4) >>>>>> does not >>>>>> + * set a response code in reg 1, so check for that along with the >>>>>> + * AP feature. >>>>>> + */ >>>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>>> + env->regs[1] = 0x10000; >>>>>> + >>>>>> + return 0; >>>>>> + } >>>>> This would imply an operation exception in case fc==4, which sounds very >>>>> wrong. >>>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be >>>> tested >>>> to know what to answer. >>>> If the feature is there, QCI must be answered correctly. >>> This is an interesting proposition which raises several issues that will >>> need to >>> be addressed. The only time the PQAP(QCI) instruction is intercepted is >>> when: >>> * A vfio-ap device is NOT defined for the guest because the vfio_ap >>> device driver >>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >>> * STFLE.12 is set for the guest >>> >>> You say that the QCI must be answered correctly, but what is the correct >>> response? >>> If we return the matrix - i.e., APM, ADM and AQM - configured via the >>> mediated >>> matrix device's sysfs attributes files, then if any APQNs are defined in >>> the matrix, >>> we will have to handle *ALL* AP instructions by executing them on behalf >>> of the >>> guest. I suppose we could return an empty matrix in which case the AP >>> bus will come >>> up without any devices on the guest, but what is the expectation of an >>> admin who >>> deliberately configures the mediated matrix device? Should we forego >>> handling interception >>> of AP instructions and consider a guest configuration that turns on >>> S390_FEAT_AP but >>> does not define a vfio-ap device to be erroneous and terminate starting >>> of the guest? >>> Anybody have any thoughts? >> Hard to really give good advice without access to the documentation, but: >> - If we tell the guest that the feature is available, but it does not >> get any cards to use, returning an empty matrix makes the most sense >> to me. >> - I would not tie starting the guest to the presence of a vfio-ap >> device. Having the feature available in theory but without any >> devices actually being usable by the guest does not really sound >> wrong (can we hotplug this later?) > For this phase of development, it is my opinion that introducing AP instruction > interception handlers is superfluous for the following reasons: > > 1. Interception handling was introduced solely to ensure an operation exception would > not be injected into the guest when CPU model feature for AP (i.e., ap=on) > is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) > is not. We can kind of (i.e. modulo EECA.28) ensure this in a different fashion I think. How about proclaiming a 'has ap instructions, but nothing to see here' in the SIE interpreted flavor (ECA.28 set) the default way of having ap instructions under KVM. This should be easily accomplished with an all zero CRYCB and eca.28 set. The for the guest to actually get real work done with AP we would still require some sort of driver to either provide a non-zero matrix by altering the CRYCB or unsettling ECA.28 and doing the intercepted flavor. Please notice, the cpu facility ap would still keep it's semantic 'has ap instructions' (opposed to 'has ap instructions implemented in SIE interpreted flavor). And give us all the flexibility. Yet implementing what we want to have in absence of a driver would become much easier (under the assumption that ECA.28 equals EECA.28). How about this?
On Fri, 6 Apr 2018 18:07:56 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 04/05/2018 06:38 PM, Tony Krowiak wrote: > > On 04/03/2018 05:36 AM, Cornelia Huck wrote: > >> On Mon, 2 Apr 2018 12:36:27 -0400 > >> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: > >> > >>> On 03/26/2018 05:03 AM, Pierre Morel wrote: > >>>> On 26/03/2018 10:32, David Hildenbrand wrote: > >>>>> On 16.03.2018 00:24, Tony Krowiak wrote: > >>>>>> + /* > >>>>>> + * The Query Configuration Information (QCI) function (fc == 4) > >>>>>> does not > >>>>>> + * set a response code in reg 1, so check for that along with the > >>>>>> + * AP feature. > >>>>>> + */ > >>>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { > >>>>>> + env->regs[1] = 0x10000; > >>>>>> + > >>>>>> + return 0; > >>>>>> + } > >>>>> This would imply an operation exception in case fc==4, which sounds very > >>>>> wrong. > >>>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be > >>>> tested > >>>> to know what to answer. > >>>> If the feature is there, QCI must be answered correctly. > >>> This is an interesting proposition which raises several issues that will > >>> need to > >>> be addressed. The only time the PQAP(QCI) instruction is intercepted is > >>> when: > >>> * A vfio-ap device is NOT defined for the guest because the vfio_ap > >>> device driver > >>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted > >>> * STFLE.12 is set for the guest > >>> > >>> You say that the QCI must be answered correctly, but what is the correct > >>> response? > >>> If we return the matrix - i.e., APM, ADM and AQM - configured via the > >>> mediated > >>> matrix device's sysfs attributes files, then if any APQNs are defined in > >>> the matrix, > >>> we will have to handle *ALL* AP instructions by executing them on behalf > >>> of the > >>> guest. I suppose we could return an empty matrix in which case the AP > >>> bus will come > >>> up without any devices on the guest, but what is the expectation of an > >>> admin who > >>> deliberately configures the mediated matrix device? Should we forego > >>> handling interception > >>> of AP instructions and consider a guest configuration that turns on > >>> S390_FEAT_AP but > >>> does not define a vfio-ap device to be erroneous and terminate starting > >>> of the guest? > >>> Anybody have any thoughts? > >> Hard to really give good advice without access to the documentation, but: > >> - If we tell the guest that the feature is available, but it does not > >> get any cards to use, returning an empty matrix makes the most sense > >> to me. > >> - I would not tie starting the guest to the presence of a vfio-ap > >> device. Having the feature available in theory but without any > >> devices actually being usable by the guest does not really sound > >> wrong (can we hotplug this later?) > > For this phase of development, it is my opinion that introducing AP instruction > > interception handlers is superfluous for the following reasons: > > > > 1. Interception handling was introduced solely to ensure an operation exception would > > not be injected into the guest when CPU model feature for AP (i.e., ap=on) > > is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) > > is not. > > We can kind of (i.e. modulo EECA.28) ensure this in a different fashion I think. How > about proclaiming a 'has ap instructions, but nothing to see here' in the > SIE interpreted flavor (ECA.28 set) the default way of having ap instructions > under KVM. This should be easily accomplished with an all zero CRYCB and eca.28 > set. The for the guest to actually get real work done with AP we would > still require some sort of driver to either provide a non-zero matrix by > altering the CRYCB or unsettling ECA.28 and doing the intercepted flavor. > > Please notice, the cpu facility ap would still keep it's semantic > 'has ap instructions' (opposed to 'has ap instructions implemented in > SIE interpreted flavor). And give us all the flexibility. > > Yet implementing what we want to have in absence of a driver would become > much easier (under the assumption that ECA.28 equals EECA.28). > > How about this? Unfortunately, this is really hard to follow without the AR... let me summarize it to check whether I got the gist of it :) - If the "ap" cpu feature is specified, set a bit that indicates "hey, we basically have have AP support" and create the basics, but don't enable actual SIE handling. This means the guest gets exceptions from the SIE already and we don't need to emulate them. - Actually enable the missing pieces if a vfio device is created. This would enable processing by the SIE, and we would not need to do emulation, either (for most of it, IIRC). I may be all wrong, though... can we at least have a translation of ECA.28 and EECA.28 (the "ap is there" bit and the "ap instructions are interpreted" bit?)
On 04/09/2018 11:32 AM, Cornelia Huck wrote: >> We can kind of (i.e. modulo EECA.28) ensure this in a different fashion I think. How >> about proclaiming a 'has ap instructions, but nothing to see here' in the >> SIE interpreted flavor (ECA.28 set) the default way of having ap instructions >> under KVM. This should be easily accomplished with an all zero CRYCB and eca.28 >> set. The for the guest to actually get real work done with AP we would >> still require some sort of driver to either provide a non-zero matrix by >> altering the CRYCB or unsettling ECA.28 and doing the intercepted flavor. >> >> Please notice, the cpu facility ap would still keep it's semantic >> 'has ap instructions' (opposed to 'has ap instructions implemented in >> SIE interpreted flavor). And give us all the flexibility. >> >> Yet implementing what we want to have in absence of a driver would become >> much easier (under the assumption that ECA.28 equals EECA.28). >> >> How about this? > Unfortunately, this is really hard to follow without the AR... let me > summarize it to check whether I got the gist of it :) > > - If the "ap" cpu feature is specified, set a bit that indicates "hey, > we basically have have AP support" and create the basics, but don't > enable actual SIE handling. This means the guest gets exceptions from > the SIE already and we don't need to emulate them. Kind of. The bit is ECA.28 and tells SIE 'hey SIE shall interpret ap instructions for the guest (as specified)'. Then SIE has an SD satellite called CRYCB that contains the which ap resources is the guest authorized to use. These are the masks. If we set each mask to all zero, we will effectively accomplish 'hey,we basically have have AP support but no resources at the moment'. So, right, we don't have to emulate that. I don't know what do you mean by exceptions. For most legit requests the SIE should say APQN invalid, with QCI being a notable exception. But of course SIE would inject program exceptions (access, specification, and privileged operation) accordingly I guess. In short, the SIE would do what we are trying to emulate in this patch. > - Actually enable the missing pieces if a vfio device is created. This > would enable processing by the SIE, and we would not need to do > emulation, either (for most of it, IIRC). Yes. It would actually assign resources to the guest. That would enable doing real work with the AP instructions. > > I may be all wrong, though... can we at least have a translation of > ECA.28 and EECA.28 (the "ap is there" bit and the "ap instructions are > interpreted" bit?) > I think we have a misunderstanding here. I will wait for Tony. Maybe he can understand this better or explain in more accessible way. Regards, Halil
On Mon, 9 Apr 2018 12:37:42 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 04/09/2018 11:32 AM, Cornelia Huck wrote: > >> We can kind of (i.e. modulo EECA.28) ensure this in a different fashion I think. How > >> about proclaiming a 'has ap instructions, but nothing to see here' in the > >> SIE interpreted flavor (ECA.28 set) the default way of having ap instructions > >> under KVM. This should be easily accomplished with an all zero CRYCB and eca.28 > >> set. The for the guest to actually get real work done with AP we would > >> still require some sort of driver to either provide a non-zero matrix by > >> altering the CRYCB or unsettling ECA.28 and doing the intercepted flavor. > >> > >> Please notice, the cpu facility ap would still keep it's semantic > >> 'has ap instructions' (opposed to 'has ap instructions implemented in > >> SIE interpreted flavor). And give us all the flexibility. > >> > >> Yet implementing what we want to have in absence of a driver would become > >> much easier (under the assumption that ECA.28 equals EECA.28). > >> > >> How about this? > > Unfortunately, this is really hard to follow without the AR... let me > > summarize it to check whether I got the gist of it :) > > > > - If the "ap" cpu feature is specified, set a bit that indicates "hey, > > we basically have have AP support" and create the basics, but don't > > enable actual SIE handling. This means the guest gets exceptions from > > the SIE already and we don't need to emulate them. > > Kind of. The bit is ECA.28 and tells SIE 'hey SIE shall interpret ap > instructions for the guest (as specified)'. Then SIE has an SD satellite > called CRYCB that contains the which ap resources is the guest authorized > to use. These are the masks. If we set each mask to all zero, we will > effectively accomplish 'hey,we basically have have AP support but no > resources at the moment'. So, right, we don't have to emulate that. > > I don't know what do you mean by exceptions. For most legit requests the > SIE should say APQN invalid, with QCI being a notable exception. But > of course SIE would inject program exceptions (access, specification, > and privileged operation) accordingly I guess. I meant "emulate exceptions"... > > > In short, the SIE would do what we are trying to emulate in this patch. ...so yes, exactly that. > > > - Actually enable the missing pieces if a vfio device is created. This > > would enable processing by the SIE, and we would not need to do > > emulation, either (for most of it, IIRC). > > Yes. It would actually assign resources to the guest. That would enable > doing real work with the AP instructions. Ok. > > > > > I may be all wrong, though... can we at least have a translation of > > ECA.28 and EECA.28 (the "ap is there" bit and the "ap instructions are > > interpreted" bit?) > > > > I think we have a misunderstanding here. I will wait for Tony. Maybe > he can understand this better or explain in more accessible way. From what I get from your explanation, this approach sounds like a good way forward. But let's wait for Tony.
On 04/09/2018 06:51 AM, Cornelia Huck wrote: > On Mon, 9 Apr 2018 12:37:42 +0200 > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> On 04/09/2018 11:32 AM, Cornelia Huck wrote: >>>> We can kind of (i.e. modulo EECA.28) ensure this in a different fashion I think. How >>>> about proclaiming a 'has ap instructions, but nothing to see here' in the >>>> SIE interpreted flavor (ECA.28 set) the default way of having ap instructions >>>> under KVM. This should be easily accomplished with an all zero CRYCB and eca.28 >>>> set. The for the guest to actually get real work done with AP we would >>>> still require some sort of driver to either provide a non-zero matrix by >>>> altering the CRYCB or unsettling ECA.28 and doing the intercepted flavor. >>>> >>>> Please notice, the cpu facility ap would still keep it's semantic >>>> 'has ap instructions' (opposed to 'has ap instructions implemented in >>>> SIE interpreted flavor). And give us all the flexibility. >>>> >>>> Yet implementing what we want to have in absence of a driver would become >>>> much easier (under the assumption that ECA.28 equals EECA.28). >>>> >>>> How about this? >>> Unfortunately, this is really hard to follow without the AR... let me >>> summarize it to check whether I got the gist of it :) >>> >>> - If the "ap" cpu feature is specified, set a bit that indicates "hey, >>> we basically have have AP support" and create the basics, but don't >>> enable actual SIE handling. This means the guest gets exceptions from >>> the SIE already and we don't need to emulate them. >> Kind of. The bit is ECA.28 and tells SIE 'hey SIE shall interpret ap >> instructions for the guest (as specified)'. Then SIE has an SD satellite >> called CRYCB that contains the which ap resources is the guest authorized >> to use. These are the masks. If we set each mask to all zero, we will >> effectively accomplish 'hey,we basically have have AP support but no >> resources at the moment'. So, right, we don't have to emulate that. >> >> I don't know what do you mean by exceptions. For most legit requests the >> SIE should say APQN invalid, with QCI being a notable exception. But >> of course SIE would inject program exceptions (access, specification, >> and privileged operation) accordingly I guess. > I meant "emulate exceptions"... > >> >> In short, the SIE would do what we are trying to emulate in this patch. > ...so yes, exactly that. > >>> - Actually enable the missing pieces if a vfio device is created. This >>> would enable processing by the SIE, and we would not need to do >>> emulation, either (for most of it, IIRC). >> Yes. It would actually assign resources to the guest. That would enable >> doing real work with the AP instructions. > Ok. > >>> I may be all wrong, though... can we at least have a translation of >>> ECA.28 and EECA.28 (the "ap is there" bit and the "ap instructions are >>> interpreted" bit?) >>> >> I think we have a misunderstanding here. I will wait for Tony. Maybe >> he can understand this better or explain in more accessible way. > From what I get from your explanation, this approach sounds like a good > way forward. But let's wait for Tony. I agree, this solves the problem with specifying installing AP facilities on the guest (-cpu xxx,ap=on) and not configuring a vfio-ap device (-device vfio-ap,sysfsdev=$path-to-mediated-device). To recap: The realize() function for the vfio-ap device opens the mediated matrix device file descriptor to set up the communication pathway with the vfio_ap kernel driver. When the fd is opened, the vfio_ap driver sets ECA.28 for the guest to instruct SIE to interpret AP instructions. The driver also configures the AP matrix for the guest in its SIE state description. Consequently, if a vfio-ap device is not configured for the guest, but AP facilities are installed, all AP instructions will be intercepted and routed to QEMU. If there are no interception handlers for the AP instructions, QEMU injects an operation exception into the guest. This results in initialization of the AP bus on the guest to terminate. This patch was intended to circumvent that problem. With Halil's suggestion, there is no need to provide these handlers. If ECA.28 is set for the guest by default when the AP facilities are installed, then the AP instructions will be interpreted and the AP bus will get initialized on the guest. Since there is no vfio-ap device to provide the AP matrix configuration for the guest, the AP bus will not detect any devices, but that's okay. AP instructions targeting at an APQN will execute successfully and set a response code in the status block returned from the instruction indicating the APQN is invalid .... but there will be no exception unless there is truly an exception condition caused by the execution of the instruction. >
On 04/11/2018 03:20 PM, Tony Krowiak wrote: >>>> I may be all wrong, though... can we at least have a translation of >>>> ECA.28 and EECA.28 (the "ap is there" bit and the "ap instructions are >>>> interpreted" bit?) >>>> >>> I think we have a misunderstanding here. I will wait for Tony. Maybe >>> he can understand this better or explain in more accessible way. >> From what I get from your explanation, this approach sounds like a good >> way forward. But let's wait for Tony. > > I agree, this solves the problem with specifying installing AP facilities > on the guest (-cpu xxx,ap=on) and not configuring a vfio-ap device > (-device vfio-ap,sysfsdev=$path-to-mediated-device). > > To recap: > The realize() function for the vfio-ap device opens the mediated matrix > device file descriptor to set up the communication pathway with the vfio_ap > kernel driver. When the fd is opened, the vfio_ap driver sets ECA.28 for the > guest to instruct SIE to interpret AP instructions. The driver also > configures the AP matrix for the guest in its SIE state description. Consequently, > if a vfio-ap device is not configured for the guest, but AP facilities are > installed, all AP instructions will be intercepted and routed to QEMU. If there > are no interception handlers for the AP instructions, QEMU injects an operation > exception into the guest. This results in initialization of the AP bus on the > guest to terminate. This patch was intended to circumvent that problem. > > With Halil's suggestion, there is no need to provide these handlers. If ECA.28 > is set for the guest by default when the AP facilities are installed, then the AP > instructions will be interpreted and the AP bus will get initialized on the > guest. Since there is no vfio-ap device to provide the AP matrix configuration > for the guest, the AP bus will not detect any devices, but that's okay. AP > instructions targeting at an APQN will execute successfully and set a response > code in the status block returned from the instruction indicating the > APQN is invalid .... but there will be no exception unless there is truly > an exception condition caused by the execution of the instruction. That's exactly the idea, but it will only work out this way if the effective ECA.28 bit (i.e. EECA.28) is also set. Could you please comment the first paragraph quoted in this email? Halil
On 04/09/2018 05:32 AM, Cornelia Huck wrote: > On Fri, 6 Apr 2018 18:07:56 +0200 > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> On 04/05/2018 06:38 PM, Tony Krowiak wrote: >>> On 04/03/2018 05:36 AM, Cornelia Huck wrote: >>>> On Mon, 2 Apr 2018 12:36:27 -0400 >>>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: >>>> >>>>> On 03/26/2018 05:03 AM, Pierre Morel wrote: >>>>>> On 26/03/2018 10:32, David Hildenbrand wrote: >>>>>>> On 16.03.2018 00:24, Tony Krowiak wrote: >>>>>>>> + /* >>>>>>>> + * The Query Configuration Information (QCI) function (fc == 4) >>>>>>>> does not >>>>>>>> + * set a response code in reg 1, so check for that along with the >>>>>>>> + * AP feature. >>>>>>>> + */ >>>>>>>> + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >>>>>>>> + env->regs[1] = 0x10000; >>>>>>>> + >>>>>>>> + return 0; >>>>>>>> + } >>>>>>> This would imply an operation exception in case fc==4, which sounds very >>>>>>> wrong. >>>>>> It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be >>>>>> tested >>>>>> to know what to answer. >>>>>> If the feature is there, QCI must be answered correctly. >>>>> This is an interesting proposition which raises several issues that will >>>>> need to >>>>> be addressed. The only time the PQAP(QCI) instruction is intercepted is >>>>> when: >>>>> * A vfio-ap device is NOT defined for the guest because the vfio_ap >>>>> device driver >>>>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >>>>> * STFLE.12 is set for the guest >>>>> >>>>> You say that the QCI must be answered correctly, but what is the correct >>>>> response? >>>>> If we return the matrix - i.e., APM, ADM and AQM - configured via the >>>>> mediated >>>>> matrix device's sysfs attributes files, then if any APQNs are defined in >>>>> the matrix, >>>>> we will have to handle *ALL* AP instructions by executing them on behalf >>>>> of the >>>>> guest. I suppose we could return an empty matrix in which case the AP >>>>> bus will come >>>>> up without any devices on the guest, but what is the expectation of an >>>>> admin who >>>>> deliberately configures the mediated matrix device? Should we forego >>>>> handling interception >>>>> of AP instructions and consider a guest configuration that turns on >>>>> S390_FEAT_AP but >>>>> does not define a vfio-ap device to be erroneous and terminate starting >>>>> of the guest? >>>>> Anybody have any thoughts? >>>> Hard to really give good advice without access to the documentation, but: >>>> - If we tell the guest that the feature is available, but it does not >>>> get any cards to use, returning an empty matrix makes the most sense >>>> to me. >>>> - I would not tie starting the guest to the presence of a vfio-ap >>>> device. Having the feature available in theory but without any >>>> devices actually being usable by the guest does not really sound >>>> wrong (can we hotplug this later?) >>> For this phase of development, it is my opinion that introducing AP instruction >>> interception handlers is superfluous for the following reasons: >>> >>> 1. Interception handling was introduced solely to ensure an operation exception would >>> not be injected into the guest when CPU model feature for AP (i.e., ap=on) >>> is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) >>> is not. >> We can kind of (i.e. modulo EECA.28) ensure this in a different fashion I think. How >> about proclaiming a 'has ap instructions, but nothing to see here' in the >> SIE interpreted flavor (ECA.28 set) the default way of having ap instructions >> under KVM. This should be easily accomplished with an all zero CRYCB and eca.28 >> set. The for the guest to actually get real work done with AP we would >> still require some sort of driver to either provide a non-zero matrix by >> altering the CRYCB or unsettling ECA.28 and doing the intercepted flavor. >> >> Please notice, the cpu facility ap would still keep it's semantic >> 'has ap instructions' (opposed to 'has ap instructions implemented in >> SIE interpreted flavor). And give us all the flexibility. >> >> Yet implementing what we want to have in absence of a driver would become >> much easier (under the assumption that ECA.28 equals EECA.28). >> >> How about this? > Unfortunately, this is really hard to follow without the AR... let me > summarize it to check whether I got the gist of it :) > > - If the "ap" cpu feature is specified, set a bit that indicates "hey, > we basically have have AP support" and create the basics, but don't > enable actual SIE handling. This means the guest gets exceptions from > the SIE already and we don't need to emulate them. > - Actually enable the missing pieces if a vfio device is created. This > would enable processing by the SIE, and we would not need to do > emulation, either (for most of it, IIRC). > > I may be all wrong, though... can we at least have a translation of > ECA.28 and EECA.28 (the "ap is there" bit and the "ap instructions are > interpreted" bit?) I am not sure what you are asking here, but I'll attempt to answer the question I think you are asking. The ap=on|off flag indicates that AP instructions are installed on the guest. This feature is enabled by the kernel only if AP instructions are installed on the host. Since there is no facilities bit to query, this is determined by attempting to execute an AP instruction using an exception table. If there is an exception, it is assumed that the AP instructions are not installed. The ECA.28 bit in the SIE state description indicates whether AP instructions are interpreted. For level 1 guests, the ECA.28 bit specified in the SIE state description is used directly. For guest level 2 guests, the value is calculated by doing a logical AND of the guest level 1 ECA.28 bit and the guest level 2 ECA.28 bit. This value is known by the term Effective Execution Control A bit 28, or EECA.28. To the best of my knowledge, - as well as verified empirically, ECA.28 for the linux host (i.e., guest level 1) is set by default, so EECA.28 will effectively be whatever value is specified by ECA.28 in the level 2 guest's SIE state description. This will not be the case for guest level 3 when we implement VSIE support. >
On 04/11/2018 09:50 AM, Halil Pasic wrote: > > On 04/11/2018 03:20 PM, Tony Krowiak wrote: >>>>> I may be all wrong, though... can we at least have a translation of >>>>> ECA.28 and EECA.28 (the "ap is there" bit and the "ap instructions are >>>>> interpreted" bit?) >>>>> >>>> I think we have a misunderstanding here. I will wait for Tony. Maybe >>>> he can understand this better or explain in more accessible way. >>> From what I get from your explanation, this approach sounds like a good >>> way forward. But let's wait for Tony. >> I agree, this solves the problem with specifying installing AP facilities >> on the guest (-cpu xxx,ap=on) and not configuring a vfio-ap device >> (-device vfio-ap,sysfsdev=$path-to-mediated-device). >> >> To recap: >> The realize() function for the vfio-ap device opens the mediated matrix >> device file descriptor to set up the communication pathway with the vfio_ap >> kernel driver. When the fd is opened, the vfio_ap driver sets ECA.28 for the >> guest to instruct SIE to interpret AP instructions. The driver also >> configures the AP matrix for the guest in its SIE state description. Consequently, >> if a vfio-ap device is not configured for the guest, but AP facilities are >> installed, all AP instructions will be intercepted and routed to QEMU. If there >> are no interception handlers for the AP instructions, QEMU injects an operation >> exception into the guest. This results in initialization of the AP bus on the >> guest to terminate. This patch was intended to circumvent that problem. >> >> With Halil's suggestion, there is no need to provide these handlers. If ECA.28 >> is set for the guest by default when the AP facilities are installed, then the AP >> instructions will be interpreted and the AP bus will get initialized on the >> guest. Since there is no vfio-ap device to provide the AP matrix configuration >> for the guest, the AP bus will not detect any devices, but that's okay. AP >> instructions targeting at an APQN will execute successfully and set a response >> code in the status block returned from the instruction indicating the >> APQN is invalid .... but there will be no exception unless there is truly >> an exception condition caused by the execution of the instruction. > That's exactly the idea, but it will only work out this way if the effective > ECA.28 bit (i.e. EECA.28) is also set. > > Could you please comment the first paragraph quoted in this email? See my response to Connie's comment in Message ID <20180409113244.380a32c4.cohuck@redhat.com>. My response is in Message ID <17b4ab9b-a8c7-37ec-bae4-c57b146ba0b8@linux.vnet.ibm.com>. > > Halil
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c index b397bb1..88e744d 100644 --- a/hw/vfio/ap.c +++ b/hw/vfio/ap.c @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error **errp) kvm_s390_set_interpret_ap(0); } +int ap_device_handle_nqap(S390CPU *cpu) +{ + CPUS390XState *env = &cpu->env; + + if (s390_has_feat(S390_FEAT_AP)) { + env->regs[1] = 0x10000; + + return 0; + } + + return -EOPNOTSUPP; +} + +int ap_device_handle_dqap(S390CPU *cpu) +{ + CPUS390XState *env = &cpu->env; + + if (s390_has_feat(S390_FEAT_AP)) { + env->regs[1] = 0x10000; + + return 0; + } + + return -EOPNOTSUPP; +} + +int ap_device_handle_pqap(S390CPU *cpu) +{ + CPUS390XState *env = &cpu->env; + int fc = 4 & (env->regs[0] >> 24); + + /* + * The Query Configuration Information (QCI) function (fc == 4) does not + * set a response code in reg 1, so check for that along with the + * AP feature. + */ + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { + env->regs[1] = 0x10000; + + return 0; + } + + return -EOPNOTSUPP; +} + static Property vfio_ap_properties[] = { DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), DEFINE_PROP_END_OF_LIST(), diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h index 693df90..d45ae38 100644 --- a/include/hw/s390x/ap-device.h +++ b/include/hw/s390x/ap-device.h @@ -11,6 +11,8 @@ #ifndef HW_S390X_AP_DEVICE_H #define HW_S390X_AP_DEVICE_H +#include "cpu.h" + #define AP_DEVICE_TYPE "ap-device" typedef struct APDevice { @@ -35,4 +37,8 @@ static inline APDevice *to_ap_dev(DeviceState *dev) #define AP_DEVICE_CLASS(klass) \ OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE) +int ap_device_handle_nqap(S390CPU *cpu); +int ap_device_handle_dqap(S390CPU *cpu); +int ap_device_handle_pqap(S390CPU *cpu); + #endif /* HW_S390X_AP_DEVICE_H */ diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 2812e28..a636394 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -50,6 +50,7 @@ #include "exec/memattrs.h" #include "hw/s390x/s390-virtio-ccw.h" #include "hw/s390x/s390-virtio-hcall.h" +#include "hw/s390x/ap-device.h" #ifndef DEBUG_KVM #define DEBUG_KVM 0 @@ -88,6 +89,9 @@ #define PRIV_B2_CHSC 0x5f #define PRIV_B2_SIGA 0x74 #define PRIV_B2_XSCH 0x76 +#define PRIV_B2_NQAP 0xad +#define PRIV_B2_DQAP 0xae +#define PRIV_B2_PQAP 0xaf #define PRIV_EB_SQBS 0x8a #define PRIV_EB_PCISTB 0xd0 @@ -1245,6 +1249,16 @@ static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) case PRIV_B2_SCLP_CALL: rc = kvm_sclp_service_call(cpu, run, ipbh0); break; + case PRIV_B2_NQAP: + rc = ap_device_handle_nqap(cpu); + break; + case PRIV_B2_DQAP: + rc = ap_device_handle_dqap(cpu); + break; + case PRIV_B2_PQAP: + rc = ap_device_handle_pqap(cpu); + break; + break; default: rc = -1; DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1);
If the CPU model indicates that AP facility is installed on the guest (i.e., -cpu xxxx,ap=on), then the expectation is that the AP bus running in the guest will initialize; however, if the AP instructions are not being interpreted by the firmware, then they will be intercepted and routed back to QEMU for handling. If a handler is not defined to process the intercepted instruciton, then an operation exception will be injected into the guest, in which case the AP bus will not initialize. There are two situations where AP instructions will not be interpreted: 1. The guest is not configured with a vfio-ap device (i.e., -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for the vfio-ap device enables interpretive execution of AP instructions. 2. The guest is a second level guest but the first level guest has not enabled interpretive execution. This patch introduces AP instruction handlers to ensure the AP bus module initializes on the guest when the AP facility is installed on the guest but AP instructions are not being interpreted. The logic incorporated is: * If the CPU model indicates AP instructions are installed * Set the status response code for the instruction to indicate that the APQN contained in the instruction is not valid. This is a valid response because there will be no devices configured for the guest in any of the above scenarios. * Else return an error from the handler. This will result in an operation being injected into the guest and the AP bus will not initialize on the guest. That is commensurate with how things work today. Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- hw/vfio/ap.c | 45 ++++++++++++++++++++++++++++++++++++++++++ include/hw/s390x/ap-device.h | 6 +++++ target/s390x/kvm.c | 14 +++++++++++++ 3 files changed, 65 insertions(+), 0 deletions(-)