[v2,7/8] s390: vfio-ap: handle bind and unbind of AP queue device
diff mbox series

Message ID 1555796980-27920-8-git-send-email-akrowiak@linux.ibm.com
State New
Headers show
Series
  • s390: vfio-ap: dynamic configuration support
Related show

Commit Message

Tony Krowiak April 20, 2019, 9:49 p.m. UTC
There is an implied guarantee that when an AP queue device is bound to a
device driver, that driver will have exclusive control over the device.
When an AP queue device is unbound from the vfio_ap driver while the
queue is in use by a guest and subsquently bound to a zcrypt driver, the
guarantee that the zcrypt driver has exclusive control of the queue
device will be violated. Likewise, when an AP queue device is bound to
the vfio_ap device driver and its APQN is assigned to an mdev device in
use by a guest, the expectation is that the guest will have access to
the queue.

The purpose of this patch is to ensure three things:

1. When an AP queue device in use by a guest is unbound, the guest shall
   no longer access the queue. Due to the limitations of the
   architecture, access to a single queue can not be denied to a guest,
   so access to the AP card to which the queue is connected will be
   denied to the guest.

2. When an AP queue device with an APQN assigned to an mdev device is
   bound to the vfio_ap driver while the mdev device is in use by a guest,
   the guest shall be granted access to the queue.

3. When a guest is started, each APQN assigned to the guest's mdev device
   must be owned by the vfio_ap driver.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
---
 drivers/s390/crypto/vfio_ap_drv.c     | 16 ++++++-
 drivers/s390/crypto/vfio_ap_ops.c     | 82 ++++++++++++++++++++++++++++++++++-
 drivers/s390/crypto/vfio_ap_private.h |  2 +
 3 files changed, 97 insertions(+), 3 deletions(-)

Comments

Pierre Morel April 23, 2019, 1:08 p.m. UTC | #1
On 20/04/2019 23:49, Tony Krowiak wrote:
> There is an implied guarantee that when an AP queue device is bound to a
> device driver, that driver will have exclusive control over the device.
> When an AP queue device is unbound from the vfio_ap driver while the
> queue is in use by a guest and subsquently bound to a zcrypt driver, the
> guarantee that the zcrypt driver has exclusive control of the queue
> device will be violated. Likewise, when an AP queue device is bound to
> the vfio_ap device driver and its APQN is assigned to an mdev device in
> use by a guest, the expectation is that the guest will have access to
> the queue.
> 
> The purpose of this patch is to ensure three things:
> 
> 1. When an AP queue device in use by a guest is unbound, the guest shall
>     no longer access the queue. Due to the limitations of the
>     architecture, access to a single queue can not be denied to a guest,
>     so access to the AP card to which the queue is connected will be
>     denied to the guest.
> 
> 2. When an AP queue device with an APQN assigned to an mdev device is
>     bound to the vfio_ap driver while the mdev device is in use by a guest,
>     the guest shall be granted access to the queue.
> 
> 3. When a guest is started, each APQN assigned to the guest's mdev device
>     must be owned by the vfio_ap driver.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> ---
>   drivers/s390/crypto/vfio_ap_drv.c     | 16 ++++++-
>   drivers/s390/crypto/vfio_ap_ops.c     | 82 ++++++++++++++++++++++++++++++++++-
>   drivers/s390/crypto/vfio_ap_private.h |  2 +
>   3 files changed, 97 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
> index e9824c35c34f..0f5dafddf5b1 100644
> --- a/drivers/s390/crypto/vfio_ap_drv.c
> +++ b/drivers/s390/crypto/vfio_ap_drv.c
> @@ -42,12 +42,26 @@ MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
>   
>   static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
>   {
> +	struct ap_queue *apq = to_ap_queue(&apdev->device);
> +	unsigned long apid = AP_QID_CARD(apq->qid);
> +	unsigned long apqi = AP_QID_QUEUE(apq->qid);
> +
> +	mutex_lock(&matrix_dev->lock);
> +	vfio_ap_mdev_probe_queue(apid, apqi);
> +	mutex_unlock(&matrix_dev->lock);
> +
>   	return 0;
>   }
>   
>   static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
>   {
> -	/* Nothing to do yet */
> +	struct ap_queue *apq = to_ap_queue(&apdev->device);
> +	unsigned long apid = AP_QID_CARD(apq->qid);
> +	unsigned long apqi = AP_QID_QUEUE(apq->qid);
> +
> +	mutex_lock(&matrix_dev->lock);
> +	vfio_ap_mdev_remove_queue(apid, apqi);
> +	mutex_unlock(&matrix_dev->lock);
>   }
>   
>   static void vfio_ap_matrix_dev_release(struct device *dev)
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 31ce30ee784d..3f9506516545 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -763,8 +763,8 @@ static void vfio_ap_mdev_wait_for_qempty(unsigned long apid, unsigned long apqi)
>   			msleep(20);
>   			break;
>   		default:
> -			pr_warn("%s: tapq err %02x: 0x%04x may not be empty\n",
> -				__func__, status.response_code, q->apqn);
> +			pr_warn("%s: tapq err %02x: %02lx%04lx may not be empty\n",
> +				__func__, status.response_code, apid, apqi);
>   			return;
>   		}
>   	} while (--retry);
> @@ -823,6 +823,14 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
>   
>   static int vfio_ap_mdev_create_shadow_crycb(struct ap_matrix_mdev *matrix_mdev)
>   {
> +	/*
> +	 * If an AP resource is not owned by the vfio_ap driver - e.g., it was
> +	 * unbound from the driver while still assigned to the mdev device
> +	 */
> +	if (ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
> +					       matrix_mdev->matrix.aqm))
> +		return -EADDRNOTAVAIL;
> +
>   	matrix_mdev->shadow_crycb = kzalloc(sizeof(*matrix_mdev->shadow_crycb),
>   					    GFP_KERNEL);
>   	if (!matrix_mdev->shadow_crycb)
> @@ -847,6 +855,18 @@ static int vfio_ap_mdev_open(struct mdev_device *mdev)
>   	if (!try_module_get(THIS_MODULE))
>   		return -ENODEV;
>   
> +	ret = ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
> +						 matrix_mdev->matrix.aqm);
> +
> +	/*
> +	 * If any APQN is owned by a default driver, it can not be used by the
> +	 * guest. This can happen if a queue is unbound from the vfio_ap
> +	 * driver but not unassigned from the mdev device.
> +	 */
> +	ret = (ret == 1) ? -EADDRNOTAVAIL : ret;
> +	if (ret)
> +		return ret;
> +
>   	matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
>   	events = VFIO_GROUP_NOTIFY_SET_KVM;
>   
> @@ -938,3 +958,61 @@ void vfio_ap_mdev_unregister(void)
>   {
>   	mdev_unregister_device(&matrix_dev->device);
>   }
> +
> +static struct ap_matrix_mdev *vfio_ap_mdev_find_matrix_mdev(unsigned long apid,
> +							    unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
> +		if (test_bit_inv(apid, matrix_mdev->matrix.apm) &&
> +		    test_bit_inv(apqi, matrix_mdev->matrix.aqm))
> +			return matrix_mdev;
> +	}
> +
> +	return NULL;
> +}
> +
> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
> +
> +	/*
> +	 * If the queue is assigned to the mdev device and the mdev device
> +	 * is in use by a guest
> +	 */
> +	if (matrix_mdev && matrix_mdev->kvm) {
> +		/*
> +		 * Unplug the adapter from the guest but don't unassign it
> +		 * from the mdev device
> +		 */
> +		clear_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
> +		vfio_ap_mdev_update_crycb(matrix_mdev);
> +	}
> +
> +	vfio_ap_mdev_reset_queue(apid, apqi);
> +}
> +
> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
> +
> +	/*
> +	 * If the queue is assigned to the mdev device and the mdev device
> +	 * is in use by a guest
> +	 */
> +	if (matrix_mdev && matrix_mdev->kvm) {
> +		/* Plug the adapter into the guest */
> +		set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
> +
> +		/* Make sure the queue is also plugged in to the guest */
> +		if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
> +			set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);

