Skip to content

Commit

Permalink
s390/vfio-ap: No need to disable IRQ after queue reset
Browse files Browse the repository at this point in the history 
The queues assigned to a matrix mediated device are currently reset when:

* The VFIO_DEVICE_RESET ioctl is invoked
* The mdev fd is closed by userspace (QEMU)
* The mdev is removed from sysfs.

Immediately after the reset of a queue, a call is made to disable
interrupts for the queue. This is entirely unnecessary because the reset of
a queue disables interrupts, so this will be removed.

Since interrupt processing may have been enabled by the guest, it may also
be necessary to clean up the resources used for interrupt processing. Part
of the cleanup operation requires a reference to KVM, so a check is also
being added to ensure the reference to KVM exists. The reason is because
the release callback - invoked when userspace closes the mdev fd - removes
the reference to KVM. When the remove callback - called when the mdev is
removed from sysfs - is subsequently invoked, there will be no reference to
KVM when the cleanup is performed.

This patch will also do a bit of refactoring due to the fact that the
remove callback, implemented in vfio_ap_drv.c, disables the queue after
resetting it. Instead of the remove callback making a call into the
vfio_ap_ops.c to clean up the resources used for interrupt processing,
let's move the probe and remove callbacks into the vfio_ap_ops.c
file keep all code related to managing queues in a single file.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
  • Loading branch information
@intel-lab-lkp
Tony Krowiak authored and intel-lab-lkp committed Oct 22, 2020
1 parent 10e5afb commit 572c94c
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 63 deletions.
45 changes: 2 additions & 43 deletions drivers/s390/crypto/vfio_ap_drv.c
View file
Expand Up @@ -43,47 +43,6 @@ static struct ap_device_id ap_queue_ids[] = {

MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);

/**
* vfio_ap_queue_dev_probe:
*
* Allocate a vfio_ap_queue structure and associate it
* with the device as driver_data.
*/
static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
{
struct vfio_ap_queue *q;

q = kzalloc(sizeof(*q), GFP_KERNEL);
if (!q)
return -ENOMEM;
dev_set_drvdata(&apdev->device, q);
q->apqn = to_ap_queue(&apdev->device)->qid;
q->saved_isc = VFIO_AP_ISC_INVALID;
return 0;
}

/**
* vfio_ap_queue_dev_remove:
*
* Takes the matrix lock to avoid actions on this device while removing
* Free the associated vfio_ap_queue structure
*/
static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
{
struct vfio_ap_queue *q;
int apid, apqi;

mutex_lock(&matrix_dev->lock);
q = dev_get_drvdata(&apdev->device);
dev_set_drvdata(&apdev->device, NULL);
apid = AP_QID_CARD(q->apqn);
apqi = AP_QID_QUEUE(q->apqn);
vfio_ap_mdev_reset_queue(apid, apqi, 1);
vfio_ap_irq_disable(q);
kfree(q);
mutex_unlock(&matrix_dev->lock);
}

