diff mbox series

[v19,05/20] s390/vfio-ap: refresh guest's APCB by filtering AP resources assigned to mdev

Message ID 20220404221039.1272245-6-akrowiak@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390/vfio-ap: dynamic configuration support | expand

Commit Message

Anthony Krowiak April 4, 2022, 10:10 p.m. UTC
Refresh the guest's APCB by filtering the APQNs and control domain numbers
assigned to the matrix mdev.

Filtering of APQNs:
-----------------
APQNs that do not reference an AP queue device bound to the vfio_ap device
driver must be filtered from the APQNs assigned to the matrix mdev before
they can be assigned to the guest's APCB. Given that the APQNs are
configured in the guest's APCB as a matrix of APIDs (adapters) and APQIs
(domains), it is not possible to filter an individual APQN. For example,
suppose the matrix of APQNs is structured as follows:

                   APIDs
             3      4      5
        0  (3,0)  (4,0)  (5,0)
APQIs   1  (3,1)  (4,1)  (5,1)
        2  (3,2)  (4,2)  (5,2)

Now suppose APQN (4,1) does not reference a queue device bound to the
vfio_ap device driver. If we filter APID 4, the APQNs (4,0), (4,1) and
(4,2) will be removed. Similarly, if we filter domain 1, APQNs (3,1),
(4,1) and (5,1) will be removed.

To resolve this dilemma, the choice was made to filter the APID - in this
case 4 - from the guest's APCB. The reason for this design decision is
because the APID references an AP adapter which is a real hardware device
that can be physically installed, removed, enabled or disabled; whereas, a
domain is a partition within the adapter. It therefore better reflects
reality to remove the APID from the guest's APCB.

Filtering of control domains:
----------------------------
Any control domains that are not assigned to the host's AP configuration
will be filtered from those assigned to the matrix mdev before assigning
them to the guest's APCB.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
---
 drivers/s390/crypto/vfio_ap_ops.c | 104 +++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 3 deletions(-)

Comments

Jason J. Herne May 16, 2022, 4:36 p.m. UTC | #1
On 4/4/22 18:10, Tony Krowiak wrote:
> |@@ -1306,8 +1392,6 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev, 
> kvm_get_kvm(kvm); matrix_mdev->kvm = kvm; - memcpy(&matrix_mdev->shadow_apcb, 
> &matrix_mdev->matrix, - sizeof(struct ap_matrix)); kvm_arch_crypto_set_masks(kvm, 
> matrix_mdev->shadow_apcb.apm, matrix_mdev->shadow_apcb.aqm, matrix_mdev->shadow_apcb.adm);|