Why are you testing the domain before setting it and not the adapter?

Eventually you do not need to test at all or if, then both.

NIT


> +
> +		vfio_ap_mdev_update_crycb(matrix_mdev);
> +	}
> +}
> diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
> index e8457aa61976..00eaae3b853f 100644
> --- a/drivers/s390/crypto/vfio_ap_private.h
> +++ b/drivers/s390/crypto/vfio_ap_private.h
> @@ -87,5 +87,7 @@ struct ap_matrix_mdev {
>   
>   extern int vfio_ap_mdev_register(void);
>   extern void vfio_ap_mdev_unregister(void);
> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi);
> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi);
>   
>   #endif /* _VFIO_AP_PRIVATE_H_ */
>
Tony Krowiak April 23, 2019, 1:36 p.m. UTC | #2
On 4/23/19 9:08 AM, Pierre Morel wrote:
> On 20/04/2019 23:49, Tony Krowiak wrote:
>> There is an implied guarantee that when an AP queue device is bound to a
>> device driver, that driver will have exclusive control over the device.
>> When an AP queue device is unbound from the vfio_ap driver while the
>> queue is in use by a guest and subsquently bound to a zcrypt driver, the
>> guarantee that the zcrypt driver has exclusive control of the queue
>> device will be violated. Likewise, when an AP queue device is bound to
>> the vfio_ap device driver and its APQN is assigned to an mdev device in
>> use by a guest, the expectation is that the guest will have access to
>> the queue.
>>
>> The purpose of this patch is to ensure three things:
>>
>> 1. When an AP queue device in use by a guest is unbound, the guest shall
>>     no longer access the queue. Due to the limitations of the
>>     architecture, access to a single queue can not be denied to a guest,
>>     so access to the AP card to which the queue is connected will be
>>     denied to the guest.
>>
>> 2. When an AP queue device with an APQN assigned to an mdev device is
>>     bound to the vfio_ap driver while the mdev device is in use by a 
>> guest,
>>     the guest shall be granted access to the queue.
>>
>> 3. When a guest is started, each APQN assigned to the guest's mdev device
>>     must be owned by the vfio_ap driver.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   drivers/s390/crypto/vfio_ap_drv.c     | 16 ++++++-
>>   drivers/s390/crypto/vfio_ap_ops.c     | 82 
>> ++++++++++++++++++++++++++++++++++-
>>   drivers/s390/crypto/vfio_ap_private.h |  2 +
>>   3 files changed, 97 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
>> b/drivers/s390/crypto/vfio_ap_drv.c
>> index e9824c35c34f..0f5dafddf5b1 100644
>> --- a/drivers/s390/crypto/vfio_ap_drv.c
>> +++ b/drivers/s390/crypto/vfio_ap_drv.c
>> @@ -42,12 +42,26 @@ MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
>>   static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
>>   {
>> +    struct ap_queue *apq = to_ap_queue(&apdev->device);
>> +    unsigned long apid = AP_QID_CARD(apq->qid);
>> +    unsigned long apqi = AP_QID_QUEUE(apq->qid);
>> +
>> +    mutex_lock(&matrix_dev->lock);
>> +    vfio_ap_mdev_probe_queue(apid, apqi);
>> +    mutex_unlock(&matrix_dev->lock);
>> +
>>       return 0;
>>   }
>>   static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
>>   {
>> -    /* Nothing to do yet */
>> +    struct ap_queue *apq = to_ap_queue(&apdev->device);
>> +    unsigned long apid = AP_QID_CARD(apq->qid);
>> +    unsigned long apqi = AP_QID_QUEUE(apq->qid);
>> +
>> +    mutex_lock(&matrix_dev->lock);
>> +    vfio_ap_mdev_remove_queue(apid, apqi);
>> +    mutex_unlock(&matrix_dev->lock);
>>   }
>>   static void vfio_ap_matrix_dev_release(struct device *dev)
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
>> b/drivers/s390/crypto/vfio_ap_ops.c
>> index 31ce30ee784d..3f9506516545 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -763,8 +763,8 @@ static void vfio_ap_mdev_wait_for_qempty(unsigned 
>> long apid, unsigned long apqi)
>>               msleep(20);
>>               break;
>>           default:
>> -            pr_warn("%s: tapq err %02x: 0x%04x may not be empty\n",
>> -                __func__, status.response_code, q->apqn);
>> +            pr_warn("%s: tapq err %02x: %02lx%04lx may not be empty\n",
>> +                __func__, status.response_code, apid, apqi);
>>               return;
>>           }
>>       } while (--retry);
>> @@ -823,6 +823,14 @@ static int vfio_ap_mdev_reset_queues(struct 
>> mdev_device *mdev)
>>   static int vfio_ap_mdev_create_shadow_crycb(struct ap_matrix_mdev 
>> *matrix_mdev)
>>   {
>> +    /*
>> +     * If an AP resource is not owned by the vfio_ap driver - e.g., 
>> it was
>> +     * unbound from the driver while still assigned to the mdev device
>> +     */
>> +    if (ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
>> +                           matrix_mdev->matrix.aqm))
>> +        return -EADDRNOTAVAIL;
>> +
>>       matrix_mdev->shadow_crycb = 
>> kzalloc(sizeof(*matrix_mdev->shadow_crycb),
>>                           GFP_KERNEL);
>>       if (!matrix_mdev->shadow_crycb)
>> @@ -847,6 +855,18 @@ static int vfio_ap_mdev_open(struct mdev_device 
>> *mdev)
>>       if (!try_module_get(THIS_MODULE))
>>           return -ENODEV;
>> +    ret = ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
>> +                         matrix_mdev->matrix.aqm);
>> +
>> +    /*
>> +     * If any APQN is owned by a default driver, it can not be used 
>> by the
>> +     * guest. This can happen if a queue is unbound from the vfio_ap
>> +     * driver but not unassigned from the mdev device.
>> +     */
>> +    ret = (ret == 1) ? -EADDRNOTAVAIL : ret;
>> +    if (ret)
>> +        return ret;
>> +
>>       matrix_mdev->group_notifier.notifier_call = 
>> vfio_ap_mdev_group_notifier;
>>       events = VFIO_GROUP_NOTIFY_SET_KVM;
>> @@ -938,3 +958,61 @@ void vfio_ap_mdev_unregister(void)
>>   {
>>       mdev_unregister_device(&matrix_dev->device);
>>   }
>> +
>> +static struct ap_matrix_mdev *vfio_ap_mdev_find_matrix_mdev(unsigned 
>> long apid,
>> +                                unsigned long apqi)
>> +{
>> +    struct ap_matrix_mdev *matrix_mdev;
>> +
>> +    list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
>> +        if (test_bit_inv(apid, matrix_mdev->matrix.apm) &&
>> +            test_bit_inv(apqi, matrix_mdev->matrix.aqm))
>> +            return matrix_mdev;
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi)
>> +{
>> +    struct ap_matrix_mdev *matrix_mdev;
>> +
>> +    matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
>> +
>> +    /*
>> +     * If the queue is assigned to the mdev device and the mdev device
>> +     * is in use by a guest
>> +     */
>> +    if (matrix_mdev && matrix_mdev->kvm) {
>> +        /*
>> +         * Unplug the adapter from the guest but don't unassign it
>> +         * from the mdev device
>> +         */
>> +        clear_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
>> +        vfio_ap_mdev_update_crycb(matrix_mdev);
>> +    }
>> +
>> +    vfio_ap_mdev_reset_queue(apid, apqi);
>> +}
>> +
>> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
>> +{
>> +    struct ap_matrix_mdev *matrix_mdev;
>> +
>> +    matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
>> +
>> +    /*
>> +     * If the queue is assigned to the mdev device and the mdev device
>> +     * is in use by a guest
>> +     */
>> +    if (matrix_mdev && matrix_mdev->kvm) {
>> +        /* Plug the adapter into the guest */
>> +        set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
>> +
>> +        /* Make sure the queue is also plugged in to the guest */
>> +        if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
>> +            set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);
> 
> Why are you testing the domain before setting it and not the adapter?
> 
> Eventually you do not need to test at all or if, then both.

My thinking was that when a queue is removed, we clear only the APID
from the APM, but do not clear the APQI from the AQM. I think you are
correct in saying we do not need to test either mask since we are
plugging the queue in regardless.

> 
> NIT
> 
> 
>> +
>> +        vfio_ap_mdev_update_crycb(matrix_mdev);
>> +    }
>> +}
>> diff --git a/drivers/s390/crypto/vfio_ap_private.h 
>> b/drivers/s390/crypto/vfio_ap_private.h
>> index e8457aa61976..00eaae3b853f 100644
>> --- a/drivers/s390/crypto/vfio_ap_private.h
>> +++ b/drivers/s390/crypto/vfio_ap_private.h
>> @@ -87,5 +87,7 @@ struct ap_matrix_mdev {
>>   extern int vfio_ap_mdev_register(void);
>>   extern void vfio_ap_mdev_unregister(void);
>> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi);
>> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi);
>>   #endif /* _VFIO_AP_PRIVATE_H_ */
>>
> 
>
Pierre Morel April 23, 2019, 1:38 p.m. UTC | #3
On 20/04/2019 23:49, Tony Krowiak wrote:
> There is an implied guarantee that when an AP queue device is bound to a
> device driver, that driver will have exclusive control over the device.
> When an AP queue device is unbound from the vfio_ap driver while the
> queue is in use by a guest and subsquently bound to a zcrypt driver, the
> guarantee that the zcrypt driver has exclusive control of the queue
> device will be violated. Likewise, when an AP queue device is bound to
> the vfio_ap device driver and its APQN is assigned to an mdev device in
> use by a guest, the expectation is that the guest will have access to
> the queue.
> 
> The purpose of this patch is to ensure three things:
> 
> 1. When an AP queue device in use by a guest is unbound, the guest shall
>     no longer access the queue. Due to the limitations of the
>     architecture, access to a single queue can not be denied to a guest,
>     so access to the AP card to which the queue is connected will be
>     denied to the guest.
> 
> 2. When an AP queue device with an APQN assigned to an mdev device is
>     bound to the vfio_ap driver while the mdev device is in use by a guest,
>     the guest shall be granted access to the queue.
> 
> 3. When a guest is started, each APQN assigned to the guest's mdev device
>     must be owned by the vfio_ap driver.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> ---
>   drivers/s390/crypto/vfio_ap_drv.c     | 16 ++++++-
>   drivers/s390/crypto/vfio_ap_ops.c     | 82 ++++++++++++++++++++++++++++++++++-
>   drivers/s390/crypto/vfio_ap_private.h |  2 +
>   3 files changed, 97 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
> index e9824c35c34f..0f5dafddf5b1 100644
> --- a/drivers/s390/crypto/vfio_ap_drv.c
> +++ b/drivers/s390/crypto/vfio_ap_drv.c
> @@ -42,12 +42,26 @@ MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
>   
>   static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
>   {
> +	struct ap_queue *apq = to_ap_queue(&apdev->device);
> +	unsigned long apid = AP_QID_CARD(apq->qid);
> +	unsigned long apqi = AP_QID_QUEUE(apq->qid);
> +
> +	mutex_lock(&matrix_dev->lock);
> +	vfio_ap_mdev_probe_queue(apid, apqi);
> +	mutex_unlock(&matrix_dev->lock);
> +
>   	return 0;
>   }
>   
>   static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
>   {
> -	/* Nothing to do yet */
> +	struct ap_queue *apq = to_ap_queue(&apdev->device);
> +	unsigned long apid = AP_QID_CARD(apq->qid);
> +	unsigned long apqi = AP_QID_QUEUE(apq->qid);
> +
> +	mutex_lock(&matrix_dev->lock);
> +	vfio_ap_mdev_remove_queue(apid, apqi);
> +	mutex_unlock(&matrix_dev->lock);
>   }
>   
>   static void vfio_ap_matrix_dev_release(struct device *dev)
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 31ce30ee784d..3f9506516545 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -763,8 +763,8 @@ static void vfio_ap_mdev_wait_for_qempty(unsigned long apid, unsigned long apqi)
>   			msleep(20);
>   			break;
>   		default:
> -			pr_warn("%s: tapq err %02x: 0x%04x may not be empty\n",
> -				__func__, status.response_code, q->apqn);
> +			pr_warn("%s: tapq err %02x: %02lx%04lx may not be empty\n",
> +				__func__, status.response_code, apid, apqi);
>   			return;
>   		}
>   	} while (--retry);
> @@ -823,6 +823,14 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
>   
>   static int vfio_ap_mdev_create_shadow_crycb(struct ap_matrix_mdev *matrix_mdev)
>   {
> +	/*
> +	 * If an AP resource is not owned by the vfio_ap driver - e.g., it was
> +	 * unbound from the driver while still assigned to the mdev device
> +	 */
> +	if (ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
> +					       matrix_mdev->matrix.aqm))
> +		return -EADDRNOTAVAIL;
> +
>   	matrix_mdev->shadow_crycb = kzalloc(sizeof(*matrix_mdev->shadow_crycb),
>   					    GFP_KERNEL);
>   	if (!matrix_mdev->shadow_crycb)
> @@ -847,6 +855,18 @@ static int vfio_ap_mdev_open(struct mdev_device *mdev)
>   	if (!try_module_get(THIS_MODULE))
>   		return -ENODEV;
>   
> +	ret = ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
> +						 matrix_mdev->matrix.aqm);
> +
> +	/*
> +	 * If any APQN is owned by a default driver, it can not be used by the
> +	 * guest. This can happen if a queue is unbound from the vfio_ap
> +	 * driver but not unassigned from the mdev device.
> +	 */
> +	ret = (ret == 1) ? -EADDRNOTAVAIL : ret;
> +	if (ret)
> +		return ret;
> +
>   	matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
>   	events = VFIO_GROUP_NOTIFY_SET_KVM;
>   
> @@ -938,3 +958,61 @@ void vfio_ap_mdev_unregister(void)
>   {
>   	mdev_unregister_device(&matrix_dev->device);
>   }
> +
> +static struct ap_matrix_mdev *vfio_ap_mdev_find_matrix_mdev(unsigned long apid,
> +							    unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
> +		if (test_bit_inv(apid, matrix_mdev->matrix.apm) &&
> +		    test_bit_inv(apqi, matrix_mdev->matrix.aqm))
> +			return matrix_mdev;
> +	}
> +
> +	return NULL;
> +}
> +
> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
> +
> +	/*
> +	 * If the queue is assigned to the mdev device and the mdev device
> +	 * is in use by a guest
> +	 */
> +	if (matrix_mdev && matrix_mdev->kvm) {
> +		/*
> +		 * Unplug the adapter from the guest but don't unassign it
> +		 * from the mdev device
> +		 */
> +		clear_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
> +		vfio_ap_mdev_update_crycb(matrix_mdev);
> +	}
> +
> +	vfio_ap_mdev_reset_queue(apid, apqi);
> +}
> +
> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
> +
> +	/*
> +	 * If the queue is assigned to the mdev device and the mdev device
> +	 * is in use by a guest
> +	 */
> +	if (matrix_mdev && matrix_mdev->kvm) {
> +		/* Plug the adapter into the guest */
> +		set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);