static void vfio_ap_matrix_dev_release(struct device *dev)
{
struct ap_matrix_dev *matrix_dev = dev_get_drvdata(dev);
Expand Down Expand Up @@ -186,8 +145,8 @@ static int __init vfio_ap_init(void)
return ret;

memset(&vfio_ap_drv, 0, sizeof(vfio_ap_drv));
vfio_ap_drv.probe = vfio_ap_queue_dev_probe;
vfio_ap_drv.remove = vfio_ap_queue_dev_remove;
vfio_ap_drv.probe = vfio_ap_mdev_probe_queue;
vfio_ap_drv.remove = vfio_ap_mdev_remove_queue;
vfio_ap_drv.ids = ap_queue_ids;

ret = ap_driver_register(&vfio_ap_drv, THIS_MODULE, VFIO_AP_DRV_NAME);
Expand Down
63 changes: 46 additions & 17 deletions drivers/s390/crypto/vfio_ap_ops.c
View file
Expand Up @@ -119,7 +119,8 @@ static void vfio_ap_wait_for_irqclear(int apqn)
*/
static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
{
if (q->saved_isc != VFIO_AP_ISC_INVALID && q->matrix_mdev)
if (q->saved_isc != VFIO_AP_ISC_INVALID && q->matrix_mdev &&
q->matrix_mdev->kvm)
kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->saved_isc);
if (q->saved_pfn && q->matrix_mdev)
vfio_unpin_pages(mdev_dev(q->matrix_mdev->mdev),
Expand All @@ -144,7 +145,7 @@ static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
* Returns if ap_aqic function failed with invalid, deconfigured or
* checkstopped AP.
*/
struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
{
struct ap_qirq_ctrl aqic_gisa = {};
struct ap_queue_status status;
Expand Down Expand Up @@ -297,6 +298,7 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
if (!q)
goto out_unlock;

q->matrix_mdev = matrix_mdev;
status = vcpu->run->s.regs.gprs[1];

/* If IR bit(16) is set we enable the interrupt */
Expand Down Expand Up @@ -1114,20 +1116,6 @@ static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
return NOTIFY_OK;
}

static void vfio_ap_irq_disable_apqn(int apqn)
{
struct device *dev;
struct vfio_ap_queue *q;

dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL,
&apqn, match_apqn);
if (dev) {
q = dev_get_drvdata(dev);
vfio_ap_irq_disable(q);
put_device(dev);
}
}

int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
unsigned int retry)
{
Expand Down Expand Up @@ -1162,6 +1150,7 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
{
int ret;
int rc = 0;
struct vfio_ap_queue *q;
unsigned long apid, apqi;
struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);

Expand All @@ -1177,7 +1166,10 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
*/
if (ret)
rc = ret;
vfio_ap_irq_disable_apqn(AP_MKQID(apid, apqi));
q = vfio_ap_get_queue(matrix_mdev,
AP_MKQID(apid, apqi));
if (q)
vfio_ap_free_aqic_resources(q);
}
}

Expand Down Expand Up @@ -1302,3 +1294,40 @@ void vfio_ap_mdev_unregister(void)
{
mdev_unregister_device(&matrix_dev->device);
}

int vfio_ap_mdev_probe_queue(struct ap_device *apdev)
{
struct vfio_ap_queue *q;
struct ap_queue *queue;

queue = to_ap_queue(&apdev->device);

q = kzalloc(sizeof(*q), GFP_KERNEL);
if (!q)
return -ENOMEM;

dev_set_drvdata(&queue->ap_dev.device, q);
q->apqn = queue->qid;
q->saved_isc = VFIO_AP_ISC_INVALID;

return 0;
}

void vfio_ap_mdev_remove_queue(struct ap_device *apdev)
{
struct vfio_ap_queue *q;
struct ap_queue *queue;
int apid, apqi;

queue = to_ap_queue(&apdev->device);

mutex_lock(&matrix_dev->lock);
q = dev_get_drvdata(&queue->ap_dev.device);
dev_set_drvdata(&queue->ap_dev.device, NULL);
apid = AP_QID_CARD(q->apqn);
apqi = AP_QID_QUEUE(q->apqn);
vfio_ap_mdev_reset_queue(apid, apqi, 1);
vfio_ap_free_aqic_resources(q);
kfree(q);
mutex_unlock(&matrix_dev->lock);
}
7 changes: 4 additions & 3 deletions drivers/s390/crypto/vfio_ap_private.h
View file
Expand Up @@ -90,8 +90,6 @@ struct ap_matrix_mdev {

extern int vfio_ap_mdev_register(void);
extern void vfio_ap_mdev_unregister(void);
int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
unsigned int retry);

struct vfio_ap_queue {
struct ap_matrix_mdev *matrix_mdev;
Expand All @@ -100,5 +98,8 @@ struct vfio_ap_queue {
#define VFIO_AP_ISC_INVALID 0xff
unsigned char saved_isc;
};
struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q);

int vfio_ap_mdev_probe_queue(struct ap_device *queue);
void vfio_ap_mdev_remove_queue(struct ap_device *queue);

#endif /* _VFIO_AP_PRIVATE_H_ */

0 comments on commit 572c94c

Please sign in to comment.