This looks like an unrelated change. Does this snippet really belong to this patch?
Anthony Krowiak May 16, 2022, 5:13 p.m. UTC | #2
On 5/16/22 12:36 PM, Jason J. Herne wrote:
> On 4/4/22 18:10, Tony Krowiak wrote:
>> |@@ -1306,8 +1392,6 @@ static int vfio_ap_mdev_set_kvm(struct 
>> ap_matrix_mdev *matrix_mdev, kvm_get_kvm(kvm); matrix_mdev->kvm = 
>> kvm; - memcpy(&matrix_mdev->shadow_apcb, &matrix_mdev->matrix, - 
>> sizeof(struct ap_matrix)); kvm_arch_crypto_set_masks(kvm, 
>> matrix_mdev->shadow_apcb.apm, matrix_mdev->shadow_apcb.aqm, 
>> matrix_mdev->shadow_apcb.adm);|
>
> This looks like an unrelated change. Does this snippet really belong 
> to this patch?

It's kind of hard to tell which snippet you are talking about without 
the patch context, but I assume you are referring to the removal of the 
memcpy statement in the vfio_ap_mdev_set_kvm() function in which case 
this snippet belongs with this patch.

This patch introduces a function that filters the contents of the 
matrix_mdev->matrix to ensure that the matrix_mdev->shadow_apcb contains 
only queues that are bound to the vfio_ap device driver. The filtering 
function is called whenever an adapter, domain or control domain is 
assigned or unassigned, so it is no longer necessary to copy the 
contents of matrix_mdev->matrix into matrix_mdev->shadow_apcb before 
setting the masks in the guest; that will have already been done by the 
filter function.
Jason J. Herne May 16, 2022, 5:50 p.m. UTC | #3
On 5/16/22 13:13, Tony Krowiak wrote:
> 
> 
> On 5/16/22 12:36 PM, Jason J. Herne wrote:
>> On 4/4/22 18:10, Tony Krowiak wrote:
>>> |@@ -1306,8 +1392,6 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev 
>>> *matrix_mdev, kvm_get_kvm(kvm); matrix_mdev->kvm = kvm; - 
>>> memcpy(&matrix_mdev->shadow_apcb, &matrix_mdev->matrix, - sizeof(struct ap_matrix)); 
>>> kvm_arch_crypto_set_masks(kvm, matrix_mdev->shadow_apcb.apm, 
>>> matrix_mdev->shadow_apcb.aqm, matrix_mdev->shadow_apcb.adm);|
>>
>> This looks like an unrelated change. Does this snippet really belong to this patch?
> 
> It's kind of hard to tell which snippet you are talking about without the patch context, 
> but I assume you are referring to the removal of the memcpy statement in the 
> vfio_ap_mdev_set_kvm() function in which case this snippet belongs with this patch.
> 
> This patch introduces a function that filters the contents of the matrix_mdev->matrix to 
> ensure that the matrix_mdev->shadow_apcb contains only queues that are bound to the 
> vfio_ap device driver. The filtering function is called whenever an adapter, domain or 
> control domain is assigned or unassigned, so it is no longer necessary to copy the 
> contents of matrix_mdev->matrix into matrix_mdev->shadow_apcb before setting the masks in 
> the guest; that will have already been done by the filter function.
> 
> 

I was apparently a little overzealous with my trimming. Yes, you are correct. Thanks for
the explanation.

Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
Anthony Krowiak May 16, 2022, 6:06 p.m. UTC | #4
Thanks for the review

On 5/16/22 1:50 PM, Jason J. Herne wrote:
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
diff mbox series

Patch

diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 9c4a3ad5369a..e53e69a033b0 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -444,6 +444,68 @@  static void vfio_ap_matrix_init(struct ap_config_info *info,
 	matrix->adm_max = info->apxa ? info->Nd : 15;
 }
 
+static void vfio_ap_mdev_filter_cdoms(struct ap_matrix_mdev *matrix_mdev)
+{
+	bitmap_and(matrix_mdev->shadow_apcb.adm, matrix_mdev->matrix.adm,
+		   (unsigned long *)matrix_dev->info.adm, AP_DOMAINS);
+}
+
+/*
+ * vfio_ap_mdev_filter_matrix - filter the APQNs assigned to the matrix mdev
+ *				to ensure no queue devices are passed through to
+ *				the guest that are not bound to the vfio_ap
+ *				device driver.
+ *
+ * @matrix_mdev: the matrix mdev whose matrix is to be filtered.
+ *
+ * Note: If an APQN referencing a queue device that is not bound to the vfio_ap
+ *	 driver, its APID will be filtered from the guest's APCB. The matrix
+ *	 structure precludes filtering an individual APQN, so its APID will be
+ *	 filtered.
+ */
+static void vfio_ap_mdev_filter_matrix(unsigned long *apm, unsigned long *aqm,
+				       struct ap_matrix_mdev *matrix_mdev)
+{
+	int ret;
+	unsigned long apid, apqi, apqn;
+
+	ret = ap_qci(&matrix_dev->info);
+	if (ret)
+		return;
+
+	vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->shadow_apcb);
+
+	/*
+	 * Copy the adapters, domains and control domains to the shadow_apcb
+	 * from the matrix mdev, but only those that are assigned to the host's
+	 * AP configuration.
+	 */
+	bitmap_and(matrix_mdev->shadow_apcb.apm, matrix_mdev->matrix.apm,
+		   (unsigned long *)matrix_dev->info.apm, AP_DEVICES);
+	bitmap_and(matrix_mdev->shadow_apcb.aqm, matrix_mdev->matrix.aqm,
+		   (unsigned long *)matrix_dev->info.aqm, AP_DOMAINS);
+
+	for_each_set_bit_inv(apid, apm, AP_DEVICES) {
+		for_each_set_bit_inv(apqi, aqm, AP_DOMAINS) {
+			/*
+			 * If the APQN is not bound to the vfio_ap device
+			 * driver, then we can't assign it to the guest's
+			 * AP configuration. The AP architecture won't
+			 * allow filtering of a single APQN, so let's filter
+			 * the APID since an adapter represents a physical
+			 * hardware device.
+			 */
+			apqn = AP_MKQID(apid, apqi);
+
+			if (!vfio_ap_mdev_get_queue(matrix_mdev, apqn)) {
+				clear_bit_inv(apid,
+					      matrix_mdev->shadow_apcb.apm);
+				break;
+			}
+		}
+	}
+}
+
 static int vfio_ap_mdev_probe(struct mdev_device *mdev)
 {
 	struct ap_matrix_mdev *matrix_mdev;
@@ -799,6 +861,8 @@  static ssize_t assign_adapter_store(struct device *dev,
 {
 	int ret;
 	unsigned long apid;
+	DECLARE_BITMAP(apm_delta, AP_DEVICES);
+
 	struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev);
 
 	mutex_lock(&matrix_dev->lock);
@@ -834,6 +898,10 @@  static ssize_t assign_adapter_store(struct device *dev,
 		goto share_err;
 
 	vfio_ap_mdev_link_adapter(matrix_mdev, apid);
+	memset(apm_delta, 0, sizeof(apm_delta));
+	set_bit_inv(apid, apm_delta);
+	vfio_ap_mdev_filter_matrix(apm_delta,
+				   matrix_mdev->matrix.aqm, matrix_mdev);
 	ret = count;
 	goto done;
 
@@ -902,6 +970,10 @@  static ssize_t unassign_adapter_store(struct device *dev,
 
 	clear_bit_inv((unsigned long)apid, matrix_mdev->matrix.apm);
 	vfio_ap_mdev_unlink_adapter(matrix_mdev, apid);
+
+	if (test_bit_inv(apid, matrix_mdev->shadow_apcb.apm))
+		clear_bit_inv(apid, matrix_mdev->shadow_apcb.apm);
+
 	ret = count;
 done:
 	mutex_unlock(&matrix_dev->lock);
@@ -975,6 +1047,7 @@  static ssize_t assign_domain_store(struct device *dev,
 {
 	int ret;
 	unsigned long apqi;
+	DECLARE_BITMAP(aqm_delta, AP_DOMAINS);
 	struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev);
 	unsigned long max_apqi = matrix_mdev->matrix.aqm_max;
 
@@ -1005,6 +1078,10 @@  static ssize_t assign_domain_store(struct device *dev,
 		goto share_err;
 
 	vfio_ap_mdev_link_domain(matrix_mdev, apqi);
+	memset(aqm_delta, 0, sizeof(aqm_delta));
+	set_bit_inv(apqi, aqm_delta);
+	vfio_ap_mdev_filter_matrix(matrix_mdev->matrix.apm, aqm_delta,
+				   matrix_mdev);
 	ret = count;
 	goto done;
 
@@ -1073,6 +1150,10 @@  static ssize_t unassign_domain_store(struct device *dev,
 
 	clear_bit_inv((unsigned long)apqi, matrix_mdev->matrix.aqm);
 	vfio_ap_mdev_unlink_domain(matrix_mdev, apqi);
+
+	if (test_bit_inv(apqi, matrix_mdev->shadow_apcb.aqm))
+		clear_bit_inv(apqi, matrix_mdev->shadow_apcb.aqm);
+
 	ret = count;
 
 done:
@@ -1126,6 +1207,7 @@  static ssize_t assign_control_domain_store(struct device *dev,
 	 * number of control domains that can be assigned.
 	 */
 	set_bit_inv(id, matrix_mdev->matrix.adm);
+	vfio_ap_mdev_filter_cdoms(matrix_mdev);
 	ret = count;
 done:
 	mutex_unlock(&matrix_dev->lock);
@@ -1173,6 +1255,10 @@  static ssize_t unassign_control_domain_store(struct device *dev,
 	}
 
 	clear_bit_inv(domid, matrix_mdev->matrix.adm);
+
+	if (test_bit_inv(domid, matrix_mdev->shadow_apcb.adm))
+		clear_bit_inv(domid, matrix_mdev->shadow_apcb.adm);
+
 	ret = count;
 done:
 	mutex_unlock(&matrix_dev->lock);
@@ -1306,8 +1392,6 @@  static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
 
 		kvm_get_kvm(kvm);
 		matrix_mdev->kvm = kvm;
-		memcpy(&matrix_mdev->shadow_apcb, &matrix_mdev->matrix,
-		       sizeof(struct ap_matrix));
 		kvm_arch_crypto_set_masks(kvm, matrix_mdev->shadow_apcb.apm,
 					  matrix_mdev->shadow_apcb.aqm,
 					  matrix_mdev->shadow_apcb.adm);
@@ -1641,6 +1725,7 @@  static void vfio_ap_queue_link_mdev(struct vfio_ap_queue *q)
 int vfio_ap_mdev_probe_queue(struct ap_device *apdev)
 {
 	struct vfio_ap_queue *q;
+	DECLARE_BITMAP(apm_delta, AP_DEVICES);
 
 	q = kzalloc(sizeof(*q), GFP_KERNEL);
 	if (!q)
@@ -1649,6 +1734,13 @@  int vfio_ap_mdev_probe_queue(struct ap_device *apdev)
 	q->apqn = to_ap_queue(&apdev->device)->qid;
 	q->saved_isc = VFIO_AP_ISC_INVALID;
 	vfio_ap_queue_link_mdev(q);
+	if (q->matrix_mdev) {
+		memset(apm_delta, 0, sizeof(apm_delta));
+		set_bit_inv(AP_QID_CARD(q->apqn), apm_delta);
+		vfio_ap_mdev_filter_matrix(apm_delta,
+					   q->matrix_mdev->matrix.aqm,
+					   q->matrix_mdev);
+	}
 	dev_set_drvdata(&apdev->device, q);
 	mutex_unlock(&matrix_dev->lock);
 
@@ -1657,14 +1749,20 @@  int vfio_ap_mdev_probe_queue(struct ap_device *apdev)
 
 void vfio_ap_mdev_remove_queue(struct ap_device *apdev)
 {
+	unsigned long apid;
 	struct vfio_ap_queue *q;
 
 	mutex_lock(&matrix_dev->lock);
 	q = dev_get_drvdata(&apdev->device);
 
-	if (q->matrix_mdev)
+	if (q->matrix_mdev) {
 		vfio_ap_unlink_queue_fr_mdev(q);
 
+		apid = AP_QID_CARD(q->apqn);
+		if (test_bit_inv(apid, q->matrix_mdev->shadow_apcb.apm))
+			clear_bit_inv(apid, q->matrix_mdev->shadow_apcb.apm);
+	}
+
 	vfio_ap_mdev_reset_queue(q, 1);
 	dev_set_drvdata(&apdev->device, NULL);
 	kfree(q);