On 3/28/19 9:06 AM, Pierre Morel wrote:
On 26/03/2019 21:45, Tony Krowiak wrote:
On 3/22/19 10:43 AM, Pierre Morel wrote:
The AP interruptions are assigned on a queue basis and
the GISA structure is handled on a VM basis, so that
we need to add a structure we can retrieve from both side
s/side/sides/
OK
holding the information we need to handle PQAP/AQIC interception
and setup the GISA.
s/setup/set up/
OK
...snip...
+
+static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q)
+{
+ struct ap_queue_status status;
+ int retry = 1;
+
+ do {
+ status = ap_zapq(q->apqn);
+ switch (status.response_code) {
+ case AP_RESPONSE_NORMAL:
+ return 0;
+ case AP_RESPONSE_RESET_IN_PROGRESS:
+ case AP_RESPONSE_BUSY:
+ msleep(20);
+ break;
+ default:
+ /* things are really broken, give up */
I'm not sure things are necessarily broken. We could end up here if
the AP is removed from the configuration via the SE or SCLP Deconfigure
Adjunct Processor command.
OK, but note that it is your original comment I just moved the function
here ;)
Yes, it is. I'm smarter now;)
+ return -EIO;
+ }
+ } while (retry--);
+
+ return -EBUSY;
+}
+
static void vfio_ap_matrix_init(struct ap_config_info *info,
struct ap_matrix *matrix)
{
@@ -45,6 +107,7 @@ static int vfio_ap_mdev_create(struct kobject
*kobj, struct mdev_device *mdev)
return -ENOMEM;
}
+ INIT_LIST_HEAD(&matrix_mdev->qlist);
vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->matrix);
mdev_set_drvdata(mdev, matrix_mdev);
mutex_lock(&matrix_dev->lock);
@@ -113,162 +176,189 @@ static struct attribute_group
*vfio_ap_mdev_type_groups[] = {
NULL,
};
-struct vfio_ap_queue_reserved {
- unsigned long *apid;
- unsigned long *apqi;
- bool reserved;
-};
+static void vfio_ap_free_queue(int apqn, struct ap_matrix_mdev
*matrix_mdev)
+{
+ struct vfio_ap_queue *q;
+
+ q = vfio_ap_get_queue(apqn, &matrix_mdev->qlist);
+ if (!q)
+ return;
+ q->matrix_mdev = NULL;
+ vfio_ap_mdev_reset_queue(q);
I'm wondering if it's necessary to reset the queue here. The only time
a queue is used is when a guest using the mdev device is started. When
that guest is terminated, the fd for the mdev device is /* Bits 41-47 must all be zeros */closed and the
mdev device's release callback is invoked. The release callback resets
the queues assigned to the mdev device. Is it really necessary to
reset the queue again when it is unassigned even if there would have
been no subsequent activity?
Yes, it is necessary, the queue can be re-assigned to another guest later.
Release will only be called when unbinding the queue from the driver.
That is true, but if the queue is never used, there is nothing to reset.
+ list_move(&q->list, &matrix_dev->free_list);
+}
...snip...
+ for_each_set_bit_inv(apid, matrix_mdev->matrix.apm, AP_DEVICES) {
+ apqn = AP_MKQID(apid, apqi);
+ q = vfio_ap_find_queue(apqn);
+ if (!q) {
+ ret = -EADDRNOTAVAIL;
+ goto rewind;
+ }
+ if (q->matrix_mdev) {
If somebody assigns the same domain a second time, the assignment will
fail because the matrix_mdev will already have been associated with the
queue. I don't think it is appropriate to fail the assignment if the
It is usual to report a failure in the case the operation requested has
already be done.
But we can do as you want. Any other opinion?
q->matrix_mdev is the same as the input matrix_mdev. This should be
changed to:
if (q->matrix_mdev != matrix_mdev)
You surely want to say: add this, not change to this. ;)
Yes
Thanks for commenting,
Regards,
Pierre
