lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 16 Apr 2018 15:05:58 +0200
From:   Pierre Morel <pmorel@...ux.vnet.ibm.com>
To:     Tony Krowiak <akrowiak@...ux.vnet.ibm.com>,
        linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     freude@...ibm.com, schwidefsky@...ibm.com,
        heiko.carstens@...ibm.com, borntraeger@...ibm.com,
        cohuck@...hat.com, kwankhede@...dia.com,
        bjsdjshi@...ux.vnet.ibm.com, pbonzini@...hat.com,
        alex.williamson@...hat.com, alifm@...ux.vnet.ibm.com,
        mjrosato@...ux.vnet.ibm.com, jjherne@...ux.vnet.ibm.com,
        thuth@...hat.com, pasic@...ux.vnet.ibm.com, berrange@...hat.com,
        fiuczy@...ux.vnet.ibm.com, buendgen@...ibm.com
Subject: Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

On 15/04/2018 23:22, Tony Krowiak wrote:
> Registers a group notifier during the open of the mediated
> matrix device to get information on KVM presence through the
> VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
> to the kvm structure is saved inside the mediated matrix
> device. Once the VFIO AP device driver has access to KVM,
> access to the APs can be configured for the guest.
>
> Access to APs is configured when the file descriptor for the
> mediated matrix device is opened by userspace. The items to be
> configured are:
>
> 1. The ECA.28 bit in the SIE state description determines whether
>     AP instructions are interpreted by the hardware or intercepted.
>     The VFIO AP device driver relies interpretive execution of
>     AP instructions so the ECA.28 bit will be set
>
> 2. Guest access to AP adapters, usage domains and control domains
>     is controlled by three bit masks referenced from 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 (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@...ux.vnet.ibm.com>
> ---
>   drivers/s390/crypto/vfio_ap_ops.c     |   50 +++++++++++++++++++++++++++++++++
>   drivers/s390/crypto/vfio_ap_private.h |    2 +
>   2 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index bc2b05e..e3ff5ab 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -53,6 +53,54 @@ 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;
> +
> +	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)
> +		return ret;
> +
> +	ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);

Do you need this call ?
apie is always enabled in KVM if AP instructions are available.

Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.

Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for the 
second call?



> +	if (ret)
> +		return ret;
> +
> +	ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
> +				      matrix_mdev->matrix);
> +
> +	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);
> +	kvm_ap_interpret_instructions(matrix_mdev->kvm, false);

This call clears the apie in KVM.
This is only OK if we have a single device present until the end of the VM,
otherwise AP instructions in the guest will fail after the release until 
the end of the VM
or until a new device is plugged.


> +	vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
> +				 &matrix_mdev->group_notifier);
> +}
> +
>   static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
>   {
>   	return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
> @@ -754,6 +802,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 f248faf..48e2806 100644
> --- a/drivers/s390/crypto/vfio_ap_private.h
> +++ b/drivers/s390/crypto/vfio_ap_private.h
> @@ -31,6 +31,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)


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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