Before you set the bit in the shadow...

> +
> +		/* Make sure the queue is also plugged in to the guest */

I Think we must take care in the use of queues and domains to avoid 
being confusing.

> +		if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
> +			set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);
> +
> +		vfio_ap_mdev_update_crycb(matrix_mdev);

...and you update the real CRYCB,

don't you need to verify that all ap queues which verify APID and any 
already pre-existing APQI are bound to the driver?

Same for pre-existing APID if you set the APQI.


> +	}
> +}
> diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
> index e8457aa61976..00eaae3b853f 100644
> --- a/drivers/s390/crypto/vfio_ap_private.h
> +++ b/drivers/s390/crypto/vfio_ap_private.h
> @@ -87,5 +87,7 @@ struct ap_matrix_mdev {
>   
>   extern int vfio_ap_mdev_register(void);
>   extern void vfio_ap_mdev_unregister(void);
> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi);
> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi);
>   
>   #endif /* _VFIO_AP_PRIVATE_H_ */
>
Halil Pasic April 23, 2019, 1:54 p.m. UTC | #4
On Sat, 20 Apr 2019 17:49:39 -0400
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
> +{
> +	struct ap_matrix_mdev *matrix_mdev;
> +
> +	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
> +
> +	/*
> +	 * If the queue is assigned to the mdev device and the mdev device
> +	 * is in use by a guest
> +	 */
> +	if (matrix_mdev && matrix_mdev->kvm) {
> +		/* Plug the adapter into the guest */
> +		set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
> +
> +		/* Make sure the queue is also plugged in to the guest */
> +		if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
> +			set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);
> +
> +		vfio_ap_mdev_update_crycb(matrix_mdev);

With this you effectively grant access to all the assigned domains on
the AP identified by the apid, not only to the domain identified by
apqi! But some of these queues may still not be bound to the vfio_ap
driver.

IMHO you should only set the apid-th bit in apm if all queues (apid, q)
such that q-th bit is set in aqm are bound to the vfio_ap driver.

BTW a 'shadow' (or effective) apm would perfectly suffice. I don't think
you fiddle with shadow_crycb->a[qd]m, and if you do, I don't think that's
a good idea.

Regards,
Halil

> +	}
> +}
Tony Krowiak April 23, 2019, 2:53 p.m. UTC | #5
On 4/23/19 9:38 AM, Pierre Morel wrote:
> On 20/04/2019 23:49, Tony Krowiak wrote:
>> There is an implied guarantee that when an AP queue device is bound to a
>> device driver, that driver will have exclusive control over the device.
>> When an AP queue device is unbound from the vfio_ap driver while the
>> queue is in use by a guest and subsquently bound to a zcrypt driver, the
>> guarantee that the zcrypt driver has exclusive control of the queue
>> device will be violated. Likewise, when an AP queue device is bound to
>> the vfio_ap device driver and its APQN is assigned to an mdev device in
>> use by a guest, the expectation is that the guest will have access to
>> the queue.
>>
>> The purpose of this patch is to ensure three things:
>>
>> 1. When an AP queue device in use by a guest is unbound, the guest shall
>>     no longer access the queue. Due to the limitations of the
>>     architecture, access to a single queue can not be denied to a guest,
>>     so access to the AP card to which the queue is connected will be
>>     denied to the guest.
>>
>> 2. When an AP queue device with an APQN assigned to an mdev device is
>>     bound to the vfio_ap driver while the mdev device is in use by a 
>> guest,
>>     the guest shall be granted access to the queue.
>>
>> 3. When a guest is started, each APQN assigned to the guest's mdev device
>>     must be owned by the vfio_ap driver.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   drivers/s390/crypto/vfio_ap_drv.c     | 16 ++++++-
>>   drivers/s390/crypto/vfio_ap_ops.c     | 82 
>> ++++++++++++++++++++++++++++++++++-
>>   drivers/s390/crypto/vfio_ap_private.h |  2 +
>>   3 files changed, 97 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
>> b/drivers/s390/crypto/vfio_ap_drv.c
>> index e9824c35c34f..0f5dafddf5b1 100644
>> --- a/drivers/s390/crypto/vfio_ap_drv.c
>> +++ b/drivers/s390/crypto/vfio_ap_drv.c
>> @@ -42,12 +42,26 @@ MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
>>   static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
>>   {
>> +    struct ap_queue *apq = to_ap_queue(&apdev->device);
>> +    unsigned long apid = AP_QID_CARD(apq->qid);
>> +    unsigned long apqi = AP_QID_QUEUE(apq->qid);
>> +
>> +    mutex_lock(&matrix_dev->lock);
>> +    vfio_ap_mdev_probe_queue(apid, apqi);
>> +    mutex_unlock(&matrix_dev->lock);
>> +
>>       return 0;
>>   }
>>   static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
>>   {
>> -    /* Nothing to do yet */
>> +    struct ap_queue *apq = to_ap_queue(&apdev->device);
>> +    unsigned long apid = AP_QID_CARD(apq->qid);
>> +    unsigned long apqi = AP_QID_QUEUE(apq->qid);
>> +
>> +    mutex_lock(&matrix_dev->lock);
>> +    vfio_ap_mdev_remove_queue(apid, apqi);
>> +    mutex_unlock(&matrix_dev->lock);
>>   }
>>   static void vfio_ap_matrix_dev_release(struct device *dev)
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
>> b/drivers/s390/crypto/vfio_ap_ops.c
>> index 31ce30ee784d..3f9506516545 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -763,8 +763,8 @@ static void vfio_ap_mdev_wait_for_qempty(unsigned 
>> long apid, unsigned long apqi)
>>               msleep(20);
>>               break;
>>           default:
>> -            pr_warn("%s: tapq err %02x: 0x%04x may not be empty\n",
>> -                __func__, status.response_code, q->apqn);
>> +            pr_warn("%s: tapq err %02x: %02lx%04lx may not be empty\n",
>> +                __func__, status.response_code, apid, apqi);
>>               return;
>>           }
>>       } while (--retry);
>> @@ -823,6 +823,14 @@ static int vfio_ap_mdev_reset_queues(struct 
>> mdev_device *mdev)
>>   static int vfio_ap_mdev_create_shadow_crycb(struct ap_matrix_mdev 
>> *matrix_mdev)
>>   {
>> +    /*
>> +     * If an AP resource is not owned by the vfio_ap driver - e.g., 
>> it was
>> +     * unbound from the driver while still assigned to the mdev device
>> +     */
>> +    if (ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
>> +                           matrix_mdev->matrix.aqm))
>> +        return -EADDRNOTAVAIL;
>> +
>>       matrix_mdev->shadow_crycb = 
>> kzalloc(sizeof(*matrix_mdev->shadow_crycb),
>>                           GFP_KERNEL);
>>       if (!matrix_mdev->shadow_crycb)
>> @@ -847,6 +855,18 @@ static int vfio_ap_mdev_open(struct mdev_device 
>> *mdev)
>>       if (!try_module_get(THIS_MODULE))
>>           return -ENODEV;
>> +    ret = ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
>> +                         matrix_mdev->matrix.aqm);
>> +
>> +    /*
>> +     * If any APQN is owned by a default driver, it can not be used 
>> by the
>> +     * guest. This can happen if a queue is unbound from the vfio_ap
>> +     * driver but not unassigned from the mdev device.
>> +     */
>> +    ret = (ret == 1) ? -EADDRNOTAVAIL : ret;
>> +    if (ret)
>> +        return ret;
>> +
>>       matrix_mdev->group_notifier.notifier_call = 
>> vfio_ap_mdev_group_notifier;
>>       events = VFIO_GROUP_NOTIFY_SET_KVM;
>> @@ -938,3 +958,61 @@ void vfio_ap_mdev_unregister(void)
>>   {
>>       mdev_unregister_device(&matrix_dev->device);
>>   }
>> +
>> +static struct ap_matrix_mdev *vfio_ap_mdev_find_matrix_mdev(unsigned 
>> long apid,
>> +                                unsigned long apqi)
>> +{
>> +    struct ap_matrix_mdev *matrix_mdev;
>> +
>> +    list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
>> +        if (test_bit_inv(apid, matrix_mdev->matrix.apm) &&
>> +            test_bit_inv(apqi, matrix_mdev->matrix.aqm))
>> +            return matrix_mdev;
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi)
>> +{
>> +    struct ap_matrix_mdev *matrix_mdev;
>> +
>> +    matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
>> +
>> +    /*
>> +     * If the queue is assigned to the mdev device and the mdev device
>> +     * is in use by a guest
>> +     */
>> +    if (matrix_mdev && matrix_mdev->kvm) {
>> +        /*
>> +         * Unplug the adapter from the guest but don't unassign it
>> +         * from the mdev device
>> +         */
>> +        clear_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
>> +        vfio_ap_mdev_update_crycb(matrix_mdev);
>> +    }
>> +
>> +    vfio_ap_mdev_reset_queue(apid, apqi);
>> +}
>> +
>> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
>> +{
>> +    struct ap_matrix_mdev *matrix_mdev;
>> +
>> +    matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
>> +
>> +    /*
>> +     * If the queue is assigned to the mdev device and the mdev device
>> +     * is in use by a guest
>> +     */
>> +    if (matrix_mdev && matrix_mdev->kvm) {
>> +        /* Plug the adapter into the guest */
>> +        set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
> 
> Before you set the bit in the shadow...

yes

> 
>> +
>> +        /* Make sure the queue is also plugged in to the guest */
> 
> I Think we must take care in the use of queues and domains to avoid 
> being confusing.

Duly noted.

> 
>> +        if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
>> +            set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);
>> +
>> +        vfio_ap_mdev_update_crycb(matrix_mdev);
> 
> ...and you update the real CRYCB,
> 
> don't you need to verify that all ap queues which verify APID and any 
> already pre-existing APQI are bound to the driver?
> 
> Same for pre-existing APID if you set the APQI.

Since I last responded to your comment, I've been doing some testing
and discovered some scenarios that need to be considered. There is
definitely some additional checking that needs to be done here. It is
not necessary to verify all queues are bound to the vfio_ap driver, but
it we must assure that no queues are reserved for use
by the zcrypt drivers (i.e., APQN set in the apmask/aqmask).

> 
> 
>> +    }
>> +}
>> diff --git a/drivers/s390/crypto/vfio_ap_private.h 
>> b/drivers/s390/crypto/vfio_ap_private.h
>> index e8457aa61976..00eaae3b853f 100644
>> --- a/drivers/s390/crypto/vfio_ap_private.h
>> +++ b/drivers/s390/crypto/vfio_ap_private.h
>> @@ -87,5 +87,7 @@ struct ap_matrix_mdev {
>>   extern int vfio_ap_mdev_register(void);
>>   extern void vfio_ap_mdev_unregister(void);
>> +void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi);
>> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi);
>>   #endif /* _VFIO_AP_PRIVATE_H_ */
>>
> 
>
Tony Krowiak April 23, 2019, 3:27 p.m. UTC | #6
On 4/23/19 9:54 AM, Halil Pasic wrote:
> On Sat, 20 Apr 2019 17:49:39 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> 
>> +void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
>> +{
>> +	struct ap_matrix_mdev *matrix_mdev;
>> +
>> +	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
>> +
>> +	/*
>> +	 * If the queue is assigned to the mdev device and the mdev device
>> +	 * is in use by a guest
>> +	 */
>> +	if (matrix_mdev && matrix_mdev->kvm) {
>> +		/* Plug the adapter into the guest */
>> +		set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
>> +
>> +		/* Make sure the queue is also plugged in to the guest */
>> +		if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
>> +			set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);
>> +
>> +		vfio_ap_mdev_update_crycb(matrix_mdev);
> 
> With this you effectively grant access to all the assigned domains on
> the AP identified by the apid, not only to the domain identified by
> apqi! But some of these queues may still not be bound to the vfio_ap
> driver.

I have been doing some more testing since I last visited and discovered
that there needs to be additional checking here.

> 
> IMHO you should only set the apid-th bit in apm if all queues (apid, q)
> such that q-th bit is set in aqm are bound to the vfio_ap driver.

This is partially correct. It is not necessary to verify that the
affected queues are bound to the vfio_ap driver. It is only necessary
to verify they are not reserved for use by a zcrypt driver since we
are allowing assignment of APQNs for queues that are not available (see
patch 3/8).

> 
> BTW a 'shadow' (or effective) apm would perfectly suffice. I don't think
> you fiddle with shadow_crycb->a[qd]m, and if you do, I don't think that's
> a good idea.

I do not think it is accurate to refer to the APM in the shadow CRYCB as
an effective mask. Effective masking is a firmware construct. The CRYCB
of the guest may be configured with APQNs that are not available.

The shadow CRYCB is in fact a copy of the guest CRYCB. Whenever the
masks in the guest CRYCB are set, they are set from the masks in the
shadow CRYCB. The lifespan of the shadow CRYCB is synonymous with the
lifespan of the guest.

Each of the mdev device sysfs assignment/unassignment interfaces does
fiddle with the masks in the shadow CRYCB if a guest is using the mdev
device. This allows us to hot plug/unplug AP resources for the guest.
Recall that an adapter or domain can not be assigned unless each
new APQN created is NOT reserved for use by the zcrypt drivers via
the AP bus's apmask/aqmask sysfs interfaces, and the APQN is not 
assigned to any other mdev device. That is how protection is
provided against inadvertently sharing AP queues between guests or
the guest and the host. I do have to add that verification to
the vfio_ap_mdev_probe_queue function though.



> 
> Regards,
> Halil
> 
>> +	}
>> +}
>

