Re: [Qemu-devel] [PATCH v3 6/7] s390x/kvm: handle AP instruction interception

[Tony Krowiak]

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