qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed
@ 2023-05-30 22:55 Tony Krowiak
  2023-05-30 22:55 ` [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping Tony Krowiak
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Tony Krowiak @ 2023-05-30 22:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-s390x, mjrosato, jjherne, pasic, fiuczy, thuth, farman,
	borntraeger, Tony Krowiak

When a user attempts to remove a vfio-ap mediated device attached to a
guest, the operation hangs until the mdev's fd is closed by the guest
(i.e., the guest is shut down). This patch series provides userspace 
(i.e., qemu) code to handle device unplug requests for AP. When notified
that the mdev is being removed, the handler will unplug the device, thus
avoiding the hang condition.

Tony Krowiak (2):
  linux-headers: Update with vfio_ap IRQ index mapping
  s390x/ap: Wire up the device request notifier interface

 hw/vfio/ap.c               | 113 +++++++++++++++++++++++++++++++++++++
 linux-headers/linux/vfio.h |   9 +++
 2 files changed, 122 insertions(+)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping
  2023-05-30 22:55 [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed Tony Krowiak
@ 2023-05-30 22:55 ` Tony Krowiak
  2023-05-31  0:56   ` Matthew Rosato
  2023-05-30 22:55 ` [PATCH 2/2] s390x/ap: Wire up the device request notifier interface Tony Krowiak
  2023-05-31 12:40 ` [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed Anthony Krowiak
  2 siblings, 1 reply; 10+ messages in thread
From: Tony Krowiak @ 2023-05-30 22:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-s390x, mjrosato, jjherne, pasic, fiuczy, thuth, farman,
	borntraeger, Tony Krowiak

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
---
 linux-headers/linux/vfio.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 4a534edbdcba..2658fda219e8 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -646,6 +646,15 @@ enum {
 	VFIO_CCW_NUM_IRQS
 };
 
+/*
+ * The vfio-ap bus driver makes use of the following IRQ index mapping.
+ * Unimplemented IRQ types return a count of zero.
+ */
+enum {
+	VFIO_AP_REQ_IRQ_INDEX,
+	VFIO_AP_NUM_IRQS
+};
+
 /**
  * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 12,
  *					      struct vfio_pci_hot_reset_info)
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] s390x/ap: Wire up the device request notifier interface
  2023-05-30 22:55 [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed Tony Krowiak
  2023-05-30 22:55 ` [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping Tony Krowiak
@ 2023-05-30 22:55 ` Tony Krowiak
  2023-05-31 12:40 ` [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed Anthony Krowiak
  2 siblings, 0 replies; 10+ messages in thread
From: Tony Krowiak @ 2023-05-30 22:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-s390x, mjrosato, jjherne, pasic, fiuczy, thuth, farman,
	borntraeger, Tony Krowiak

Let's wire up the device request notifier interface to handle device unplug
requests for AP.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
---
 hw/vfio/ap.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index e0dd561e85a3..6e21d1da5a70 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -18,6 +18,8 @@
 #include "hw/vfio/vfio-common.h"
 #include "hw/s390x/ap-device.h"
 #include "qemu/error-report.h"
+#include "qemu/event_notifier.h"
+#include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
 #include "qemu/config-file.h"
@@ -33,6 +35,7 @@
 struct VFIOAPDevice {
     APDevice apdev;
     VFIODevice vdev;
+    EventNotifier req_notifier;
 };
 
 OBJECT_DECLARE_SIMPLE_TYPE(VFIOAPDevice, VFIO_AP_DEVICE)
@@ -84,10 +87,110 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
     return vfio_get_group(groupid, &address_space_memory, errp);
 }
 
+static void vfio_ap_req_notifier_handler(void *opaque)
+{
+    VFIOAPDevice *vapdev = opaque;
+    Error *err = NULL;
+
+    if (!event_notifier_test_and_clear(&vapdev->req_notifier)) {
+        return;
+    }
+
+    qdev_unplug(DEVICE(vapdev), &err);
+
+    if (err) {
+        warn_reportf_err(err, VFIO_MSG_PREFIX, vapdev->vdev.name);
+    }
+}
+
+static void vfio_ap_register_irq_notifier(VFIOAPDevice *vapdev,
+                                          unsigned int irq, Error **errp)
+{
+    int fd;
+    size_t argsz;
+    IOHandler *fd_read;
+    EventNotifier *notifier;
+    struct vfio_irq_info *irq_info;
+    VFIODevice *vdev = &vapdev->vdev;
+
+    switch (irq) {
+    case VFIO_AP_REQ_IRQ_INDEX:
+        notifier = &vapdev->req_notifier;
+        fd_read = vfio_ap_req_notifier_handler;
+        break;
+    default:
+        error_setg(errp, "vfio: Unsupported device irq(%d)", irq);
+        return;
+    }
+
+    if (vdev->num_irqs < irq + 1) {
+        error_setg(errp, "vfio: IRQ %u not available (number of irqs %u)",
+                   irq, vdev->num_irqs);
+        return;
+    }
+
+    argsz = sizeof(*irq_info);
+    irq_info = g_malloc0(argsz);
+    irq_info->index = irq;
+    irq_info->argsz = argsz;
+
+    if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
+              irq_info) < 0 || irq_info->count < 1) {
+        error_setg_errno(errp, errno, "vfio: Error getting irq info");
+        goto out_free_info;
+    }
+
+    if (event_notifier_init(notifier, 0)) {
+        error_setg_errno(errp, errno,
+                         "vfio: Unable to init event notifier for irq (%d)",
+                         irq);
+        goto out_free_info;
+    }
+
+    fd = event_notifier_get_fd(notifier);
+    qemu_set_fd_handler(fd, fd_read, NULL, vapdev);
+
+    if (vfio_set_irq_signaling(vdev, irq, 0, VFIO_IRQ_SET_ACTION_TRIGGER, fd,
+                               errp)) {
+        qemu_set_fd_handler(fd, NULL, NULL, vapdev);
+        event_notifier_cleanup(notifier);
+    }
+
+out_free_info:
+    g_free(irq_info);
+
+}
+
+static void vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev,
+                                            unsigned int irq)
+{
+    Error *err = NULL;
+    EventNotifier *notifier;
+
+    switch (irq) {
+    case VFIO_AP_REQ_IRQ_INDEX:
+        notifier = &vapdev->req_notifier;
+        break;
+    default:
+        error_report("vfio: Unsupported device irq(%d)", irq);
+        return;
+    }
+
+    if (vfio_set_irq_signaling(&vapdev->vdev, irq, 0,
+                               VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) {
+        warn_reportf_err(err, VFIO_MSG_PREFIX, vapdev->vdev.name);
+    }
+
+    qemu_set_fd_handler(event_notifier_get_fd(notifier),
+                        NULL, NULL, vapdev);
+    event_notifier_cleanup(notifier);
+}
+
 static void vfio_ap_realize(DeviceState *dev, Error **errp)
 {
     int ret;
     char *mdevid;
+    Error *err = NULL;
     VFIOGroup *vfio_group;
     APDevice *apdev = AP_DEVICE(dev);
     VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
@@ -116,6 +219,15 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
         goto out_get_dev_err;
     }
 
+    vfio_ap_register_irq_notifier(vapdev, VFIO_AP_REQ_IRQ_INDEX, &err);
+    if (err) {
+        /*
+         * Report this error, but do not make it a failing condition.
+         * Lack of this IRQ in the host does not prevent normal operation.
+         */
+        error_report_err(err);
+    }
+
     return;
 
 out_get_dev_err:
@@ -129,6 +241,7 @@ static void vfio_ap_unrealize(DeviceState *dev)
     VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
     VFIOGroup *group = vapdev->vdev.group;
 
+    vfio_ap_unregister_irq_notifier(vapdev, VFIO_AP_REQ_IRQ_INDEX);
     vfio_ap_put_device(vapdev);
     vfio_put_group(group);
 }
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping
  2023-05-30 22:55 ` [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping Tony Krowiak
@ 2023-05-31  0:56   ` Matthew Rosato
  2023-05-31 12:52     ` Anthony Krowiak
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Rosato @ 2023-05-31  0:56 UTC (permalink / raw)
  To: Tony Krowiak, qemu-devel
  Cc: qemu-s390x, jjherne, pasic, fiuczy, thuth, farman, borntraeger

On 5/30/23 6:55 PM, Tony Krowiak wrote:
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> ---
>  linux-headers/linux/vfio.h | 9 +++++++++
>  1 file changed, 9 insertions(+)

Worth nothing here that linux-headers patches should be generated using scripts/update-linux-headers.sh.

Since this linux-headers update includes changes that aren't merged into the kernel yet, I would still use update-linux-headers.sh -- but also include in the commit message that this is a placeholder patch that includes unmerged uapi changes.  Then once the kernel changes merge you can just have a proper linux-headers update patch in a subsequent qemu series.

> 
> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> index 4a534edbdcba..2658fda219e8 100644
> --- a/linux-headers/linux/vfio.h
> +++ b/linux-headers/linux/vfio.h
> @@ -646,6 +646,15 @@ enum {
>  	VFIO_CCW_NUM_IRQS
>  };
>  
> +/*
> + * The vfio-ap bus driver makes use of the following IRQ index mapping.
> + * Unimplemented IRQ types return a count of zero.
> + */
> +enum {
> +	VFIO_AP_REQ_IRQ_INDEX,
> +	VFIO_AP_NUM_IRQS
> +};
> +
>  /**
>   * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 12,
>   *					      struct vfio_pci_hot_reset_info)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed
  2023-05-30 22:55 [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed Tony Krowiak
  2023-05-30 22:55 ` [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping Tony Krowiak
  2023-05-30 22:55 ` [PATCH 2/2] s390x/ap: Wire up the device request notifier interface Tony Krowiak
@ 2023-05-31 12:40 ` Anthony Krowiak
  2 siblings, 0 replies; 10+ messages in thread
From: Anthony Krowiak @ 2023-05-31 12:40 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-s390x, mjrosato, jjherne, pasic, fiuczy, thuth, farman,
	borntraeger

The required kernel changes associated with the patch are here:

https://lore.kernel.org/linux-s390/20230530223538.279198-1-akrowiak@linux.ibm.com/

On 5/30/23 6:55 PM, Tony Krowiak wrote:
> When a user attempts to remove a vfio-ap mediated device attached to a
> guest, the operation hangs until the mdev's fd is closed by the guest
> (i.e., the guest is shut down). This patch series provides userspace
> (i.e., qemu) code to handle device unplug requests for AP. When notified
> that the mdev is being removed, the handler will unplug the device, thus
> avoiding the hang condition.
> 
> Tony Krowiak (2):
>    linux-headers: Update with vfio_ap IRQ index mapping
>    s390x/ap: Wire up the device request notifier interface
> 
>   hw/vfio/ap.c               | 113 +++++++++++++++++++++++++++++++++++++
>   linux-headers/linux/vfio.h |   9 +++
>   2 files changed, 122 insertions(+)
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping
  2023-05-31  0:56   ` Matthew Rosato
@ 2023-05-31 12:52     ` Anthony Krowiak
  2023-05-31 13:07       ` Matthew Rosato
  2023-05-31 13:07       ` Cornelia Huck
  0 siblings, 2 replies; 10+ messages in thread
From: Anthony Krowiak @ 2023-05-31 12:52 UTC (permalink / raw)
  To: Matthew Rosato, qemu-devel
  Cc: qemu-s390x, jjherne, pasic, fiuczy, thuth, farman, borntraeger



On 5/30/23 8:56 PM, Matthew Rosato wrote:
> On 5/30/23 6:55 PM, Tony Krowiak wrote:
>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   linux-headers/linux/vfio.h | 9 +++++++++
>>   1 file changed, 9 insertions(+)
> 
> Worth nothing here that linux-headers patches should be generated using scripts/update-linux-headers.sh.
> 
> Since this linux-headers update includes changes that aren't merged into the kernel yet, I would still use update-linux-headers.sh -- but also include in the commit message that this is a placeholder patch that includes unmerged uapi changes.  Then once the kernel changes merge you can just have a proper linux-headers update patch in a subsequent qemu series.

I guess I do not understand the procedure here. I first determined the 
latest kernel release in which the vfio.h file was updated with the 
following command:
git log --oneline origin/master -- linux-headers/linux/vfio.h

According to the git log, the vfio.h file was last updated in kernel 
v6.3-rc5. I cloned that kernel from 
git.kernel.org/pub/scm/linux/kernel/git/stable and checked out kernel 
6.3-rc5. I then made the changes to the linux-headers/linux/vfio.h file 
and ran the update-linux-headers.sh script and created this patch from 
that. Where did I go wrong?

> 
>>
>> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
>> index 4a534edbdcba..2658fda219e8 100644
>> --- a/linux-headers/linux/vfio.h
>> +++ b/linux-headers/linux/vfio.h
>> @@ -646,6 +646,15 @@ enum {
>>   	VFIO_CCW_NUM_IRQS
>>   };
>>   
>> +/*
>> + * The vfio-ap bus driver makes use of the following IRQ index mapping.
>> + * Unimplemented IRQ types return a count of zero.
>> + */
>> +enum {
>> +	VFIO_AP_REQ_IRQ_INDEX,
>> +	VFIO_AP_NUM_IRQS
>> +};
>> +
>>   /**
>>    * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 12,
>>    *					      struct vfio_pci_hot_reset_info)
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping
  2023-05-31 12:52     ` Anthony Krowiak
@ 2023-05-31 13:07       ` Matthew Rosato
  2023-05-31 13:12         ` Anthony Krowiak
  2023-05-31 13:07       ` Cornelia Huck
  1 sibling, 1 reply; 10+ messages in thread
From: Matthew Rosato @ 2023-05-31 13:07 UTC (permalink / raw)
  To: Anthony Krowiak, qemu-devel
  Cc: qemu-s390x, jjherne, pasic, fiuczy, thuth, farman, borntraeger

On 5/31/23 8:52 AM, Anthony Krowiak wrote:
> 
> 
> On 5/30/23 8:56 PM, Matthew Rosato wrote:
>> On 5/30/23 6:55 PM, Tony Krowiak wrote:
>>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>>> ---
>>>   linux-headers/linux/vfio.h | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>
>> Worth nothing here that linux-headers patches should be generated using scripts/update-linux-headers.sh.
>>
>> Since this linux-headers update includes changes that aren't merged into the kernel yet, I would still use update-linux-headers.sh -- but also include in the commit message that this is a placeholder patch that includes unmerged uapi changes.  Then once the kernel changes merge you can just have a proper linux-headers update patch in a subsequent qemu series.
> 
> I guess I do not understand the procedure here. I first determined the latest kernel release in which the vfio.h file was updated with the following command:
> git log --oneline origin/master -- linux-headers/linux/vfio.h
> 
> According to the git log, the vfio.h file was last updated in kernel v6.3-rc5. I cloned that kernel from git.kernel.org/pub/scm/linux/kernel/git/stable and checked out kernel 6.3-rc5. I then made the changes to the linux-headers/linux/vfio.h file and ran the update-linux-headers.sh script and created this patch from that. Where did I go wrong?

Presumably your kernel series that you just posted was built on top of 6.4-rc4, not v6.3-rc5 (if it's not, you should rebase onto a recent kernel like 6.4-rc4).  Then, you want to point update-linux-headers.sh at that source repository which includes your changes.  This will pull in all of the changes to the uapi up to kernel 6.4-rc* + your additional unmerged changes.  FWIW, I just pointed update-linux-headers.sh at kernel master from today and I got the following:

---
 include/standard-headers/linux/const.h        |  2 +-
 include/standard-headers/linux/virtio_blk.h   | 18 +++----
 .../standard-headers/linux/virtio_config.h    |  6 +++
 include/standard-headers/linux/virtio_net.h   |  1 +
 linux-headers/asm-arm64/kvm.h                 | 33 ++++++++++++
 linux-headers/asm-riscv/kvm.h                 | 53 ++++++++++++++++++-
 linux-headers/asm-riscv/unistd.h              |  9 ++++
 linux-headers/asm-s390/unistd_32.h            |  1 +
 linux-headers/asm-s390/unistd_64.h            |  1 +
 linux-headers/asm-x86/kvm.h                   |  3 ++
 linux-headers/linux/const.h                   |  2 +-
 linux-headers/linux/kvm.h                     | 12 +++--
 linux-headers/linux/psp-sev.h                 |  7 +++
 linux-headers/linux/userfaultfd.h             | 17 +++++-
 14 files changed, 149 insertions(+), 16 deletions(-)
---

In your case you would also see an additional line for linux-headers/linux/vfio.h, which would be your unmerged kernel uapi changes.

Then you can include a cover letter something like:

This is a placeholder that pulls in 6.4-rc4 + unmerged kernel changes
required by this series.  A proper header sync can be done once the
associated kernel code merges.

Here's an example from an old series where I did this before:

https://lore.kernel.org/qemu-devel/20220606203614.110928-2-mjrosato@linux.ibm.com/

One of the main advantages of doing it this way is that if there are any uapi breakages unrelated to your patch we catch them now.  That helps whoever might take your series (e.g. Thomas) avoid having to deal with the fallout later when sending a pull request.

> 
>>
>>>
>>> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
>>> index 4a534edbdcba..2658fda219e8 100644
>>> --- a/linux-headers/linux/vfio.h
>>> +++ b/linux-headers/linux/vfio.h
>>> @@ -646,6 +646,15 @@ enum {
>>>       VFIO_CCW_NUM_IRQS
>>>   };
>>>   +/*
>>> + * The vfio-ap bus driver makes use of the following IRQ index mapping.
>>> + * Unimplemented IRQ types return a count of zero.
>>> + */
>>> +enum {
>>> +    VFIO_AP_REQ_IRQ_INDEX,
>>> +    VFIO_AP_NUM_IRQS
>>> +};
>>> +
>>>   /**
>>>    * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 12,
>>>    *                          struct vfio_pci_hot_reset_info)
>>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping
  2023-05-31 12:52     ` Anthony Krowiak
  2023-05-31 13:07       ` Matthew Rosato
@ 2023-05-31 13:07       ` Cornelia Huck
  2023-05-31 13:21         ` Anthony Krowiak
  1 sibling, 1 reply; 10+ messages in thread
From: Cornelia Huck @ 2023-05-31 13:07 UTC (permalink / raw)
  To: Anthony Krowiak, Matthew Rosato, qemu-devel
  Cc: qemu-s390x, jjherne, pasic, fiuczy, thuth, farman, borntraeger

On Wed, May 31 2023, Anthony Krowiak <akrowiak@linux.ibm.com> wrote:

> On 5/30/23 8:56 PM, Matthew Rosato wrote:
>> On 5/30/23 6:55 PM, Tony Krowiak wrote:
>>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>>> ---
>>>   linux-headers/linux/vfio.h | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>> 
>> Worth nothing here that linux-headers patches should be generated using scripts/update-linux-headers.sh.
>> 
>> Since this linux-headers update includes changes that aren't merged into the kernel yet, I would still use update-linux-headers.sh -- but also include in the commit message that this is a placeholder patch that includes unmerged uapi changes.  Then once the kernel changes merge you can just have a proper linux-headers update patch in a subsequent qemu series.
>
> I guess I do not understand the procedure here. I first determined the 
> latest kernel release in which the vfio.h file was updated with the 
> following command:
> git log --oneline origin/master -- linux-headers/linux/vfio.h
>
> According to the git log, the vfio.h file was last updated in kernel 
> v6.3-rc5. I cloned that kernel from 
> git.kernel.org/pub/scm/linux/kernel/git/stable and checked out kernel 
> 6.3-rc5. I then made the changes to the linux-headers/linux/vfio.h file 
> and ran the update-linux-headers.sh script and created this patch from 
> that. Where did I go wrong?

I think your procedure is fine for changes that are local to a single
header file. The one thing I'd recommend is to put an explicit "dummy
patch, to be replaced by a proper headers update" note into the patch,
so that it doesn't get merged by accident.

(For complex changes, headers update + explicit note might be better,
but the simple approach works in most cases.)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping
  2023-05-31 13:07       ` Matthew Rosato
@ 2023-05-31 13:12         ` Anthony Krowiak
  0 siblings, 0 replies; 10+ messages in thread
From: Anthony Krowiak @ 2023-05-31 13:12 UTC (permalink / raw)
  To: Matthew Rosato, qemu-devel
  Cc: qemu-s390x, jjherne, pasic, fiuczy, thuth, farman, borntraeger



On 5/31/23 9:07 AM, Matthew Rosato wrote:
> On 5/31/23 8:52 AM, Anthony Krowiak wrote:
>>
>>
>> On 5/30/23 8:56 PM, Matthew Rosato wrote:
>>> On 5/30/23 6:55 PM, Tony Krowiak wrote:
>>>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>>>> ---
>>>>    linux-headers/linux/vfio.h | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)
>>>
>>> Worth nothing here that linux-headers patches should be generated using scripts/update-linux-headers.sh.
>>>
>>> Since this linux-headers update includes changes that aren't merged into the kernel yet, I would still use update-linux-headers.sh -- but also include in the commit message that this is a placeholder patch that includes unmerged uapi changes.  Then once the kernel changes merge you can just have a proper linux-headers update patch in a subsequent qemu series.
>>
>> I guess I do not understand the procedure here. I first determined the latest kernel release in which the vfio.h file was updated with the following command:
>> git log --oneline origin/master -- linux-headers/linux/vfio.h
>>
>> According to the git log, the vfio.h file was last updated in kernel v6.3-rc5. I cloned that kernel from git.kernel.org/pub/scm/linux/kernel/git/stable and checked out kernel 6.3-rc5. I then made the changes to the linux-headers/linux/vfio.h file and ran the update-linux-headers.sh script and created this patch from that. Where did I go wrong?
> 
> Presumably your kernel series that you just posted was built on top of 6.4-rc4, not v6.3-rc5 (if it's not, you should rebase onto a recent kernel like 6.4-rc4).  Then, you want to point update-linux-headers.sh at that source repository which includes your changes.  This will pull in all of the changes to the uapi up to kernel 6.4-rc* + your additional unmerged changes.  FWIW, I just pointed update-linux-headers.sh at kernel master from today and I got the following:
> 
> ---
>   include/standard-headers/linux/const.h        |  2 +-
>   include/standard-headers/linux/virtio_blk.h   | 18 +++----
>   .../standard-headers/linux/virtio_config.h    |  6 +++
>   include/standard-headers/linux/virtio_net.h   |  1 +
>   linux-headers/asm-arm64/kvm.h                 | 33 ++++++++++++
>   linux-headers/asm-riscv/kvm.h                 | 53 ++++++++++++++++++-
>   linux-headers/asm-riscv/unistd.h              |  9 ++++
>   linux-headers/asm-s390/unistd_32.h            |  1 +
>   linux-headers/asm-s390/unistd_64.h            |  1 +
>   linux-headers/asm-x86/kvm.h                   |  3 ++
>   linux-headers/linux/const.h                   |  2 +-
>   linux-headers/linux/kvm.h                     | 12 +++--
>   linux-headers/linux/psp-sev.h                 |  7 +++
>   linux-headers/linux/userfaultfd.h             | 17 +++++-
>   14 files changed, 149 insertions(+), 16 deletions(-)
> ---
> 
> In your case you would also see an additional line for linux-headers/linux/vfio.h, which would be your unmerged kernel uapi changes.
> 
> Then you can include a cover letter something like:
> 
> This is a placeholder that pulls in 6.4-rc4 + unmerged kernel changes
> required by this series.  A proper header sync can be done once the
> associated kernel code merges.
> 
> Here's an example from an old series where I did this before:
> 
> https://lore.kernel.org/qemu-devel/20220606203614.110928-2-mjrosato@linux.ibm.com/
> 
> One of the main advantages of doing it this way is that if there are any uapi breakages unrelated to your patch we catch them now.  That helps whoever might take your series (e.g. Thomas) avoid having to deal with the fallout later when sending a pull request.

Thanks Matt, I'll do this for v2.

> 
>>
>>>
>>>>
>>>> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
>>>> index 4a534edbdcba..2658fda219e8 100644
>>>> --- a/linux-headers/linux/vfio.h
>>>> +++ b/linux-headers/linux/vfio.h
>>>> @@ -646,6 +646,15 @@ enum {
>>>>        VFIO_CCW_NUM_IRQS
>>>>    };
>>>>    +/*
>>>> + * The vfio-ap bus driver makes use of the following IRQ index mapping.
>>>> + * Unimplemented IRQ types return a count of zero.
>>>> + */
>>>> +enum {
>>>> +    VFIO_AP_REQ_IRQ_INDEX,
>>>> +    VFIO_AP_NUM_IRQS
>>>> +};
>>>> +
>>>>    /**
>>>>     * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 12,
>>>>     *                          struct vfio_pci_hot_reset_info)
>>>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping
  2023-05-31 13:07       ` Cornelia Huck
@ 2023-05-31 13:21         ` Anthony Krowiak
  0 siblings, 0 replies; 10+ messages in thread
From: Anthony Krowiak @ 2023-05-31 13:21 UTC (permalink / raw)
  To: Cornelia Huck, Matthew Rosato, qemu-devel
  Cc: qemu-s390x, jjherne, pasic, fiuczy, thuth, farman, borntraeger



On 5/31/23 9:07 AM, Cornelia Huck wrote:
> On Wed, May 31 2023, Anthony Krowiak <akrowiak@linux.ibm.com> wrote:
> 
>> On 5/30/23 8:56 PM, Matthew Rosato wrote:
>>> On 5/30/23 6:55 PM, Tony Krowiak wrote:
>>>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>>>> ---
>>>>    linux-headers/linux/vfio.h | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)
>>>
>>> Worth nothing here that linux-headers patches should be generated using scripts/update-linux-headers.sh.
>>>
>>> Since this linux-headers update includes changes that aren't merged into the kernel yet, I would still use update-linux-headers.sh -- but also include in the commit message that this is a placeholder patch that includes unmerged uapi changes.  Then once the kernel changes merge you can just have a proper linux-headers update patch in a subsequent qemu series.
>>
>> I guess I do not understand the procedure here. I first determined the
>> latest kernel release in which the vfio.h file was updated with the
>> following command:
>> git log --oneline origin/master -- linux-headers/linux/vfio.h
>>
>> According to the git log, the vfio.h file was last updated in kernel
>> v6.3-rc5. I cloned that kernel from
>> git.kernel.org/pub/scm/linux/kernel/git/stable and checked out kernel
>> 6.3-rc5. I then made the changes to the linux-headers/linux/vfio.h file
>> and ran the update-linux-headers.sh script and created this patch from
>> that. Where did I go wrong?
> 
> I think your procedure is fine for changes that are local to a single
> header file. The one thing I'd recommend is to put an explicit "dummy
> patch, to be replaced by a proper headers update" note into the patch,
> so that it doesn't get merged by accident.
> 
> (For complex changes, headers update + explicit note might be better,
> but the simple approach works in most cases.)

Will do.

> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-05-31 13:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-30 22:55 [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed Tony Krowiak
2023-05-30 22:55 ` [PATCH 1/2] linux-headers: Update with vfio_ap IRQ index mapping Tony Krowiak
2023-05-31  0:56   ` Matthew Rosato
2023-05-31 12:52     ` Anthony Krowiak
2023-05-31 13:07       ` Matthew Rosato
2023-05-31 13:12         ` Anthony Krowiak
2023-05-31 13:07       ` Cornelia Huck
2023-05-31 13:21         ` Anthony Krowiak
2023-05-30 22:55 ` [PATCH 2/2] s390x/ap: Wire up the device request notifier interface Tony Krowiak
2023-05-31 12:40 ` [PATCH 0/2] s390x/ap: fix hang when mdev attached to guest is removed Anthony Krowiak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).