Patch
diff mbox series

diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
index e9824c35c34f..0f5dafddf5b1 100644
--- a/drivers/s390/crypto/vfio_ap_drv.c
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -42,12 +42,26 @@  MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
 
 static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
 {
+	struct ap_queue *apq = to_ap_queue(&apdev->device);
+	unsigned long apid = AP_QID_CARD(apq->qid);
+	unsigned long apqi = AP_QID_QUEUE(apq->qid);
+
+	mutex_lock(&matrix_dev->lock);
+	vfio_ap_mdev_probe_queue(apid, apqi);
+	mutex_unlock(&matrix_dev->lock);
+
 	return 0;
 }
 
 static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
 {
-	/* Nothing to do yet */
+	struct ap_queue *apq = to_ap_queue(&apdev->device);
+	unsigned long apid = AP_QID_CARD(apq->qid);
+	unsigned long apqi = AP_QID_QUEUE(apq->qid);
+
+	mutex_lock(&matrix_dev->lock);
+	vfio_ap_mdev_remove_queue(apid, apqi);
+	mutex_unlock(&matrix_dev->lock);
 }
 
 static void vfio_ap_matrix_dev_release(struct device *dev)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 31ce30ee784d..3f9506516545 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -763,8 +763,8 @@  static void vfio_ap_mdev_wait_for_qempty(unsigned long apid, unsigned long apqi)
 			msleep(20);
 			break;
 		default:
