Re: [PATCH v2 2/4] eventfd: simplify eventfd_signal()
For vfio_ap_ops.c
Reviewed-by: Anthony Krowiak
On 2/6/24 2:44 PM, Stefan Hajnoczi wrote:
vhost and VIRTIO-related parts:
Reviewed-by: Stefan Hajnoczi
On Wed, 22 Nov 2023 at 07:50, Christian Brauner wrote:
Ever since the evenfd type was introduced back in 2007 in commit
e1ad7468c77d ("signal/timer/event: eventfd core") the eventfd_signal()
function only ever passed 1 as a value for @n. There's no point in
keeping that additional argument.
Signed-off-by: Christian Brauner
---
arch/x86/kvm/hyperv.c | 2 +-
arch/x86/kvm/xen.c| 2 +-
drivers/accel/habanalabs/common/device.c | 2 +-
drivers/fpga/dfl.c| 2 +-
drivers/gpu/drm/drm_syncobj.c | 6 +++---
drivers/gpu/drm/i915/gvt/interrupt.c | 2 +-
drivers/infiniband/hw/mlx5/devx.c | 2 +-
drivers/misc/ocxl/file.c | 2 +-
drivers/s390/cio/vfio_ccw_chp.c | 2 +-
drivers/s390/cio/vfio_ccw_drv.c | 4 ++--
drivers/s390/cio/vfio_ccw_ops.c | 6 +++---
drivers/s390/crypto/vfio_ap_ops.c | 2 +-
drivers/usb/gadget/function/f_fs.c| 4 ++--
drivers/vdpa/vdpa_user/vduse_dev.c| 6 +++---
drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c| 2 +-
drivers/vfio/pci/vfio_pci_core.c | 6 +++---
drivers/vfio/pci/vfio_pci_intrs.c | 12 ++--
drivers/vfio/platform/vfio_platform_irq.c | 4 ++--
drivers/vhost/vdpa.c | 4 ++--
drivers/vhost/vhost.c | 10 +-
drivers/vhost/vhost.h | 2 +-
drivers/virt/acrn/ioeventfd.c | 2 +-
drivers/xen/privcmd.c | 2 +-
fs/aio.c | 2 +-
fs/eventfd.c | 9 +++--
include/linux/eventfd.h | 4 ++--
mm/memcontrol.c | 10 +-
mm/vmpressure.c | 2 +-
samples/vfio-mdev/mtty.c | 4 ++--
virt/kvm/eventfd.c| 4 ++--
30 files changed, 60 insertions(+), 63 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 238afd7335e4..4943f6b2bbee 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2388,7 +2388,7 @@ static u16 kvm_hvcall_signal_event(struct kvm_vcpu *vcpu,
struct kvm_hv_hcall *h
if (!eventfd)
return HV_STATUS_INVALID_PORT_ID;
- eventfd_signal(eventfd, 1);
+ eventfd_signal(eventfd);
return HV_STATUS_SUCCESS;
}
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index e53fad915a62..523bb6df5ac9 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -2088,7 +2088,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu
*vcpu, u64 param, u64 *r)
if (ret < 0 && ret != -ENOTCONN)
return false;
} else {
- eventfd_signal(evtchnfd->deliver.eventfd.ctx, 1);
+ eventfd_signal(evtchnfd->deliver.eventfd.ctx);
}
*r = 0;
diff --git a/drivers/accel/habanalabs/common/device.c
b/drivers/accel/habanalabs/common/device.c
index 9711e8fc979d..3a89644f087c 100644
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -2044,7 +2044,7 @@ static void hl_notifier_event_send(struct
hl_notifier_event *notifier_event, u64
notifier_event->events_mask |= event_mask;
if (notifier_event->eventfd)
- eventfd_signal(notifier_event->eventfd, 1);
+ eventfd_signal(notifier_event->eventfd);
mutex_unlock(¬ifier_event->lock);
}
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index dd7a783d53b5..e73f88050f08 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1872,7 +1872,7 @@ static irqreturn_t dfl_irq_handler(int irq, void *arg)
{
struct eventfd_ctx *trigger = arg;
- eventfd_signal(trigger, 1);
+ eventfd_signal(trigger);
return IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 01da6789d044..b9cc62982196 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1365,7 +1365,7 @@ static void syncobj_eventfd_entry_fence_func(struct
dma_fence *fence,
struct syncobj_eventfd_entry *entry =
container_of(cb, struct syncobj_eventfd_entry, fence_cb);
- eventfd_signal(entry->ev_fd_ctx, 1);
+ eventfd_signal(entry->ev_fd_ctx);
syncobj_eventfd_entry_free(entry);
}
@@ -1388,13 +1388,13 @@ syncobj_eventfd_entry_func(struct drm_syncobj *syncobj,
entry->fence = fence;
if (entry->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE) {
- eventfd_signal(entry->ev_fd_ctx, 1);
+ eventfd_signal(entry->ev_fd_ctx)
Re: [Intel-gfx] [PATCH 1/2] eventfd: simplify eventfd_signal()
For vfio_ap_ops.c:
Reviewed-by: Tony Krowiak
On 7/13/23 6:05 AM, Christian Brauner wrote:
Ever since the evenfd type was introduced back in 2007 in commit
e1ad7468c77d ("signal/timer/event: eventfd core") the eventfd_signal()
function only ever passed 1 as a value for @n. There's no point in
keeping that additional argument.
Signed-off-by: Christian Brauner
---
arch/x86/kvm/hyperv.c | 2 +-
arch/x86/kvm/xen.c| 2 +-
drivers/accel/habanalabs/common/device.c | 2 +-
drivers/fpga/dfl.c| 2 +-
drivers/gpu/drm/i915/gvt/interrupt.c | 2 +-
drivers/infiniband/hw/mlx5/devx.c | 2 +-
drivers/misc/ocxl/file.c | 2 +-
drivers/s390/cio/vfio_ccw_chp.c | 2 +-
drivers/s390/cio/vfio_ccw_drv.c | 4 ++--
drivers/s390/cio/vfio_ccw_ops.c | 6 +++---
drivers/s390/crypto/vfio_ap_ops.c | 2 +-
drivers/usb/gadget/function/f_fs.c| 4 ++--
drivers/vdpa/vdpa_user/vduse_dev.c| 6 +++---
drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c| 2 +-
drivers/vfio/pci/vfio_pci_core.c | 6 +++---
drivers/vfio/pci/vfio_pci_intrs.c | 12 ++--
drivers/vfio/platform/vfio_platform_irq.c | 4 ++--
drivers/vhost/vdpa.c | 4 ++--
drivers/vhost/vhost.c | 10 +-
drivers/vhost/vhost.h | 2 +-
drivers/virt/acrn/ioeventfd.c | 2 +-
fs/aio.c | 2 +-
fs/eventfd.c | 9 +++--
include/linux/eventfd.h | 4 ++--
mm/memcontrol.c | 10 +-
mm/vmpressure.c | 2 +-
samples/vfio-mdev/mtty.c | 4 ++--
virt/kvm/eventfd.c| 4 ++--
28 files changed, 56 insertions(+), 59 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index b28fd020066f..2f4bd74b482c 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2387,7 +2387,7 @@ static u16 kvm_hvcall_signal_event(struct kvm_vcpu *vcpu,
struct kvm_hv_hcall *h
if (!eventfd)
return HV_STATUS_INVALID_PORT_ID;
- eventfd_signal(eventfd, 1);
+ eventfd_signal(eventfd);
return HV_STATUS_SUCCESS;
}
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 40edf4d1974c..a7b62bafd57b 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -2043,7 +2043,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu
*vcpu, u64 param, u64 *r)
if (ret < 0 && ret != -ENOTCONN)
return false;
} else {
- eventfd_signal(evtchnfd->deliver.eventfd.ctx, 1);
+ eventfd_signal(evtchnfd->deliver.eventfd.ctx);
}
*r = 0;
diff --git a/drivers/accel/habanalabs/common/device.c
b/drivers/accel/habanalabs/common/device.c
index b97339d1f7c6..30357b371d61 100644
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -1963,7 +1963,7 @@ static void hl_notifier_event_send(struct
hl_notifier_event *notifier_event, u64
notifier_event->events_mask |= event_mask;
if (notifier_event->eventfd)
- eventfd_signal(notifier_event->eventfd, 1);
+ eventfd_signal(notifier_event->eventfd);
mutex_unlock(¬ifier_event->lock);
}
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index dd7a783d53b5..e73f88050f08 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1872,7 +1872,7 @@ static irqreturn_t dfl_irq_handler(int irq, void *arg)
{
struct eventfd_ctx *trigger = arg;
- eventfd_signal(trigger, 1);
+ eventfd_signal(trigger);
return IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c
index 68eca023bbc6..3d9e09c2add4 100644
--- a/drivers/gpu/drm/i915/gvt/interrupt.c
+++ b/drivers/gpu/drm/i915/gvt/interrupt.c
@@ -435,7 +435,7 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
*/
if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
return -ESRCH;
- if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1)
+ if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger) != 1)
return -EFAULT;
return 0;
}
diff --git a/drivers/infiniband/hw/mlx5/devx.c
b/drivers/infiniband/hw/mlx5/devx.c
index db5fb196c728..ad50487790ff 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -2498,7 +2498,7 @@ static void dispatch_event_fd(struct list_head *fd_list,
list_for_each_entry_rcu(item, fd_list, xa_list) {
if (item->eventfd)
- eventfd_signal(item->eventfd, 1);
+ eventfd_signal(item->eventfd);
else
deliver_event(item,
Re: [Intel-gfx] [PATCH v4 2/2] vfio: no need to pass kvm pointer during device open
Tested-by: Tony Krowiak
On 2/3/23 4:50 PM, Matthew Rosato wrote:
Nothing uses this value during vfio_device_open anymore so it's safe
to remove it.
Signed-off-by: Matthew Rosato
---
drivers/vfio/group.c | 2 +-
drivers/vfio/vfio.h | 3 +--
drivers/vfio/vfio_main.c | 7 +++
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index 98621ac082f0..0e9036e2b9c4 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -187,7 +187,7 @@ static int vfio_device_group_open(struct vfio_device
*device)
if (device->open_count == 0)
vfio_device_group_get_kvm_safe(device);
- ret = vfio_device_open(device, device->group->iommufd, device->kvm);
+ ret = vfio_device_open(device, device->group->iommufd);
if (device->open_count == 0)
vfio_device_put_kvm(device);
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index 24d6cd285945..4f39ab549a80 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -18,8 +18,7 @@ struct vfio_container;
void vfio_device_put_registration(struct vfio_device *device);
bool vfio_device_try_get_registration(struct vfio_device *device);
-int vfio_device_open(struct vfio_device *device,
-struct iommufd_ctx *iommufd, struct kvm *kvm);
+int vfio_device_open(struct vfio_device *device, struct iommufd_ctx *iommufd);
void vfio_device_close(struct vfio_device *device,
struct iommufd_ctx *iommufd);
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 28c47cd6a6b5..3a597e799918 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -397,7 +397,7 @@ static bool vfio_assert_device_open(struct vfio_device
*device)
}
static int vfio_device_first_open(struct vfio_device *device,
- struct iommufd_ctx *iommufd, struct kvm *kvm)
+ struct iommufd_ctx *iommufd)
{
int ret;
@@ -444,8 +444,7 @@ static void vfio_device_last_close(struct vfio_device *device,
module_put(device->dev->driver->owner);
}
-int vfio_device_open(struct vfio_device *device,
-struct iommufd_ctx *iommufd, struct kvm *kvm)
+int vfio_device_open(struct vfio_device *device, struct iommufd_ctx *iommufd)
{
int ret = 0;
@@ -453,7 +452,7 @@ int vfio_device_open(struct vfio_device *device,
device->open_count++;
if (device->open_count == 1) {
- ret = vfio_device_first_open(device, iommufd, kvm);
+ ret = vfio_device_first_open(device, iommufd);
if (ret)
device->open_count--;
}
Re: [Intel-gfx] [PATCH v4 1/2] vfio: fix deadlock between group lock and kvm lock
Tested-by: Tony Krowiak
On 2/3/23 4:50 PM, Matthew Rosato wrote:
After 51cdc8bc120e, we have another deadlock scenario between the
kvm->lock and the vfio group_lock with two different codepaths acquiring
the locks in different order. Specifically in vfio_open_device, vfio
holds the vfio group_lock when issuing device->ops->open_device but some
drivers (like vfio-ap) need to acquire kvm->lock during their open_device
routine; Meanwhile, kvm_vfio_release will acquire the kvm->lock first
before calling vfio_file_set_kvm which will acquire the vfio group_lock.
To resolve this, let's remove the need for the vfio group_lock from the
kvm_vfio_release codepath. This is done by introducing a new spinlock to
protect modifications to the vfio group kvm pointer, and acquiring a kvm
ref from within vfio while holding this spinlock, with the reference held
until the last close for the device in question.
Fixes: 51cdc8bc120e ("kvm/vfio: Fix potential deadlock on vfio group_lock")
Reported-by: Anthony Krowiak
Suggested-by: Jason Gunthorpe
Signed-off-by: Matthew Rosato
---
drivers/vfio/group.c | 44 +++-
drivers/vfio/vfio.h | 15 ++
drivers/vfio/vfio_main.c | 63 +++-
include/linux/vfio.h | 2 +-
4 files changed, 109 insertions(+), 15 deletions(-)
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index bb24b2f0271e..98621ac082f0 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -154,6 +154,18 @@ static int vfio_group_ioctl_set_container(struct
vfio_group *group,
return ret;
}
+static void vfio_device_group_get_kvm_safe(struct vfio_device *device)
+{
+ spin_lock(&device->group->kvm_ref_lock);
+ if (!device->group->kvm)
+ goto unlock;
+
+ _vfio_device_get_kvm_safe(device, device->group->kvm);
+
+unlock:
+ spin_unlock(&device->group->kvm_ref_lock);
+}
+
static int vfio_device_group_open(struct vfio_device *device)
{
int ret;
@@ -164,13 +176,23 @@ static int vfio_device_group_open(struct vfio_device
*device)
goto out_unlock;
}
+ mutex_lock(&device->dev_set->lock);
+
/*
-* Here we pass the KVM pointer with the group under the lock. If the
-* device driver will use it, it must obtain a reference and release it
-* during close_device.
+* Before the first device open, get the KVM pointer currently
+* associated with the group (if there is one) and obtain a reference
+* now that will be held until the open_count reaches 0 again. Save
+* the pointer in the device for use by drivers.
*/
- ret = vfio_device_open(device, device->group->iommufd,
- device->group->kvm);
+ if (device->open_count == 0)
+ vfio_device_group_get_kvm_safe(device);
+
+ ret = vfio_device_open(device, device->group->iommufd, device->kvm);
+
+ if (device->open_count == 0)
+ vfio_device_put_kvm(device);
+
+ mutex_unlock(&device->dev_set->lock);
out_unlock:
mutex_unlock(&device->group->group_lock);
@@ -180,7 +202,14 @@ static int vfio_device_group_open(struct vfio_device
*device)
void vfio_device_group_close(struct vfio_device *device)
{
mutex_lock(&device->group->group_lock);
+ mutex_lock(&device->dev_set->lock);
+
vfio_device_close(device, device->group->iommufd);
+
+ if (device->open_count == 0)
+ vfio_device_put_kvm(device);
+
+ mutex_unlock(&device->dev_set->lock);
mutex_unlock(&device->group->group_lock);
}
@@ -450,6 +479,7 @@ static struct vfio_group *vfio_group_alloc(struct iommu_group *iommu_group,
refcount_set(&group->drivers, 1);
mutex_init(&group->group_lock);
+ spin_lock_init(&group->kvm_ref_lock);
INIT_LIST_HEAD(&group->device_list);
mutex_init(&group->device_lock);
group->iommu_group = iommu_group;
@@ -803,9 +833,9 @@ void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
if (!vfio_file_is_group(file))
return;
- mutex_lock(&group->group_lock);
+ spin_lock(&group->kvm_ref_lock);
group->kvm = kvm;
- mutex_unlock(&group->group_lock);
+ spin_unlock(&group->kvm_ref_lock);
}
EXPORT_SYMBOL_GPL(vfio_file_set_kvm);
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index f8219a438bfb..24d6cd285945 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -74,6 +74,7 @@ struct vfio_group {
struct file *opened_file;
struct blocking_notifier_head notifier;
struct iommufd_ctx *iommufd;
+ spinlock_t
Re: [Intel-gfx] [PATCH] kvm/vfio: Fix potential deadlock on vfio group_lock
On 1/31/23 9:34 AM, Jason Gunthorpe wrote: On Tue, Jan 31, 2023 at 09:27:54AM -0500, Anthony Krowiak wrote: I encountered a lockdep splat while running some regression tests today (see below). I suspected it might be this patch so I reverted it, rebuilt the kernel and ran the regression tests again; this time, the test ran cleanly. It looks like this patch may not have fixed the problem for which it was intended. Here is the relevant dmesg output: Well, it fixes the deadlock it intended to fix and created another one :) It means device drivers cannot obtain the kvm lock from their open functions in this new model. Why does ap need to touch kvm->lock? (via get_update_locks_for_kvm) We need the kvm->lock because we take the vCPUs out of SIE in order to dynamically change values in the APCB. Maybe you should split that lock and have a dedicated apcb lock? I don't think that would suffice for taking the vCPUs out of SIE. Jason
Re: [Intel-gfx] [PATCH] kvm/vfio: Fix potential deadlock on vfio group_lock
I encountered a lockdep splat while running some regression tests today
(see below). I suspected it might be this patch so I reverted it,
rebuilt the kernel and ran the regression tests again; this time, the
test ran cleanly. It looks like this patch may not have fixed the
problem for which it was intended. Here is the relevant dmesg output:
[ 579.471402] hades[1099]: Start test run
[ 579.473486] hades[1099]: Start
'tests.test_vfio_ap.VfioAPAssignMdevToGuestTest.runTest' test
[ 579.505804] vfio_ap matrix: MDEV: Registered
[ 579.604024] vfio_ap_mdev 529654a9-bea4-461a-b64d-9d9c63df0deb: Adding
to iommu group 0
[ 585.043898] ==
[ 585.043900] WARNING: possible circular locking dependency detected
[ 585.043902] 6.2.0-rc6-00057-g41c03ba9beea-dirty #18 Not tainted
[ 585.043904] --
[ 585.043905] CPU 0/KVM/1173 is trying to acquire lock:
[ 585.043907] 8cfb24b0 (&group->group_lock){+.+.}-{3:3}, at:
vfio_file_set_kvm+0x50/0x68 [vfio]
[ 585.043919]
but task is already holding lock:
[ 585.043920] b2960ba0 (&kvm->lock){+.+.}-{3:3}, at:
kvm_device_release+0x4a/0xb8 [kvm]
[ 585.043960]
which lock already depends on the new lock.
[ 585.043962]
the existing dependency chain (in reverse order) is:
[ 585.043963]
-> #3 (&kvm->lock){+.+.}-{3:3}:
[ 585.043967] __lock_acquire+0x3e2/0x750
[ 585.043974] lock_acquire.part.0+0xe2/0x250
[ 585.043977] lock_acquire+0xac/0x1d0
[ 585.043980] __mutex_lock+0x9e/0x868
[ 585.043985] mutex_lock_nested+0x32/0x40
[ 585.043988] vfio_ap_mdev_open_device+0x9a/0x198 [vfio_ap]
[ 585.043991] vfio_device_open+0x122/0x168 [vfio]
[ 585.043995] vfio_device_open_file+0x64/0x120 [vfio]
[ 585.043999] vfio_group_fops_unl_ioctl+0xd4/0x1e0 [vfio]
[ 585.044002] __s390x_sys_ioctl+0xc0/0x100
[ 585.044007] do_syscall+0xee/0x118
[ 585.044032] __do_syscall+0xd2/0x120
[ 585.044035] system_call+0x82/0xb0
[ 585.044037]
-> #2 (&matrix_dev->guests_lock){+.+.}-{3:3}:
[ 585.044041] __lock_acquire+0x3e2/0x750
[ 585.044044] lock_acquire.part.0+0xe2/0x250
[ 585.044047] lock_acquire+0xac/0x1d0
[ 585.044049] __mutex_lock+0x9e/0x868
[ 585.044052] mutex_lock_nested+0x32/0x40
[ 585.044054] vfio_ap_mdev_open_device+0x8c/0x198 [vfio_ap]
[ 585.044057] vfio_device_open+0x122/0x168 [vfio]
[ 585.044060] vfio_device_open_file+0x64/0x120 [vfio]
[ 585.044064] vfio_group_fops_unl_ioctl+0xd4/0x1e0 [vfio]
[ 585.044068] __s390x_sys_ioctl+0xc0/0x100
[ 585.044070] do_syscall+0xee/0x118
[ 585.044072] __do_syscall+0xd2/0x120
[ 585.044074] system_call+0x82/0xb0
[ 585.044076]
-> #1 (&new_dev_set->lock){+.+.}-{3:3}:
[ 585.044080] __lock_acquire+0x3e2/0x750
[ 585.044082] lock_acquire.part.0+0xe2/0x250
[ 585.044085] lock_acquire+0xac/0x1d0
[ 585.044088] __mutex_lock+0x9e/0x868
[ 585.044090] mutex_lock_nested+0x32/0x40
[ 585.044093] vfio_device_open+0x3e/0x168 [vfio]
[ 585.044096] vfio_device_open_file+0x64/0x120 [vfio]
[ 585.044100] vfio_group_fops_unl_ioctl+0xd4/0x1e0 [vfio]
[ 585.044104] __s390x_sys_ioctl+0xc0/0x100
[ 585.044106] do_syscall+0xee/0x118
[ 585.044108] __do_syscall+0xd2/0x120
[ 585.044110] system_call+0x82/0xb0
[ 585.044112]
-> #0 (&group->group_lock){+.+.}-{3:3}:
[ 585.044115] check_prev_add+0xd4/0xf10
[ 585.044118] validate_chain+0x698/0x8e8
[ 585.044120] __lock_acquire+0x3e2/0x750
[ 585.044123] lock_acquire.part.0+0xe2/0x250
[ 585.044125] lock_acquire+0xac/0x1d0
[ 585.044128] __mutex_lock+0x9e/0x868
[ 585.044130] mutex_lock_nested+0x32/0x40
[ 585.044133] vfio_file_set_kvm+0x50/0x68 [vfio]
[ 585.044137] kvm_vfio_release+0x5e/0xf8 [kvm]
[ 585.044156] kvm_device_release+0x90/0xb8 [kvm]
[ 585.044175] __fput+0xaa/0x2a0
[ 585.044180] task_work_run+0x76/0xd0
[ 585.044183] do_exit+0x248/0x538
[ 585.044186] do_group_exit+0x40/0xb0
[ 585.044188] get_signal+0x614/0x698
[ 585.044192] arch_do_signal_or_restart+0x58/0x370
[ 585.044195] exit_to_user_mode_loop+0xe8/0x1b8
[ 585.044200] exit_to_user_mode_prepare+0x164/0x190
[ 585.044203] __do_syscall+0xd2/0x120
[ 585.044205] system_call+0x82/0xb0
[ 585.044207]
other info that might help us debug this:
[ 585.044209] Chain exists of:
&group->group_lock --> &matrix_dev->guests_lock -->
&kvm->lock
[ 585.044213] Possible unsafe locking scenario:
[ 585.044214] CPU0 CPU1
[ 585.044216]
[ 585.04421
Re: [Intel-gfx] [PATCH 1/4] vfio-mdev: allow building the samples into the kernel
On 1/10/23 10:27 AM, Christoph Hellwig wrote: On Tue, Jan 10, 2023 at 09:54:51AM -0500, Anthony Krowiak wrote: + tristate "Build VFIO mtty example mediated device sample code" + depends on VFIO_MDEV Admittedly, I'm not very fluent with Kconfig, but in patch 2 you stated, "VFIO_MDEV is just a library with helpers for the drivers. Stop making it a user choice and just select it by the drivers that use the helpers". Why are you not selecting it here? Because this changes one thing at a time. Patch 2 then switches this depends to a select. My bad, I missed it. Reviewed-by: Tony Krowiak
Re: [Intel-gfx] [PATCH 1/4] vfio-mdev: allow building the samples into the kernel
On 1/10/23 4:10 AM, Christoph Hellwig wrote: There is nothing in the vfio-mdev sample drivers that requires building them as modules, so remove that restriction. Signed-off-by: Christoph Hellwig --- samples/Kconfig | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/samples/Kconfig b/samples/Kconfig index 0d81c00289ee36..f1b8d4ff123036 100644 --- a/samples/Kconfig +++ b/samples/Kconfig @@ -184,23 +184,23 @@ config SAMPLE_UHID Build UHID sample program. config SAMPLE_VFIO_MDEV_MTTY - tristate "Build VFIO mtty example mediated device sample code -- loadable modules only" - depends on VFIO_MDEV && m + tristate "Build VFIO mtty example mediated device sample code" + depends on VFIO_MDEV Admittedly, I'm not very fluent with Kconfig, but in patch 2 you stated, "VFIO_MDEV is just a library with helpers for the drivers. Stop making it a user choice and just select it by the drivers that use the helpers". Why are you not selecting it here? help Build a virtual tty sample driver for use as a VFIO mediated device config SAMPLE_VFIO_MDEV_MDPY - tristate "Build VFIO mdpy example mediated device sample code -- loadable modules only" - depends on VFIO_MDEV && m + tristate "Build VFIO mdpy example mediated device sample code" + depends on VFIO_MDEV help Build a virtual display sample driver for use as a VFIO mediated device. It is a simple framebuffer and supports the region display interface (VFIO_GFX_PLANE_TYPE_REGION). config SAMPLE_VFIO_MDEV_MDPY_FB - tristate "Build VFIO mdpy example guest fbdev driver -- loadable module only" - depends on FB && m + tristate "Build VFIO mdpy example guest fbdev driver" + depends on FB select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -208,8 +208,8 @@ config SAMPLE_VFIO_MDEV_MDPY_FB Guest fbdev driver for the virtual display sample driver. config SAMPLE_VFIO_MDEV_MBOCHS - tristate "Build VFIO mdpy example mediated device sample code -- loadable modules only" - depends on VFIO_MDEV && m + tristate "Build VFIO mdpy example mediated device sample code" + depends on VFIO_MDEV select DMA_SHARED_BUFFER help Build a virtual display sample driver for use as a VFIO
Re: [Intel-gfx] [PATCH 2/4] vfio-mdev: turn VFIO_MDEV into a selectable symbol
Reviewed-by: Tony Krowiak On 1/10/23 4:10 AM, Christoph Hellwig wrote: VFIO_MDEV is just a library with helpers for the drivers. Stop making it a user choice and just select it by the drivers that use the helpers. Signed-off-by: Christoph Hellwig --- Documentation/s390/vfio-ap.rst| 1 - arch/s390/Kconfig | 6 -- arch/s390/configs/debug_defconfig | 1 - arch/s390/configs/defconfig | 1 - drivers/gpu/drm/i915/Kconfig | 2 +- drivers/vfio/mdev/Kconfig | 8 +--- samples/Kconfig | 6 +++--- 7 files changed, 9 insertions(+), 16 deletions(-) diff --git a/Documentation/s390/vfio-ap.rst b/Documentation/s390/vfio-ap.rst index 00f4a04f6d4c6a..d46e98c7c1ec6c 100644 --- a/Documentation/s390/vfio-ap.rst +++ b/Documentation/s390/vfio-ap.rst @@ -553,7 +553,6 @@ These are the steps: * ZCRYPT * S390_AP_IOMMU * VFIO - * VFIO_MDEV * KVM If using make menuconfig select the following to build the vfio_ap module:: diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 318fce77601d35..60fddcdad495e6 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -705,7 +705,8 @@ config EADM_SCH config VFIO_CCW def_tristate n prompt "Support for VFIO-CCW subchannels" - depends on S390_CCW_IOMMU && VFIO_MDEV + depends on S390_CCW_IOMMU + select VFIO_MDEV help This driver allows usage of I/O subchannels via VFIO-CCW. @@ -715,7 +716,8 @@ config VFIO_CCW config VFIO_AP def_tristate n prompt "VFIO support for AP devices" - depends on S390_AP_IOMMU && VFIO_MDEV && KVM + depends on S390_AP_IOMMU && KVM + select VFIO_MDEV depends on ZCRYPT help This driver grants access to Adjunct Processor (AP) devices diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig index 2a827002934bc6..e78fc3ba7d442a 100644 --- a/arch/s390/configs/debug_defconfig +++ b/arch/s390/configs/debug_defconfig @@ -596,7 +596,6 @@ CONFIG_SYNC_FILE=y CONFIG_VFIO=m CONFIG_VFIO_PCI=m CONFIG_MLX5_VFIO_PCI=m -CONFIG_VFIO_MDEV=m CONFIG_VIRTIO_PCI=m CONFIG_VIRTIO_BALLOON=m CONFIG_VIRTIO_INPUT=y diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig index fb780e80e4c8f7..f7eb2e527b6e65 100644 --- a/arch/s390/configs/defconfig +++ b/arch/s390/configs/defconfig @@ -585,7 +585,6 @@ CONFIG_SYNC_FILE=y CONFIG_VFIO=m CONFIG_VFIO_PCI=m CONFIG_MLX5_VFIO_PCI=m -CONFIG_VFIO_MDEV=m CONFIG_VIRTIO_PCI=m CONFIG_VIRTIO_BALLOON=m CONFIG_VIRTIO_INPUT=y diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 3efce05d7b57ca..d06da455253cdb 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -116,9 +116,9 @@ config DRM_I915_GVT_KVMGT depends on X86 depends on 64BIT depends on KVM - depends on VFIO_MDEV select DRM_I915_GVT select KVM_EXTERNAL_WRITE_TRACKING + select VFIO_MDEV help Choose this option if you want to enable Intel GVT-g graphics diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig index 646dbed44eb283..e5fb84e0796507 100644 --- a/drivers/vfio/mdev/Kconfig +++ b/drivers/vfio/mdev/Kconfig @@ -1,10 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only config VFIO_MDEV - tristate "Mediated device driver framework" - default n - help - Provides a framework to virtualize devices. - See Documentation/driver-api/vfio-mediated-device.rst for more details. - - If you don't know what do here, say N. + tristate diff --git a/samples/Kconfig b/samples/Kconfig index f1b8d4ff123036..56b191d128d88f 100644 --- a/samples/Kconfig +++ b/samples/Kconfig @@ -185,14 +185,14 @@ config SAMPLE_UHID config SAMPLE_VFIO_MDEV_MTTY tristate "Build VFIO mtty example mediated device sample code" - depends on VFIO_MDEV + select VFIO_MDEV help Build a virtual tty sample driver for use as a VFIO mediated device config SAMPLE_VFIO_MDEV_MDPY tristate "Build VFIO mdpy example mediated device sample code" - depends on VFIO_MDEV + select VFIO_MDEV help Build a virtual display sample driver for use as a VFIO mediated device. It is a simple framebuffer and supports @@ -209,7 +209,7 @@ config SAMPLE_VFIO_MDEV_MDPY_FB config SAMPLE_VFIO_MDEV_MBOCHS tristate "Build VFIO mdpy example mediated device sample code" - depends on VFIO_MDEV + select VFIO_MDEV select DMA_SHARED_BUFFER help Build a virtual display sample driver for use as a VFIO
Re: [Intel-gfx] [PATCH 4/4] vfio-mdev: remove an non-existing driver from vfio-mediated-device
LGTM Reviewed-by: Tony Krowiak On 1/10/23 4:10 AM, Christoph Hellwig wrote: The nvidia mdev driver does not actually exist anywhere in the tree. Signed-off-by: Christoph Hellwig --- Documentation/driver-api/vfio-mediated-device.rst | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index d4267243b4f525..bbd548b66b4255 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -60,7 +60,7 @@ devices as examples, as these devices are the first devices to use this module:: | mdev.ko | | +---+ | mdev_register_parent() +--+ | | | +<+ | - | | | | | nvidia.ko |<-> physical + | | | | | ccw_device.ko|<-> physical | | | +>+ |device | | | |callbacks+--+ | | Physical | | @@ -69,12 +69,6 @@ devices as examples, as these devices are the first devices to use this module:: | | | | | i915.ko |<-> physical | | | +>+ |device | | | |callbacks+--+ - | | | | - | | | | mdev_register_parent() +--+ - | | | +<+ | - | | | | | ccw_device.ko|<-> physical - | | | +>+ |device - | | | |callbacks+--+ | +---+ | +---+
Re: [Intel-gfx] [PATCH 1/2] KVM: async kvm_destroy_vm for vfio devices
LGTM
Reviewed-by: Tony Krowiak
On 1/9/23 3:10 PM, Matthew Rosato wrote:
Currently it is possible that the final put of a KVM reference comes from
vfio during its device close operation. This occurs while the vfio group
lock is held; however, if the vfio device is still in the kvm device list,
then the following call chain could result in a deadlock:
kvm_put_kvm
-> kvm_destroy_vm
-> kvm_destroy_devices
-> kvm_vfio_destroy
-> kvm_vfio_file_set_kvm
-> vfio_file_set_kvm
-> group->group_lock/group_rwsem
Avoid this scenario by adding kvm_put_kvm_async which will perform the
kvm_destroy_vm asynchronously if the refcount reaches 0.
Fixes: 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM")
Reported-by: Alex Williamson
Signed-off-by: Matthew Rosato
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +-
drivers/s390/crypto/vfio_ap_ops.c | 7 ++-
include/linux/kvm_host.h | 3 +++
virt/kvm/kvm_main.c | 22 ++
4 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 8ae7039b3683..24511c877572 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -703,7 +703,11 @@ static void intel_vgpu_close_device(struct vfio_device
*vfio_dev)
kvm_page_track_unregister_notifier(vgpu->vfio_device.kvm,
&vgpu->track_node);
- kvm_put_kvm(vgpu->vfio_device.kvm);
+ /*
+* Avoid possible deadlock on any currently-held vfio lock by
+* ensuring the potential kvm_destroy_vm call is done asynchronously
+*/
+ kvm_put_kvm_async(vgpu->vfio_device.kvm);
kvmgt_protect_table_destroy(vgpu);
gvt_cache_destroy(vgpu);
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index e93bb9c468ce..a37b2baefb36 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1574,7 +1574,12 @@ static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev
*matrix_mdev)
kvm_arch_crypto_clear_masks(kvm);
vfio_ap_mdev_reset_queues(&matrix_mdev->qtable);
- kvm_put_kvm(kvm);
+ /*
+* Avoid possible deadlock on any currently-held vfio lock by
+* ensuring the potential kvm_destroy_vm call is done
+* asynchronously
+*/
+ kvm_put_kvm_async(kvm);
matrix_mdev->kvm = NULL;
release_update_locks_for_kvm(kvm);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4f26b244f6d0..2ef6a5102265 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -793,6 +794,7 @@ struct kvm {
struct kvm_stat_data **debugfs_stat_data;
struct srcu_struct srcu;
struct srcu_struct irq_srcu;
+ struct work_struct async_work;
pid_t userspace_pid;
bool override_halt_poll_ns;
unsigned int max_halt_poll_ns;
@@ -963,6 +965,7 @@ void kvm_exit(void);
void kvm_get_kvm(struct kvm *kvm);
bool kvm_get_kvm_safe(struct kvm *kvm);
void kvm_put_kvm(struct kvm *kvm);
+void kvm_put_kvm_async(struct kvm *kvm);
bool file_is_kvm(struct file *file);
void kvm_put_kvm_no_destroy(struct kvm *kvm);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 13e88297f999..fbe8d127028b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1353,6 +1353,28 @@ void kvm_put_kvm(struct kvm *kvm)
}
EXPORT_SYMBOL_GPL(kvm_put_kvm);
+static void kvm_put_async_fn(struct work_struct *work)
+{
+ struct kvm *kvm = container_of(work, struct kvm,
+ async_work);
+
+ kvm_destroy_vm(kvm);
+}
+
+/*
+ * Put a reference but only destroy the vm asynchronously. Can be used in
+ * cases where the caller holds a mutex that could cause deadlock if
+ * kvm_destroy_vm is triggered
+ */
+void kvm_put_kvm_async(struct kvm *kvm)
+{
+ if (refcount_dec_and_test(&kvm->users_count)) {
+ INIT_WORK(&kvm->async_work, kvm_put_async_fn);
+ schedule_work(&kvm->async_work);
+ }
+}
+EXPORT_SYMBOL_GPL(kvm_put_kvm_async);
+
/*
* Used to put a reference that was taken on behalf of an object associated
* with a user-visible file descriptor, e.g. a vcpu or device, if installation
Re: [Intel-gfx] [PATCH v2 7/7] vfio: Remove vfio_free_device
Reviewed-by: Tony Krowiak : vfio_ap part
On 11/2/22 11:01 AM, Eric Farman wrote:
With the "mess" sorted out, we should be able to inline the
vfio_free_device call introduced by commit cb9ff3f3b84c
("vfio: Add helpers for unifying vfio_device life cycle")
and remove them from driver release callbacks.
Signed-off-by: Eric Farman
Reviewed-by: Jason Gunthorpe
Reviewed-by: Kevin Tian
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 1 -
drivers/s390/cio/vfio_ccw_ops.c | 2 --
drivers/s390/crypto/vfio_ap_ops.c | 6 --
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 1 -
drivers/vfio/pci/vfio_pci_core.c | 1 -
drivers/vfio/platform/vfio_amba.c | 1 -
drivers/vfio/platform/vfio_platform.c | 1 -
drivers/vfio/vfio_main.c | 22 --
include/linux/vfio.h | 1 -
samples/vfio-mdev/mbochs.c| 1 -
samples/vfio-mdev/mdpy.c | 1 -
samples/vfio-mdev/mtty.c | 1 -
12 files changed, 4 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 7a45e5360caf..eee6805e67de 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1461,7 +1461,6 @@ static void intel_vgpu_release_dev(struct vfio_device
*vfio_dev)
struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
intel_gvt_destroy_vgpu(vgpu);
- vfio_free_device(vfio_dev);
}
static const struct vfio_device_ops intel_vgpu_dev_ops = {
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 1155f8bcedd9..598a3814d428 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -143,8 +143,6 @@ static void vfio_ccw_mdev_release_dev(struct vfio_device
*vdev)
kmem_cache_free(vfio_ccw_io_region, private->io_region);
kfree(private->cp.guest_cp);
mutex_destroy(&private->io_mutex);
-
- vfio_free_device(vdev);
}
static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index 0b4cc8c597ae..f108c0f14712 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -765,11 +765,6 @@ static void vfio_ap_mdev_unlink_fr_queues(struct
ap_matrix_mdev *matrix_mdev)
}
}
-static void vfio_ap_mdev_release_dev(struct vfio_device *vdev)
-{
- vfio_free_device(vdev);
-}
-
static void vfio_ap_mdev_remove(struct mdev_device *mdev)
{
struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(&mdev->dev);
@@ -1784,7 +1779,6 @@ static const struct attribute_group vfio_queue_attr_group
= {
static const struct vfio_device_ops vfio_ap_matrix_dev_ops = {
.init = vfio_ap_mdev_init_dev,
- .release = vfio_ap_mdev_release_dev,
.open_device = vfio_ap_mdev_open_device,
.close_device = vfio_ap_mdev_close_device,
.ioctl = vfio_ap_mdev_ioctl,
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index b16874e913e4..7b8889f55007 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -568,7 +568,6 @@ static void vfio_fsl_mc_release_dev(struct vfio_device
*core_vdev)
vfio_fsl_uninit_device(vdev);
mutex_destroy(&vdev->igate);
- vfio_free_device(core_vdev);
}
static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index badc9d828cac..9be2d5be5d95 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -2109,7 +2109,6 @@ void vfio_pci_core_release_dev(struct vfio_device
*core_vdev)
mutex_destroy(&vdev->vma_lock);
kfree(vdev->region);
kfree(vdev->pm_save);
- vfio_free_device(core_vdev);
}
EXPORT_SYMBOL_GPL(vfio_pci_core_release_dev);
diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
index eaea63e5294c..18faf2678b99 100644
--- a/drivers/vfio/platform/vfio_amba.c
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -95,7 +95,6 @@ static void vfio_amba_release_dev(struct vfio_device
*core_vdev)
vfio_platform_release_common(vdev);
kfree(vdev->name);
- vfio_free_device(core_vdev);
}
static void vfio_amba_remove(struct amba_device *adev)
diff --git a/drivers/vfio/platform/vfio_platform.c
b/drivers/vfio/platform/vfio_platform.c
index 82cedcebfd90..9910451dc341 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -83,7 +83,6 @@ static void vfio_platform_release_dev(struct vfio_device
*core_vdev)
container_of(core_vdev, struct vfio_platform_device, vdev);
vfio_platform_release_common(vdev);
- vfio_free_device(core_vdev);
}
static int vfio_platform_remove(struct platform_device *pdev)
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_
Re: [Intel-gfx] [PATCH 09/15] vfio/ap: Use the new device life cycle helpers
Reviewed-by: Tony Krowiak
On 8/27/22 1:10 PM, Kevin Tian wrote:
From: Yi Liu
and manage available_instances inside @init/@release.
Signed-off-by: Yi Liu
Signed-off-by: Kevin Tian
---
drivers/s390/crypto/vfio_ap_ops.c | 50 ++-
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index 6c8c41fac4e1..803aadfd0876 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -684,42 +684,44 @@ static bool vfio_ap_mdev_filter_matrix(unsigned long
*apm, unsigned long *aqm,
AP_DOMAINS);
}
-static int vfio_ap_mdev_probe(struct mdev_device *mdev)
+static int vfio_ap_mdev_init_dev(struct vfio_device *vdev)
{
- struct ap_matrix_mdev *matrix_mdev;
- int ret;
+ struct ap_matrix_mdev *matrix_mdev =
+ container_of(vdev, struct ap_matrix_mdev, vdev);
if ((atomic_dec_if_positive(&matrix_dev->available_instances) < 0))
return -EPERM;
- matrix_mdev = kzalloc(sizeof(*matrix_mdev), GFP_KERNEL);
- if (!matrix_mdev) {
- ret = -ENOMEM;
- goto err_dec_available;
- }
- vfio_init_group_dev(&matrix_mdev->vdev, &mdev->dev,
- &vfio_ap_matrix_dev_ops);
-
- matrix_mdev->mdev = mdev;
+ matrix_mdev->mdev = to_mdev_device(vdev->dev);
vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->matrix);
matrix_mdev->pqap_hook = handle_pqap;
vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->shadow_apcb);
hash_init(matrix_mdev->qtable.queues);
+ return 0;
+}
+
+static int vfio_ap_mdev_probe(struct mdev_device *mdev)
+{
+ struct ap_matrix_mdev *matrix_mdev;
+ int ret;
+
+ matrix_mdev = vfio_alloc_device(ap_matrix_mdev, vdev, &mdev->dev,
+ &vfio_ap_matrix_dev_ops);
+ if (IS_ERR(matrix_mdev))
+ return PTR_ERR(matrix_mdev);
+
ret = vfio_register_emulated_iommu_dev(&matrix_mdev->vdev);
if (ret)
- goto err_list;
+ goto err_put_vdev;
dev_set_drvdata(&mdev->dev, matrix_mdev);
mutex_lock(&matrix_dev->mdevs_lock);
list_add(&matrix_mdev->node, &matrix_dev->mdev_list);
mutex_unlock(&matrix_dev->mdevs_lock);
return 0;
-err_list:
- vfio_uninit_group_dev(&matrix_mdev->vdev);
- kfree(matrix_mdev);
-err_dec_available:
- atomic_inc(&matrix_dev->available_instances);
+err_put_vdev:
+ vfio_put_device(&matrix_mdev->vdev);
return ret;
}
@@ -766,6 +768,12 @@ static void vfio_ap_mdev_unlink_fr_queues(struct ap_matrix_mdev *matrix_mdev)
}
}
+static void vfio_ap_mdev_release_dev(struct vfio_device *vdev)
+{
+ vfio_free_device(vdev);
+ atomic_inc(&matrix_dev->available_instances);
+}
+
static void vfio_ap_mdev_remove(struct mdev_device *mdev)
{
struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(&mdev->dev);
@@ -779,9 +787,7 @@ static void vfio_ap_mdev_remove(struct mdev_device *mdev)
list_del(&matrix_mdev->node);
mutex_unlock(&matrix_dev->mdevs_lock);
mutex_unlock(&matrix_dev->guests_lock);
- vfio_uninit_group_dev(&matrix_mdev->vdev);
- kfree(matrix_mdev);
- atomic_inc(&matrix_dev->available_instances);
+ vfio_put_device(&matrix_mdev->vdev);
}
static ssize_t name_show(struct mdev_type *mtype,
@@ -1794,6 +1800,8 @@ static const struct attribute_group vfio_queue_attr_group
= {
};
static const struct vfio_device_ops vfio_ap_matrix_dev_ops = {
+ .init = vfio_ap_mdev_init_dev,
+ .release = vfio_ap_mdev_release_dev,
.open_device = vfio_ap_mdev_open_device,
.close_device = vfio_ap_mdev_close_device,
.ioctl = vfio_ap_mdev_ioctl,
Re: [Intel-gfx] [PATCH 01/15] vfio: Add helpers for unifying vfio_device life cycle
On 8/30/22 11:10 AM, Jason Gunthorpe wrote: On Tue, Aug 30, 2022 at 09:42:42AM -0400, Anthony Krowiak wrote: +/* + * Alloc and initialize vfio_device so it can be registered to vfio + * core. + * + * Drivers should use the wrapper vfio_alloc_device() for allocation. + * @size is the size of the structure to be allocated, including any + * private data used by the driver. It seems the purpose of the wrapper is to ensure that the object being allocated has as its first field a struct vfio_device object and to return its container. Why not just make that a requirement for this function - which I would rename vfio_alloc_device - and document it in the prologue? The caller can then cast the return pointer or use container_of. There are three fairly common patterns for this kind of thing 1) The caller open codes everything: driver_struct = kzalloc() core_init(&driver_struct->core) 2) Some 'get priv' / 'get data' is used instead of container_of(): core_struct = core_alloc(sizeof(*driver_struct)) driver_struct = core_get_priv(core_struct) 3) The allocations and initialization are consolidated in the core, but we continue to use container_of() driver_struct = core_alloc(typeof(*driver_struct)) #1 has a general drawback that people routinely mess up the lifecycle model and get really confused about when to do kfree() vs put(), creating bugs. #2 has a general drawback of not using container_of() at all, and being a bit confusing in some cases #3 has the general drawback of being a bit magical, but solves 1 and 2's problems. I would not fix the struct layout without the BUILD_BUG_ON because someone will accidently change the order and that becomes a subtle runtime error - so at a minimum the wrapper macro has to exist to check that. If you want to allow a dynamic struct layout and avoid the pitfall of exposing the user to kalloc/kfree, then you still need the macro, and it does some more complicated offset stuff. Having the wrapper macro be entirely type safe is appealing and reduces code in the drivers, IMHO. Tell it what type you are initing and get back init'd memory for that type that you always, always free with a put operation. Sounds reasonable, okay I'm buying. Jason
Re: [Intel-gfx] [PATCH 01/15] vfio: Add helpers for unifying vfio_device life cycle
Reviewed-by: Tony Krowiak
I have a couple of review comments, but nothing critical that would
prevent my r-b.
On 8/27/22 1:10 PM, Kevin Tian wrote:
The idea is to let vfio core manage the vfio_device life cycle instead
of duplicating the logic cross drivers. This is also a preparatory
step for adding struct device into vfio_device.
New pair of helpers together with a kref in vfio_device:
- vfio_alloc_device()
- vfio_put_device()
Drivers can register @init/@release callbacks to manage any private
state wrapping the vfio_device.
However vfio-ccw doesn't fit this model due to a life cycle mess
that its private structure mixes both parent and mdev info hence must
be allocated/free'ed outside of the life cycle of vfio device.
Per prior discussions this won't be fixed in short term by IBM folks.
Instead of waiting introduce another helper vfio_init_device() so ccw
can call it to initialize a pre-allocated vfio_device.
Further implication of the ccw trick is that vfio_device cannot be
free'ed uniformly in vfio core. Instead, require *EVERY* driver to
implement @release and free vfio_device inside. Then ccw can choose
to delay the free at its own discretion.
Another trick down the road is that kvzalloc() is used to accommodate
the need of gvt which uses vzalloc() while all others use kzalloc().
So drivers should call a helper vfio_free_device() to free the
vfio_device instead of assuming that kfree() or vfree() is appliable.
Later once the ccw mess is fixed we can remove those tricks and
fully handle structure alloc/free in vfio core.
Existing vfio_{un}init_group_dev() will be deprecated after all
existing usages are converted to the new model.
Suggested-by: Jason Gunthorpe
Co-developed-by: Yi Liu
Signed-off-by: Yi Liu
Signed-off-by: Kevin Tian
---
drivers/vfio/vfio_main.c | 92
include/linux/vfio.h | 25 ++-
2 files changed, 116 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 7cb56c382c97..af8aad116f2b 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -496,6 +496,98 @@ void vfio_uninit_group_dev(struct vfio_device *device)
}
EXPORT_SYMBOL_GPL(vfio_uninit_group_dev);
+/*
+ * Alloc and initialize vfio_device so it can be registered to vfio
+ * core.
+ *
+ * Drivers should use the wrapper vfio_alloc_device() for allocation.
+ * @size is the size of the structure to be allocated, including any
+ * private data used by the driver.
It seems the purpose of the wrapper is to ensure that the object being
allocated has as its first field a struct vfio_device object and to
return its container. Why not just make that a requirement for this
function - which I would rename vfio_alloc_device - and document it in
the prologue? The caller can then cast the return pointer or use
container_of.
+ *
+ * Driver may provide an @init callback to cover device private data.
+ *
+ * Use vfio_put_device() to release the structure after success return.
+ */
+struct vfio_device *_vfio_alloc_device(size_t size, struct device *dev,
+ const struct vfio_device_ops *ops)
+{
+ struct vfio_device *device;
+ int ret;
+
+ if (WARN_ON(size < sizeof(struct vfio_device)))
+ return ERR_PTR(-EINVAL);
+
+ device = kvzalloc(size, GFP_KERNEL);
+ if (!device)
+ return ERR_PTR(-ENOMEM);
+
+ ret = vfio_init_device(device, dev, ops);
+ if (ret)
+ goto out_free;
+ return device;
+
+out_free:
+ kvfree(device);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(_vfio_alloc_device);
+
+/*
+ * Initialize a vfio_device so it can be registered to vfio core.
+ *
+ * Only vfio-ccw driver should call this interface.
+ */
+int vfio_init_device(struct vfio_device *device, struct device *dev,
+const struct vfio_device_ops *ops)
+{
+ int ret;
+
+ vfio_init_group_dev(device, dev, ops);
+
+ if (ops->init) {
+ ret = ops->init(device);
+ if (ret)
+ goto out_uninit;
+ }
+
+ kref_init(&device->kref);
+ return 0;
+
+out_uninit:
+ vfio_uninit_group_dev(device);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(vfio_init_device);
+
+/*
+ * The helper called by driver @release callback to free the device
+ * structure. Drivers which don't have private data to clean can
+ * simply use this helper as its @release.
+ */
+void vfio_free_device(struct vfio_device *device)
+{
+ kvfree(device);
+}
+EXPORT_SYMBOL_GPL(vfio_free_device);
+
+/* Release helper called by vfio_put_device() */
+void vfio_device_release(struct kref *kref)
+{
+ struct vfio_device *device =
+ container_of(kref, struct vfio_device, kref);
+
+ vfio_uninit_group_dev(device);
+
+ /*
+* kvfree() cannot be done here due to a life cycle mess in
+* vfio-ccw. Before the ccw part is f
Re: [Intel-gfx] [PATCH v3 05/10] vfio: Pass in starting IOVA to vfio_pin/unpin_pages API
Reviewed-by: Tony Krowiak
On 7/8/22 6:44 PM, Nicolin Chen wrote:
The vfio_pin/unpin_pages() so far accepted arrays of PFNs of user IOVA.
Among all three callers, there was only one caller possibly passing in
a non-contiguous PFN list, which is now ensured to have contiguous PFN
inputs too.
Pass in the starting address with "iova" alone to simplify things, so
callers no longer need to maintain a PFN list or to pin/unpin one page
at a time. This also allows VFIO to use more efficient implementations
of pin/unpin_pages.
For now, also update vfio_iommu_type1 to fit this new parameter too,
while keeping its input intact (being user_iova) since we don't want
to spend too much effort swapping its parameters and local variables
at that level.
Reviewed-by: Christoph Hellwig
Reviewed by: Kirti Wankhede
Reviewed-by: Jason Gunthorpe
Reviewed-by: Kevin Tian
Acked-by: Eric Farman
Tested-by: Terrence Xu
Tested-by: Eric Farman
Signed-off-by: Nicolin Chen
---
.../driver-api/vfio-mediated-device.rst | 4 +--
drivers/gpu/drm/i915/gvt/kvmgt.c | 22 ++-
drivers/s390/cio/vfio_ccw_cp.c| 4 +--
drivers/s390/crypto/vfio_ap_ops.c | 9 +++
drivers/vfio/vfio.c | 27 +--
drivers/vfio/vfio.h | 4 +--
drivers/vfio/vfio_iommu_type1.c | 15 +--
include/linux/vfio.h | 5 ++--
8 files changed, 39 insertions(+), 51 deletions(-)
diff --git a/Documentation/driver-api/vfio-mediated-device.rst
b/Documentation/driver-api/vfio-mediated-device.rst
index 4307421dcaa0..af31eaf836e8 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -260,10 +260,10 @@ Translation APIs for Mediated Devices
The following APIs are provided for translating user pfn to host pfn in a VFIO
driver::
- int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
+ int vfio_pin_pages(struct vfio_device *device, dma_addr_t iova,
int npage, int prot, unsigned long *phys_pfn);
- void vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
+ void vfio_unpin_pages(struct vfio_device *device, dma_addr_t iova,
int npage);
These functions call back into the back-end IOMMU module by using the pin_pages
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 7ce7b09aa5b2..d3ac8383d759 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -231,14 +231,8 @@ static void intel_gvt_cleanup_vgpu_type_groups(struct
intel_gvt *gvt)
static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn,
unsigned long size)
{
- int total_pages = DIV_ROUND_UP(size, PAGE_SIZE);
- int npage;
-
- for (npage = 0; npage < total_pages; npage++) {
- unsigned long cur_gfn = gfn + npage;
-
- vfio_unpin_pages(&vgpu->vfio_device, &cur_gfn, 1);
- }
+ vfio_unpin_pages(&vgpu->vfio_device, gfn << PAGE_SHIFT,
+DIV_ROUND_UP(size, PAGE_SIZE));
}
/* Pin a normal or compound guest page for dma. */
@@ -255,14 +249,14 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu,
unsigned long gfn,
* on stack to hold pfns.
*/
for (npage = 0; npage < total_pages; npage++) {
- unsigned long cur_gfn = gfn + npage;
+ dma_addr_t cur_iova = (gfn + npage) << PAGE_SHIFT;
unsigned long pfn;
- ret = vfio_pin_pages(&vgpu->vfio_device, &cur_gfn, 1,
+ ret = vfio_pin_pages(&vgpu->vfio_device, cur_iova, 1,
IOMMU_READ | IOMMU_WRITE, &pfn);
if (ret != 1) {
- gvt_vgpu_err("vfio_pin_pages failed for gfn 0x%lx, ret
%d\n",
-cur_gfn, ret);
+ gvt_vgpu_err("vfio_pin_pages failed for iova %pad, ret
%d\n",
+&cur_iova, ret);
goto err;
}
@@ -306,7 +300,7 @@ static int gvt_dma_map_page(struct intel_vgpu *vgpu, unsigned long gfn,
if (dma_mapping_error(dev, *dma_addr)) {
gvt_vgpu_err("DMA mapping failed for pfn 0x%lx, ret %d\n",
page_to_pfn(page), ret);
- gvt_unpin_guest_page(vgpu, gfn, size);
+ gvt_unpin_guest_page(vgpu, gfn << PAGE_SHIFT, size);
return -ENOMEM;
}
@@ -319,7 +313,7 @@ static void gvt_dma_unmap_page(struct intel_vgpu *vgpu, unsigned long gfn,
struct device *dev = vgpu->gvt->gt->i915->drm.dev;
dma_unmap_page(dev, dma_addr, size, DMA_BIDIRECTIONAL);
- gvt_unpin_guest_page(vgpu, gfn, size);
+ gvt_unpin_guest_page(vgpu, gfn << PAGE_SHI
Re: [Intel-gfx] [PATCH v3 01/10] vfio: Make vfio_unpin_pages() return void
On 7/8/22 6:44 PM, Nicolin Chen wrote:
There's only one caller that checks its return value with a WARN_ON_ONCE,
while all other callers don't check the return value at all. Above that,
an undo function should not fail. So, simplify the API to return void by
embedding similar WARN_ONs.
Also for users to pinpoint which condition fails, separate WARN_ON lines,
yet remove the "driver->ops->unpin_pages" check, since it's unreasonable
for callers to unpin on something totally random that wasn't even pinned.
And remove NULL pointer checks for they would trigger oops vs. warnings.
Note that npage is already validated in the vfio core, thus drop the same
check in the type1 code.
Suggested-by: Christoph Hellwig
Reviewed-by: Christoph Hellwig
Reviewed-by: Jason Gunthorpe
Reviewed-by: Kirti Wankhede
Tested-by: Terrence Xu
Signed-off-by: Nicolin Chen
---
.../driver-api/vfio-mediated-device.rst | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 5 +
drivers/vfio/vfio.c | 21 +++
drivers/vfio/vfio.h | 2 +-
drivers/vfio/vfio_iommu_type1.c | 15 ++---
include/linux/vfio.h | 4 ++--
6 files changed, 18 insertions(+), 31 deletions(-)
diff --git a/Documentation/driver-api/vfio-mediated-device.rst
b/Documentation/driver-api/vfio-mediated-device.rst
index d7a512676853..4307421dcaa0 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -263,7 +263,7 @@ driver::
int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
int npage, int prot, unsigned long *phys_pfn);
- int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
+ void vfio_unpin_pages(struct vfio_device *device, unsigned long
*user_pfn,
int npage);
These functions call back into the back-end IOMMU module by using the pin_pages
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e2f6c56ab342..8c67c9aba82d 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -231,18 +231,15 @@ static void intel_gvt_cleanup_vgpu_type_groups(struct
intel_gvt *gvt)
static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn,
unsigned long size)
{
- struct drm_i915_private *i915 = vgpu->gvt->gt->i915;
int total_pages;
int npage;
- int ret;
total_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
for (npage = 0; npage < total_pages; npage++) {
unsigned long cur_gfn = gfn + npage;
- ret = vfio_unpin_pages(&vgpu->vfio_device, &cur_gfn, 1);
- drm_WARN_ON(&i915->drm, ret != 1);
+ vfio_unpin_pages(&vgpu->vfio_device, &cur_gfn, 1);
}
}
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 100a3d84380c..ad90adbfddc8 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1964,31 +1964,24 @@ EXPORT_SYMBOL(vfio_pin_pages);
* PFNs should not be greater than VFIO_PIN_PAGES_MAX_ENTRIES.
* @npage [in] : count of elements in user_pfn array. This count should not
* be greater than VFIO_PIN_PAGES_MAX_ENTRIES.
- * Return error or number of pages unpinned.
*/
-int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
-int npage)
+void vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
+ int npage)
{
struct vfio_container *container;
struct vfio_iommu_driver *driver;
- int ret;
- if (!user_pfn || !npage || !vfio_assert_device_open(device))
- return -EINVAL;
You left out the check for !user_pfn?
+ if (WARN_ON(npage <= 0 || npage > VFIO_PIN_PAGES_MAX_ENTRIES))
+ return;
- if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
- return -E2BIG;
+ if (WARN_ON(!vfio_assert_device_open(device)))
+ return;
/* group->container cannot change while a vfio device is open */
container = device->group->container;
driver = container->iommu_driver;
- if (likely(driver && driver->ops->unpin_pages))
- ret = driver->ops->unpin_pages(container->iommu_data, user_pfn,
- npage);
- else
- ret = -ENOTTY;
- return ret;
+ driver->ops->unpin_pages(container->iommu_data, user_pfn, npage);
}
EXPORT_SYMBOL(vfio_unpin_pages);
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index a67130221151..bef4edf58138 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -53,7 +53,7 @@ struct vfio_iommu_driver_ops {
unsigned long *user_pfn,
int npage, int prot,
Re: [Intel-gfx] [PATCH v3 06/10] vfio/ap: Change saved_pfn to saved_iova
Reviewed-by: Tony Krowiak
On 7/8/22 6:44 PM, Nicolin Chen wrote:
The vfio_ap_ops code maintains both nib address and its PFN, which
is redundant, merely because vfio_pin/unpin_pages API wanted pfn.
Since vfio_pin/unpin_pages() now accept "iova", change "saved_pfn"
to "saved_iova" and remove pfn in the vfio_ap_validate_nib().
Reviewed-by: Jason Gunthorpe
Tested-by: Eric Farman
Signed-off-by: Nicolin Chen
---
drivers/s390/crypto/vfio_ap_ops.c | 42 +++
drivers/s390/crypto/vfio_ap_private.h | 4 +--
2 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index 8a2018ab3cf0..e8856a7e151c 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -112,7 +112,7 @@ static void vfio_ap_wait_for_irqclear(int apqn)
*
* Unregisters the ISC in the GIB when the saved ISC not invalid.
* Unpins the guest's page holding the NIB when it exists.
- * Resets the saved_pfn and saved_isc to invalid values.
+ * Resets the saved_iova and saved_isc to invalid values.
*/
static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
{
@@ -123,9 +123,9 @@ static void vfio_ap_free_aqic_resources(struct
vfio_ap_queue *q)
kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->saved_isc);
q->saved_isc = VFIO_AP_ISC_INVALID;
}
- if (q->saved_pfn && !WARN_ON(!q->matrix_mdev)) {
- vfio_unpin_pages(&q->matrix_mdev->vdev, q->saved_pfn <<
PAGE_SHIFT, 1);
- q->saved_pfn = 0;
+ if (q->saved_iova && !WARN_ON(!q->matrix_mdev)) {
+ vfio_unpin_pages(&q->matrix_mdev->vdev, q->saved_iova, 1);
+ q->saved_iova = 0;
}
}
@@ -189,27 +189,19 @@ static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
*
* @vcpu: the object representing the vcpu executing the PQAP(AQIC)
instruction.
* @nib: the location for storing the nib address.
- * @g_pfn: the location for storing the page frame number of the page
containing
- *the nib.
*
* When the PQAP(AQIC) instruction is executed, general register 2 contains
the
* address of the notification indicator byte (nib) used for IRQ notification.
- * This function parses the nib from gr2 and calculates the page frame
- * number for the guest of the page containing the nib. The values are
- * stored in @nib and @g_pfn respectively.
- *
- * The g_pfn of the nib is then validated to ensure the nib address is valid.
+ * This function parses and validates the nib from gr2.
*
* Return: returns zero if the nib address is a valid; otherwise, returns
* -EINVAL.
*/
-static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, unsigned long *nib,
- unsigned long *g_pfn)
+static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, dma_addr_t *nib)
{
*nib = vcpu->run->s.regs.gprs[2];
- *g_pfn = *nib >> PAGE_SHIFT;
- if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *g_pfn)))
+ if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *nib >> PAGE_SHIFT)))
return -EINVAL;
return 0;
@@ -239,34 +231,34 @@ static struct ap_queue_status vfio_ap_irq_enable(struct
vfio_ap_queue *q,
int isc,
struct kvm_vcpu *vcpu)
{
- unsigned long nib;
struct ap_qirq_ctrl aqic_gisa = {};
struct ap_queue_status status = {};
struct kvm_s390_gisa *gisa;
int nisc;
struct kvm *kvm;
- unsigned long g_pfn, h_pfn;
+ unsigned long h_pfn;
phys_addr_t h_nib;
+ dma_addr_t nib;
int ret;
/* Verify that the notification indicator byte address is valid */
- if (vfio_ap_validate_nib(vcpu, &nib, &g_pfn)) {
- VFIO_AP_DBF_WARN("%s: invalid NIB address: nib=%#lx, g_pfn=%#lx,
apqn=%#04x\n",
-__func__, nib, g_pfn, q->apqn);
+ if (vfio_ap_validate_nib(vcpu, &nib)) {
+ VFIO_AP_DBF_WARN("%s: invalid NIB address: nib=%pad,
apqn=%#04x\n",
+__func__, &nib, q->apqn);
status.response_code = AP_RESPONSE_INVALID_ADDRESS;
return status;
}
- ret = vfio_pin_pages(&q->matrix_mdev->vdev, g_pfn << PAGE_SHIFT, 1,
+ ret = vfio_pin_pages(&q->matrix_mdev->vdev, nib, 1,
IOMMU_READ | IOMMU_WRITE, &h_pfn);
switch (ret) {
case 1:
break;
default:
VFIO_AP_DBF_WARN("%s: vfio_pin_pages failed: rc=%d,"
-"nib=%#lx, g_pfn=%#lx, apqn=%#04x\n",
-__func__, ret, nib, g_pfn, q->apqn);
+"nib=%pad, apqn=%#04x\n",
+__func__, ret, &nib, q->apqn);
status.response_code = AP_RESPONSE_INVAL
