Re: [PATCH v3 2/9] s390: ap: kvm: setting a hook for PQAP instructions

From: Pierre Morel
Date: Thu Feb 21 2019 - 07:40:43 EST

On 19/02/2019 23:36, Tony Krowiak wrote:
On 2/19/19 2:50 PM, Pierre Morel wrote:
On 18/02/2019 23:42, Cornelia Huck wrote:
On Mon, 18 Feb 2019 19:29:10 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

On 15/02/2019 23:02, Tony Krowiak wrote:
On 2/14/19 8:51 AM, Pierre Morel wrote:

+ * handle_pqap: Handling pqap interception
+ * @vcpu: the vcpu having issue the pqap instruction
+ *
+ * This callback only handles PQAP/AQIC instruction and
+ * calls a dedicated callback for this instruction if
+ * a driver did register one in the CRYPTO satellite of the
+ * SIE block.
+ *
+ * Do not change the behavior if, return -EOPNOTSUPP if:
+ * - the hook is not used do not change the behavior.
+ * - AP instructions are not available or not available to the guest
+ * - the instruction is not PQAP with function code indicating
+ *ÂÂ AQIC do not change the previous behavior.
+ *
+ * For PQAP/AQIC instruction, verify privilege and specifications
+ *
+ * return the value returned by the callback.
+ */
+static int handle_pqap(struct kvm_vcpu *vcpu)
+ÂÂÂ uint8_t fc;
+ÂÂÂ /* Verify that the hook callback is registered */
+ÂÂÂ if (!vcpu->kvm->arch.crypto.pqap_hook)
+ÂÂÂ /* Verify that the AP instruction are available */
+ÂÂÂ if (!ap_instructions_available())
+ÂÂÂ /* Verify that the guest is allowed to use AP instructions */
+ÂÂÂ if (!(vcpu->arch.sie_block->eca & ECA_APIE))
+ÂÂÂ /* Verify that the function code is AQIC */
+ÂÂÂ fc = vcpu->run->s.regs.gprs[0] >> 24;
+ÂÂÂ if (fc != 0x03)

This does not belong here. Function code 3 is one of 7 function codes
that can be sent with the PQAP instruction. This belongs in the PQAP
hook code.

On one hand, effectively I would prefer to put the code in the VFIO
driver code.
On the other hand, doing this would lead to export the code for
test_kvm_facility() and kvm_s390_inject_program_int() from the kvm-s390.h

I choose not to export these functions from the KVM code.

Would like opinion from KVM maintainers?

Looking at this (and without access to the specification...), I think
the check for problem state makes sense in here (if this applies to all
PQAP functions equally, which seems likely). The check for the facility
makes more sense in the handler. You can probably still inject the
specification exception here if you use a clever return code.

If there is no objection on exporting the KVM functions... I can do this.

I do not understand why you would have to export KVM functions to place
the check for FC 0x03 in the pqap hook? What am I missing here? Maybe
you misunderstood my comment?

No I did not but in between I discovered an error in the handling of the interception of PQAP/AQIC.

QEMU and KVM can both accept PQAP/AQIC even if the vfio_ap driver is not loaded.
However now that the guest officially get the PQAP/AQIC instruction we need to handle the specification and operation exceptions inside KVM
_before_ testing and even calling the driver hook.

I will make the changes in the next iteration.


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