-			pr_warn("%s: tapq err %02x: 0x%04x may not be empty\n",
-				__func__, status.response_code, q->apqn);
+			pr_warn("%s: tapq err %02x: %02lx%04lx may not be empty\n",
+				__func__, status.response_code, apid, apqi);
 			return;
 		}
 	} while (--retry);
@@ -823,6 +823,14 @@  static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
 
 static int vfio_ap_mdev_create_shadow_crycb(struct ap_matrix_mdev *matrix_mdev)
 {
+	/*
+	 * If an AP resource is not owned by the vfio_ap driver - e.g., it was
+	 * unbound from the driver while still assigned to the mdev device
+	 */
+	if (ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
+					       matrix_mdev->matrix.aqm))
+		return -EADDRNOTAVAIL;
+
 	matrix_mdev->shadow_crycb = kzalloc(sizeof(*matrix_mdev->shadow_crycb),
 					    GFP_KERNEL);
 	if (!matrix_mdev->shadow_crycb)
@@ -847,6 +855,18 @@  static int vfio_ap_mdev_open(struct mdev_device *mdev)
 	if (!try_module_get(THIS_MODULE))
 		return -ENODEV;
 
+	ret = ap_apqn_in_matrix_owned_by_def_drv(matrix_mdev->matrix.apm,
+						 matrix_mdev->matrix.aqm);
+
+	/*
+	 * If any APQN is owned by a default driver, it can not be used by the
+	 * guest. This can happen if a queue is unbound from the vfio_ap
+	 * driver but not unassigned from the mdev device.
+	 */
+	ret = (ret == 1) ? -EADDRNOTAVAIL : ret;
+	if (ret)
+		return ret;
+
 	matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
 	events = VFIO_GROUP_NOTIFY_SET_KVM;
 
