Re: [PATCH v5 1/7] s390: ap: kvm: add PQAP interception for AQIC

[Pierre Morel]

On 19/03/2019 15:54, Halil Pasic wrote:
> On Tue, 19 Mar 2019 11:01:44 +0100
> Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
>
> > On 15/03/2019 18:28, Halil Pasic wrote:
>
> [..]
>
> > > Things get complicated when one considers that ECA.28 is an effective
> > > control.
> >
> > I don't think so, ECA_28 is not really a problem.
> > We do not propagate ECA_AIV in VSIE and ECA_AIV is tested in the vfio
> > driver to support GISA.
> > So that the guest 3 will not support interrupt.
>
> That was not my concern, but while we are at it... I guess you refer to
> the check in handle_pqap(). That seems to do -EOPNOTSUPP, i.e. got to
> userspace, i.e. with today's QEMU operation exception. Which does not
> seem right.

We already discussed this. no?

> My concern was the following. Let assume
> ECA.28 == 1 and EECA.28 == 0 != 1
> and guest issues a PQAP (for simplicity AQIC).
>
> Currently I guess we take a 0x04 interception and go to userspace, which
> may or may not be the best thing to do.
>
> With this patch we would take a 0x04, but (opposed to before) if guest
> does not have facility 65 we go with a specification exception.

This is not right.
We return -EOPNOTSUPP which will be intercepted by QEMU which will 
report an OPERATION exception as before.

> Operation exception should however take priority over this kind of
> specification exception. So basically everything except PQAP/AQIC would
> give you and operation exception (with current QEMU), but PQAP/AQIC would
> give you a specification exception. Which is wrong!
>
> AFAICT there is no way to tell if we got a 04 interception because
> EECA.28 != 1 (and ECA.28 == 1) and FW won't interpret the AP
> instructions for us, or because it PQAP/AQIC is a mandatory intercept.
> In other words I don't see a way to tell if EECA.28 is 1 when
> interpreting PQAP/AQIC.
>
> Do you agree?


No.
EECA = HOST_ECA & GUEST_ECA
after we made sure that AP instructions are available, HOST_ECA=1

(vcpu->arch.sie_block->eca & ECA_APIE) gives us the answer.

In the case HOST_ECA=0 we always go to userland as before.

> [..]
>
> > Yes, the alternative is:
> >
> > 1) We do things right but this mean we change the ABI (SPECIFICATION
> > instead of OPERATION)
> >
> > I thing this is the best thing to do, it is the implementation
> > proposed by this patch where all is done in Kernel, so that we are
> > right what ever the userland user is (QEMU or other).
> >
> > 2) We want to preserve the old ABI for old QEMU
> > Then I proposed the implementation here under.
> >
> >
> > My personal opinion, is that we should change the ABI and do things
> > right now.
>
> I tend to agree. Giving an operation exception instead of a specification
> exception is a bug. If it is a kernel or qemu bug it ain't clear to me
> at the moment.
>
> > We should also do it right for TAPQ with t bit set. I remember
> > Christian already warned about this but we did not implement it.
>
> Yes, I have some blurry memories of something similar myself. I wonder
> if there was a reason, or did we just forget to address this issue.


I will integrate it in the next iteration too, even it is not IRQ, the 
PQAP hook patch can be more general.

Regards,
Pierre

> Regards,
> Halil


--
Pierre Morel
Linux/KVM/QEMU in BÃblingen - Germany