lkml.org 
[lkml]   [2019]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v5 1/7] s390: ap: kvm: add PQAP interception for AQIC
    From
    Date
    On 15/03/2019 15:10, Pierre Morel wrote:
    > On 15/03/2019 14:26, Pierre Morel wrote:
    >> On 15/03/2019 11:20, Cornelia Huck wrote:
    >>> On Wed, 13 Mar 2019 17:04:58 +0100
    >>> Pierre Morel <pmorel@linux.ibm.com> wrote:
    >>>
    >>>> +/*
    >>>> + * handle_pqap: Handling pqap interception
    >>>> + * @vcpu: the vcpu having issue the pqap instruction
    >>>> + *
    >>>> + * We now support PQAP/AQIC instructions and we need to correctly
    >>>> + * answer the guest even if no dedicated driver's hook is available.
    >>>> + *
    >>>> + * The intercepting code calls a dedicated callback for this
    >>>> instruction
    >>>> + * if a driver did register one in the CRYPTO satellite of the
    >>>> + * SIE block.
    >>>> + *
    >>>> + * For PQAP/AQIC instructions only, verify privilege and
    >>>> specifications.
    >>>> + *
    >>>> + * If no callback available, the queues are not available, return
    >>>> this to
    >>>> + * the caller.
    >>>> + * Else return the value returned by the callback.
    >>>> + */
    >>>> +static int handle_pqap(struct kvm_vcpu *vcpu)
    >>>> +{
    >>>> +    uint8_t fc;
    >>>> +    struct ap_queue_status status = {};
    >>>> +    int ret;
    >>>> +    /* Verify that the AP instruction are available */
    >>>> +    if (!ap_instructions_available())
    >>>> +        return -EOPNOTSUPP;
    >>>> +    /* Verify that the guest is allowed to use AP instructions */
    >>>> +    if (!(vcpu->arch.sie_block->eca & ECA_APIE))
    >>>> +        return -EOPNOTSUPP;
    >>>> +    /* Verify that the function code is AQIC */
    >>>> +    fc = vcpu->run->s.regs.gprs[0] >> 24;
    >>>> +    /* We do not want to change the behavior we had before this
    >>>> patch*/
    >>>> +    if (fc != 0x03)
    >>>> +        return -EOPNOTSUPP;
    >>>> +
    >>>> +    /* PQAP instructions are allowed for guest kernel only */
    >>>> +    if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)
    >>>> +        return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP);
    >>>> +    /* AQIC instruction is allowed only if facility 65 is available */
    >>>> +    if (!test_kvm_facility(vcpu->kvm, 65))
    >>>> +        return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
    >>>> +    /* Verify that the hook callback is registered and call it */
    >>>> +    if (vcpu->kvm->arch.crypto.pqap_hook) {
    >>>> +        if (!try_module_get(vcpu->kvm->arch.crypto.pqap_hook->owner))
    >>>> +            return -EOPNOTSUPP;
    >>>> +        ret = vcpu->kvm->arch.crypto.pqap_hook->hook(vcpu);
    >>>> +        module_put(vcpu->kvm->arch.crypto.pqap_hook->owner);
    >>>> +        return ret;
    >>>> +    }
    >>>> +    /*
    >>>> +     * It is the duty of the vfio_driver to register a hook
    >>>> +     * If it does not and we get an exception on AQIC we must
    >>>> +     * guess that there is no vfio_ap_driver at all and no one
    >>>> +     * to handle the guests's CRYCB and the CRYCB is empty.
    >>>> +     */
    >>>> +    status.response_code = 0x01;
    >>>
    >>> I'm still confused here, sorry. From previous discussions I recall that
    >>> this indicates "no crypto device" (please correct me if I'm wrong.)
    >>>
    >>> Before this patch, we had:
    >>> - guest issues PQAP/AQIC -> drop to userspace
    >>>
    >>> With a correct implementation, we get:
    >>> - guest issues PQAP/AQIC -> callback does what needs to be done
    >>>
    >>> With an incorrect implementation (no callback), we get:
    >>> - guest issues PQAP/AQIC -> guest gets response code 0x01
    >>>
    >>> Why not drop to userspace in that case?
    >>
    >> This is what I had in the previous patches.
    >> Hum, I do not remember which discussion lead me to modify this.
    >>
    >> Anyway, now that you put the finger on this problem, I think the
    >> problem is worse.
    >>
    >> The behavior with old / new Linux, vfio driver and qemu is:
    >>
    >> LINUX    VFIO_AP    QEMU    PGM
    >> OLD    x    x    OPERATION
    >> NEW    -    OLD    SPECIFICATION
    >> NEW    -    NEW/aqic=off    SPECIFICATION
    >> NEW    x    NEW/aqic=on    -
    >>
    >> x = whatever
    >> - = absent/none
    >>
    >> So yes there is a change in behavior for the userland for the case
    >> QEMU do not set the AQIC facility 65, OLD QEMU or NEW QEMU wanting to
    >> behave like an older one.
    >>
    >> I fear we have the same problem with the privileged operation...
    >>
    >> For the last case, when the kvm_facility(65) is set, the explication
    >> is the following:
    >>
    >> This is related to the handling of PQAP AQIC which is now authorized
    >> by this patch series.
    >> If we authorize PQAP AQIC, by setting the bit for facility 65, the
    >> guest can use this instruction.
    >> If the instruction follows the specifications we must answer something
    >> realistic and since there is nothing in the CRYCB (no driver) we
    >> answer that there is no queue.
    >>
    >> Conclusion:  we must handle this in userland, it will have the benefit
    >> to keep old behavior when there is no callback.
    >> OLD QEMU will not see change as they will not set aqic facility
    >> NEW QEMU will handle this correctly.
    >>
    >
    > Sorry, wrong conclusion, handling this in userland will bring us much
    > too far if we want to answer correctly for the case the hook is not
    > there but QEMU accepted the facility for AQIC.

    Sorry, forget it, I was tired.

    Pierre

    >
    > The alternative is easier, we just continue to respond with the
    > OPERATION exception here and only handle the specification and
    > privileged exception cases in QEMU and in the hook.
    >
    > So, I think the discussion will go on until you come back :)
    >
    > Regards,
    > Pierre
    >


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

    \
     
     \ /
      Last update: 2019-03-19 10:57    [W:3.474 / U:22.604 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site