@@ -938,3 +958,61 @@  void vfio_ap_mdev_unregister(void)
 {
 	mdev_unregister_device(&matrix_dev->device);
 }
+
+static struct ap_matrix_mdev *vfio_ap_mdev_find_matrix_mdev(unsigned long apid,
+							    unsigned long apqi)
+{
+	struct ap_matrix_mdev *matrix_mdev;
+
+	list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
+		if (test_bit_inv(apid, matrix_mdev->matrix.apm) &&
+		    test_bit_inv(apqi, matrix_mdev->matrix.aqm))
+			return matrix_mdev;
+	}
+
+	return NULL;
+}
+
+void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi)
+{
+	struct ap_matrix_mdev *matrix_mdev;
+
+	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
+
+	/*
+	 * If the queue is assigned to the mdev device and the mdev device
+	 * is in use by a guest
+	 */
+	if (matrix_mdev && matrix_mdev->kvm) {
+		/*
+		 * Unplug the adapter from the guest but don't unassign it
+		 * from the mdev device
+		 */
+		clear_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
+		vfio_ap_mdev_update_crycb(matrix_mdev);
+	}
+
+	vfio_ap_mdev_reset_queue(apid, apqi);
+}
+
+void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi)
+{
+	struct ap_matrix_mdev *matrix_mdev;
+
+	matrix_mdev = vfio_ap_mdev_find_matrix_mdev(apid, apqi);
+
+	/*
+	 * If the queue is assigned to the mdev device and the mdev device
+	 * is in use by a guest
+	 */
+	if (matrix_mdev && matrix_mdev->kvm) {
+		/* Plug the adapter into the guest */
+		set_bit_inv(apid, matrix_mdev->shadow_crycb->apm);
+
+		/* Make sure the queue is also plugged in to the guest */
+		if (!test_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm))
+			set_bit_inv(apqi, matrix_mdev->shadow_crycb->aqm);
+
+		vfio_ap_mdev_update_crycb(matrix_mdev);
+	}
+}
diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
index e8457aa61976..00eaae3b853f 100644
--- a/drivers/s390/crypto/vfio_ap_private.h
+++ b/drivers/s390/crypto/vfio_ap_private.h
@@ -87,5 +87,7 @@  struct ap_matrix_mdev {
 
 extern int vfio_ap_mdev_register(void);
 extern void vfio_ap_mdev_unregister(void);
+void vfio_ap_mdev_remove_queue(unsigned long apid, unsigned long apqi);
+void vfio_ap_mdev_probe_queue(unsigned long apid, unsigned long apqi);
 
 #endif /* _VFIO_AP_PRIVATE_H_ */