Re: [PATCH v11 11/26] s390: vfio-ap: implement mediated device open callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/28/2018 06:14 AM, Cornelia Huck wrote:
On Tue, 25 Sep 2018 19:16:26 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> wrote:

From: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>

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. The open callback must ensure that only one
mediated device shall be opened per guest.

Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
Acked-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
Tested-by: Michael Mueller <mimu@xxxxxxxxxxxxx>
Tested-by: Farhan Ali <alifm@xxxxxxxxxxxxx>
Tested-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
Acked-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
  arch/s390/include/asm/kvm_host.h      |   1 +
  drivers/s390/crypto/vfio_ap_ops.c     | 159 ++++++++++++++++++++++++++
  drivers/s390/crypto/vfio_ap_private.h |   5 +
  3 files changed, 165 insertions(+)


+static void vfio_ap_mdev_copy_masks(struct ap_matrix_mdev *matrix_mdev)
+{
+	int nbytes;
+	unsigned long *apm, *aqm, *adm;
+	struct kvm_s390_crypto_cb *crycb = matrix_mdev->kvm->arch.crypto.crycb;
+
+	switch (matrix_mdev->kvm->arch.crypto.crycbd & CRYCB_FORMAT_MASK) {
+	case CRYCB_FORMAT2:
+		apm = (unsigned long *)crycb->apcb1.apm;
+		aqm = (unsigned long *)crycb->apcb1.aqm;
+		adm = (unsigned long *)crycb->apcb1.adm;
+		break;
+	case CRYCB_FORMAT1:
+	case CRYCB_FORMAT0:
+	default:

That one looks a tad odd: the masking above should guarantee that we
hit either the format 2, 1, or 0 cases, so the default case should
never hit. Moreover, if the mask above will ever be changed, we don't
know where we should look for the correct masks (what will a FORMAT<n>
use: apcb1, apcb0, or something else?) Would it be better to moan in
the default case and copy nothing?

It probably is a bit odd, but is not a problem now. If a new
format is added, one would hope the developer coding the change
would be smart enough to figure out what to do here.


+		apm = (unsigned long *)crycb->apcb0.apm;
+		aqm = (unsigned long *)crycb->apcb0.aqm;
+		adm = (unsigned long *)crycb->apcb0.adm;
+		break;
+	}
+
+	nbytes = DIV_ROUND_UP(matrix_mdev->matrix.apm_max + 1, BITS_PER_BYTE);
+	memcpy(apm, matrix_mdev->matrix.apm, nbytes);
+	nbytes = DIV_ROUND_UP(matrix_mdev->matrix.aqm_max + 1, BITS_PER_BYTE);
+	memcpy(aqm, matrix_mdev->matrix.aqm, nbytes);
+	nbytes = DIV_ROUND_UP(matrix_mdev->matrix.adm_max + 1, BITS_PER_BYTE);
+	memcpy(adm, matrix_mdev->matrix.adm, nbytes);
+}

Anyway, that can be easily changed later on; in general,

true


Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux