lkml.org 
[lkml]   [2018]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v5 11/13] KVM: s390: implement mediated device open callback
    From
    Date
    On 05/16/2018 04:03 AM, Pierre Morel wrote:
    > On 07/05/2018 17:11, Tony Krowiak wrote:
    >> Implements the open callback on the mediated matrix device.
    >> The function registers a group notifier to receive notification
    >> of the VFIO_GROUP_NOTIFY_SET_KVM event. When notified,
    >> the vfio_ap device driver will get access to the guest's
    >> kvm structure. With access to this structure the driver will:
    >>
    >> 1. Ensure that only one mediated device is opened for the guest
    >>
    >> 2. Configure access to the AP devices for the guest.
    >>
    >> Access to AP adapters, usage domains and control domains
    >> is controlled by three bit masks contained in the Crypto Control
    >> Block (CRYCB) referenced from the guest's SIE state description:
    >>
    >> * The AP Mask (APM) controls access to the AP adapters. Each bit
    >> in the APM represents an adapter number - from most significant
    >> to least significant bit - from 0 to 255. The bits in the APM
    >> are set according to the adapter numbers assigned to the mediated
    >> matrix device via its 'assign_adapter' sysfs attribute file.
    >>
    >> * The AP Queue Mask (AQM) controls access to the AP queues. Each bit
    >> in the AQM represents an AP queue index - from most significant
    >> to least significant bit - from 0 to 255. A queue index references
    >> a specific domain and is synonymous with the domian number. The
    >> bits in the AQM are set according to the domain numbers assigned
    >> to the mediated matrix device via its 'assign_domain' sysfs
    >> attribute file.
    >>
    >> * The AP Domain Mask (ADM) controls access to the AP control
    >> domains.
    >> Each bit in the ADM represents a control domain - from most
    >> significant to least significant bit - from 0-255. The
    >> bits in the ADM are set according to the domain numbers assigned
    >> to the mediated matrix device via its 'assign_control_domain'
    >> sysfs attribute file.
    >>
    >> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
    >> ---
    >> arch/s390/include/asm/kvm-ap.h | 21 ++++++++++
    >> arch/s390/include/asm/kvm_host.h | 1 +
    >> arch/s390/kvm/kvm-ap.c | 19 +++++++++
    >> drivers/s390/crypto/vfio_ap_ops.c | 68
    >> +++++++++++++++++++++++++++++++++
    >> drivers/s390/crypto/vfio_ap_private.h | 2 +
    >> 5 files changed, 111 insertions(+), 0 deletions(-)
    >>
    >> diff --git a/arch/s390/include/asm/kvm-ap.h
    >> b/arch/s390/include/asm/kvm-ap.h
    >> index 21fe9f2..68c5a67 100644
    >> --- a/arch/s390/include/asm/kvm-ap.h
    >> +++ b/arch/s390/include/asm/kvm-ap.h
    >> @@ -83,6 +83,27 @@ struct kvm_ap_matrix {
    >> bool kvm_ap_instructions_available(void);
    >>
    >> /**
    >> + * kvm_ap_refcount_read
    >> + *
    >> + * Read the AP reference count and return it.
    >> + */
    >> +int kvm_ap_refcount_read(struct kvm *kvm);
    >> +
    >> +/**
    >> + * kvm_ap_refcount_inc
    >> + *
    >> + * Increment the AP reference count.
    >> + */
    >> +void kvm_ap_refcount_inc(struct kvm *kvm);
    >> +
    >> +/**
    >> + * kvm_ap_refcount_dec
    >> + *
    >> + * Decrement the AP reference count
    >> + */
    >> +void kvm_ap_refcount_dec(struct kvm *kvm);
    >> +
    >> +/**
    >> * kvm_ap_configure_matrix
    >> *
    >> * Configure the AP matrix for a KVM guest.
    >> diff --git a/arch/s390/include/asm/kvm_host.h
    >> b/arch/s390/include/asm/kvm_host.h
    >> index 8736cde..5f1ad02 100644
    >> --- a/arch/s390/include/asm/kvm_host.h
    >> +++ b/arch/s390/include/asm/kvm_host.h
    >> @@ -717,6 +717,7 @@ struct kvm_s390_crypto {
    >> __u8 aes_kw;
    >> __u8 dea_kw;
    >> __u8 apie;
    >> + atomic_t aprefs;
    >> };
    >>
    >> #define APCB0_MASK_SIZE 1
    >> diff --git a/arch/s390/kvm/kvm-ap.c b/arch/s390/kvm/kvm-ap.c
    >> index 98b53c7..848fb37 100644
    >> --- a/arch/s390/kvm/kvm-ap.c
    >> +++ b/arch/s390/kvm/kvm-ap.c
    >> @@ -9,6 +9,7 @@
    >> #include <linux/kernel.h>
    >> #include <linux/bitops.h>
    >> #include <asm/kvm-ap.h>
    >> +#include <asm/atomic.h>
    >>
    >> #include "kvm-s390.h"
    >>
    >> @@ -218,6 +219,24 @@ static int kvm_ap_validate_queue_sharing(struct
    >> kvm *kvm,
    >> return 0;
    >> }
    >>
    >> +int kvm_ap_refcount_read(struct kvm *kvm)
    >> +{
    >> + return atomic_read(&kvm->arch.crypto.aprefs);
    >> +}
    >> +EXPORT_SYMBOL(kvm_ap_refcount_read);
    >> +
    >> +void kvm_ap_refcount_inc(struct kvm *kvm)
    >> +{
    >> + atomic_inc(&kvm->arch.crypto.aprefs);
    >> +}
    >> +EXPORT_SYMBOL(kvm_ap_refcount_inc);
    >> +
    >> +void kvm_ap_refcount_dec(struct kvm *kvm)
    >> +{
    >> + atomic_dec(&kvm->arch.crypto.aprefs);
    >> +}
    >> +EXPORT_SYMBOL(kvm_ap_refcount_dec);
    >
    > Why are these functions inside kvm-ap ?
    > Will anyone use this outer of vfio-ap ?

    As I've stated before, I made the choice to contain all interfaces that
    access KVM in kvm-ap because I don't think it is appropriate for the device
    driver to have to have "knowledge" of the inner workings of KVM. Why does
    it matter whether any entity outside of the vfio_ap device driver calls
    these functions? I could ask a similar question if the interfaces were
    contained in vfio-ap; what if another device driver needs access to these
    interfaces?

    >
    >
    >> +
    >> int kvm_ap_configure_matrix(struct kvm *kvm, struct kvm_ap_matrix
    >> *matrix)
    >> {
    >> int ret = 0;
    >> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
    >> b/drivers/s390/crypto/vfio_ap_ops.c
    >> index 81e03b8..8866b0e 100644
    >> --- a/drivers/s390/crypto/vfio_ap_ops.c
    >> +++ b/drivers/s390/crypto/vfio_ap_ops.c
    >> @@ -11,6 +11,8 @@
    >> #include <linux/device.h>
    >> #include <linux/list.h>
    >> #include <linux/ctype.h>
    >> +#include <linux/module.h>
    >> +#include <asm/kvm-ap.h>
    >>
    >> #include "vfio_ap_private.h"
    >>
    >> @@ -47,6 +49,70 @@ static int vfio_ap_mdev_remove(struct mdev_device
    >> *mdev)
    >> return 0;
    >> }
    >>
    >> +static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
    >> + unsigned long action, void *data)
    >> +{
    >> + struct ap_matrix_mdev *matrix_mdev;
    >> +
    >> + if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
    >> + matrix_mdev = container_of(nb, struct ap_matrix_mdev,
    >> + group_notifier);
    >> + matrix_mdev->kvm = data;
    >> + }
    >> +
    >> + return NOTIFY_OK;
    >> +}
    >> +
    >> +static int vfio_ap_mdev_open(struct mdev_device *mdev)
    >> +{
    >> + struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
    >> + unsigned long events;
    >> + int ret;
    >> +
    >> + if (!try_module_get(THIS_MODULE))
    >> + return -ENODEV;
    >> +
    >> + matrix_mdev->group_notifier.notifier_call =
    >> vfio_ap_mdev_group_notifier;
    >> + events = VFIO_GROUP_NOTIFY_SET_KVM;
    >> +
    >> + ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
    >> + &events, &matrix_mdev->group_notifier);
    >> + if (ret)
    >> + goto out_err;
    >> +
    >> + /* Only one mediated device allowed per guest */
    >> + if (kvm_ap_refcount_read(matrix_mdev->kvm) != 0) {
    >> + ret = -EEXIST;
    >> + goto out_err;
    >> + }
    >
    > Testing the existence should be the first thing to do.

    That would be better but access to KVM is not available until the
    notifier runs.

    >
    >
    >> +
    >> + kvm_ap_refcount_inc(matrix_mdev->kvm);
    >> +
    >> + ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
    >> &matrix_mdev->matrix);
    >> + if (ret)
    >> + goto config_err;
    >> +
    >> + return 0;
    >> +
    >> +config_err:
    >> + kvm_ap_refcount_dec(matrix_mdev->kvm);
    >> +out_err:
    >> + module_put(THIS_MODULE);
    >> +
    >> + return ret;
    >> +}
    >> +
    >> +static void vfio_ap_mdev_release(struct mdev_device *mdev)
    >> +{
    >> + struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
    >> +
    >> + kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
    >> + vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
    >> + &matrix_mdev->group_notifier);
    >> + kvm_ap_refcount_dec(matrix_mdev->kvm);
    >> + module_put(THIS_MODULE);
    >> +}
    >> +
    >> static ssize_t name_show(struct kobject *kobj, struct device *dev,
    >> char *buf)
    >> {
    >> return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
    >> @@ -773,6 +839,8 @@ static ssize_t matrix_show(struct device *dev,
    >> struct device_attribute *attr,
    >> .mdev_attr_groups = vfio_ap_mdev_attr_groups,
    >> .create = vfio_ap_mdev_create,
    >> .remove = vfio_ap_mdev_remove,
    >> + .open = vfio_ap_mdev_open,
    >> + .release = vfio_ap_mdev_release,
    >> };
    >>
    >> int vfio_ap_mdev_register(struct ap_matrix *ap_matrix)
    >> diff --git a/drivers/s390/crypto/vfio_ap_private.h
    >> b/drivers/s390/crypto/vfio_ap_private.h
    >> index 8b6ad66..ab072e9 100644
    >> --- a/drivers/s390/crypto/vfio_ap_private.h
    >> +++ b/drivers/s390/crypto/vfio_ap_private.h
    >> @@ -32,6 +32,8 @@ struct ap_matrix {
    >>
    >> struct ap_matrix_mdev {
    >> struct kvm_ap_matrix matrix;
    >> + struct notifier_block group_notifier;
    >> + struct kvm *kvm;
    >> };
    >>
    >> static inline struct ap_matrix *to_ap_matrix(struct device *dev)
    >
    >

    \
     
     \ /
      Last update: 2018-05-23 16:46    [W:5.018 / U:0.240 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site