[PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Consider the following nested setup: An L1 host uses some virtio device
(e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
device through to the L3 guest. Since the L3 guest sees a virtio device,
it might send virtio notifications to the QEMU in L2 for that device.
But since the QEMU in L2 defined this device as vfio-ccw, the function
handle_virtio_ccw_notify() cannot handle this and crashes: It calls
virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
but since "sch" belongs to a vfio-ccw device, that driver_data rather
points to a CcwDevice instead. So as soon as QEMU tries to use some
VirtioCcwDevice specific data from that device, we've lost.

We must not take virtio notifications for such devices. Thus fix the
issue by adding a check to the handle_virtio_ccw_notify() handler to
refuse all devices that are not our own virtio devices.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2: Now with the required #include statement

 hw/s390x/s390-hypercall.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/s390x/s390-hypercall.c b/hw/s390x/s390-hypercall.c
index ac1b08b2cd5..38f1c6132e0 100644
--- a/hw/s390x/s390-hypercall.c
+++ b/hw/s390x/s390-hypercall.c
@@ -10,6 +10,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "cpu.h"
 #include "hw/s390x/s390-virtio-ccw.h"
 #include "hw/s390x/s390-hypercall.h"
@@ -42,6 +43,18 @@ static int handle_virtio_ccw_notify(uint64_t subch_id, uint64_t data)
     if (!sch || !css_subch_visible(sch)) {
         return -EINVAL;
     }
+    if (sch->id.cu_type != VIRTIO_CCW_CU_TYPE) {
+        /*
+         * This might happen in nested setups: If the L1 host defined the
+         * L2 guest with a virtio device (e.g. virtio-keyboard), and the
+         * L2 guest passes this device through to the L3 guest, the L3 guest
+         * might send virtio notifications to the QEMU in L2 for that device.
+         * But since the QEMU in L2 defined this device as vfio-ccw, it's not
+         * a VirtIODevice that we can handle here!
+         */
+        warn_report_once("Got virtio notification for unsupported device!");
+        return -EINVAL;
+    }
 
     vdev = virtio_ccw_get_vdev(sch);
     if (vq_idx >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, vq_idx)) {
-- 
2.51.1

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Cornelia Huck]

On Tue, Nov 18 2025, Thomas Huth <thuth@redhat.com> wrote:

> From: Thomas Huth <thuth@redhat.com>
>
> Consider the following nested setup: An L1 host uses some virtio device
> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
> device through to the L3 guest. Since the L3 guest sees a virtio device,
> it might send virtio notifications to the QEMU in L2 for that device.
> But since the QEMU in L2 defined this device as vfio-ccw, the function
> handle_virtio_ccw_notify() cannot handle this and crashes: It calls
> virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
> but since "sch" belongs to a vfio-ccw device, that driver_data rather
> points to a CcwDevice instead. So as soon as QEMU tries to use some
> VirtioCcwDevice specific data from that device, we've lost.
>
> We must not take virtio notifications for such devices. Thus fix the
> issue by adding a check to the handle_virtio_ccw_notify() handler to
> refuse all devices that are not our own virtio devices.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Now with the required #include statement
>
>  hw/s390x/s390-hypercall.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/hw/s390x/s390-hypercall.c b/hw/s390x/s390-hypercall.c
> index ac1b08b2cd5..38f1c6132e0 100644
> --- a/hw/s390x/s390-hypercall.c
> +++ b/hw/s390x/s390-hypercall.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>  #include "cpu.h"
>  #include "hw/s390x/s390-virtio-ccw.h"
>  #include "hw/s390x/s390-hypercall.h"
> @@ -42,6 +43,18 @@ static int handle_virtio_ccw_notify(uint64_t subch_id, uint64_t data)
>      if (!sch || !css_subch_visible(sch)) {
>          return -EINVAL;
>      }
> +    if (sch->id.cu_type != VIRTIO_CCW_CU_TYPE) {
> +        /*
> +         * This might happen in nested setups: If the L1 host defined the
> +         * L2 guest with a virtio device (e.g. virtio-keyboard), and the
> +         * L2 guest passes this device through to the L3 guest, the L3 guest
> +         * might send virtio notifications to the QEMU in L2 for that device.
> +         * But since the QEMU in L2 defined this device as vfio-ccw, it's not
> +         * a VirtIODevice that we can handle here!
> +         */
> +        warn_report_once("Got virtio notification for unsupported device!");

Maybe also print which device ended up here?

> +        return -EINVAL;
> +    }
>  
>      vdev = virtio_ccw_get_vdev(sch);
>      if (vq_idx >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, vq_idx)) {

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Halil Pasic]

On Tue, 18 Nov 2025 10:39:45 +0100
Thomas Huth <thuth@redhat.com> wrote:

> Consider the following nested setup: An L1 host uses some virtio device
> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
> device through to the L3 guest. Since the L3 guest sees a virtio device,
> it might send virtio notifications to the QEMU in L2 for that device.

Hm, but conceptually the notification is sent to the virtio device,
regardless of hypervisors, right? But because for virtio-ccw the
notification is an DIAG 500, we have the usual cascade of intercept
handling. And because we have never considered this scenario up till now
the intercept handler in L2 QEMU gets called, because it is usually the
responsibility of L2 QEMU to emulate instructions for an L3 guest.

I think vfio-ccw pass through was supposed to be only about DASD. 

> But since the QEMU in L2 defined this device as vfio-ccw, the function
> handle_virtio_ccw_notify() cannot handle this and crashes: It calls
> virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
> but since "sch" belongs to a vfio-ccw device, that driver_data rather
> points to a CcwDevice instead. So as soon as QEMU tries to use some
> VirtioCcwDevice specific data from that device, we've lost.
> 
> We must not take virtio notifications for such devices. Thus fix the
> issue by adding a check to the handle_virtio_ccw_notify() handler to
> refuse all devices that are not our own virtio devices.

I'm on board with this patch! Virtio notifications are only supported
for virtio devices and if a guest for what ever reason attempts
to do a virtio notification on a non-virtio device, that should be
handled accordingly. Which would be some sort of a program exception
I guess. Maybe you could add what kind of exception do we end up
with to the commit message. I would guess specification exception.

But I would argue that the L3 guest didn't do anything wrong.
Pass-through of virtio-ccw devices is simply not implemented yet
properly. And even  if we were to swallow that notification silently,
it would be effectively loss of initiative I guess.

So I think it would really make sense to prevent passing through
virtio-ccw devices with vfio-ccw. Eric what is your take?

Regards,
Halil

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Thomas Huth]

On 18/11/2025 12.52, Cornelia Huck wrote:
> On Tue, Nov 18 2025, Thomas Huth <thuth@redhat.com> wrote:
> 
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Consider the following nested setup: An L1 host uses some virtio device
>> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
>> device through to the L3 guest. Since the L3 guest sees a virtio device,
>> it might send virtio notifications to the QEMU in L2 for that device.
>> But since the QEMU in L2 defined this device as vfio-ccw, the function
>> handle_virtio_ccw_notify() cannot handle this and crashes: It calls
>> virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
>> but since "sch" belongs to a vfio-ccw device, that driver_data rather
>> points to a CcwDevice instead. So as soon as QEMU tries to use some
>> VirtioCcwDevice specific data from that device, we've lost.
>>
>> We must not take virtio notifications for such devices. Thus fix the
>> issue by adding a check to the handle_virtio_ccw_notify() handler to
>> refuse all devices that are not our own virtio devices.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   v2: Now with the required #include statement
>>
>>   hw/s390x/s390-hypercall.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/hw/s390x/s390-hypercall.c b/hw/s390x/s390-hypercall.c
>> index ac1b08b2cd5..38f1c6132e0 100644
>> --- a/hw/s390x/s390-hypercall.c
>> +++ b/hw/s390x/s390-hypercall.c
>> @@ -10,6 +10,7 @@
>>    */
>>   
>>   #include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>>   #include "cpu.h"
>>   #include "hw/s390x/s390-virtio-ccw.h"
>>   #include "hw/s390x/s390-hypercall.h"
>> @@ -42,6 +43,18 @@ static int handle_virtio_ccw_notify(uint64_t subch_id, uint64_t data)
>>       if (!sch || !css_subch_visible(sch)) {
>>           return -EINVAL;
>>       }
>> +    if (sch->id.cu_type != VIRTIO_CCW_CU_TYPE) {
>> +        /*
>> +         * This might happen in nested setups: If the L1 host defined the
>> +         * L2 guest with a virtio device (e.g. virtio-keyboard), and the
>> +         * L2 guest passes this device through to the L3 guest, the L3 guest
>> +         * might send virtio notifications to the QEMU in L2 for that device.
>> +         * But since the QEMU in L2 defined this device as vfio-ccw, it's not
>> +         * a VirtIODevice that we can handle here!
>> +         */
>> +        warn_report_once("Got virtio notification for unsupported device!");
> 
> Maybe also print which device ended up here?

You mean the values for cssid, ssid and schid ? Or which information did you 
have in mind?

  Thomas

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Cornelia Huck]

On Tue, Nov 18 2025, Thomas Huth <thuth@redhat.com> wrote:

> On 18/11/2025 12.52, Cornelia Huck wrote:
>> On Tue, Nov 18 2025, Thomas Huth <thuth@redhat.com> wrote:
>> 
>>> From: Thomas Huth <thuth@redhat.com>
>>>
>>> Consider the following nested setup: An L1 host uses some virtio device
>>> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
>>> device through to the L3 guest. Since the L3 guest sees a virtio device,
>>> it might send virtio notifications to the QEMU in L2 for that device.
>>> But since the QEMU in L2 defined this device as vfio-ccw, the function
>>> handle_virtio_ccw_notify() cannot handle this and crashes: It calls
>>> virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
>>> but since "sch" belongs to a vfio-ccw device, that driver_data rather
>>> points to a CcwDevice instead. So as soon as QEMU tries to use some
>>> VirtioCcwDevice specific data from that device, we've lost.
>>>
>>> We must not take virtio notifications for such devices. Thus fix the
>>> issue by adding a check to the handle_virtio_ccw_notify() handler to
>>> refuse all devices that are not our own virtio devices.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   v2: Now with the required #include statement
>>>
>>>   hw/s390x/s390-hypercall.c | 13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/hw/s390x/s390-hypercall.c b/hw/s390x/s390-hypercall.c
>>> index ac1b08b2cd5..38f1c6132e0 100644
>>> --- a/hw/s390x/s390-hypercall.c
>>> +++ b/hw/s390x/s390-hypercall.c
>>> @@ -10,6 +10,7 @@
>>>    */
>>>   
>>>   #include "qemu/osdep.h"
>>> +#include "qemu/error-report.h"
>>>   #include "cpu.h"
>>>   #include "hw/s390x/s390-virtio-ccw.h"
>>>   #include "hw/s390x/s390-hypercall.h"
>>> @@ -42,6 +43,18 @@ static int handle_virtio_ccw_notify(uint64_t subch_id, uint64_t data)
>>>       if (!sch || !css_subch_visible(sch)) {
>>>           return -EINVAL;
>>>       }
>>> +    if (sch->id.cu_type != VIRTIO_CCW_CU_TYPE) {
>>> +        /*
>>> +         * This might happen in nested setups: If the L1 host defined the
>>> +         * L2 guest with a virtio device (e.g. virtio-keyboard), and the
>>> +         * L2 guest passes this device through to the L3 guest, the L3 guest
>>> +         * might send virtio notifications to the QEMU in L2 for that device.
>>> +         * But since the QEMU in L2 defined this device as vfio-ccw, it's not
>>> +         * a VirtIODevice that we can handle here!
>>> +         */
>>> +        warn_report_once("Got virtio notification for unsupported device!");
>> 
>> Maybe also print which device ended up here?
>
> You mean the values for cssid, ssid and schid ? Or which information did you 
> have in mind?

Yes, so that you can correlate this message to the configuration.

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Thomas Huth]

On 18/11/2025 13.02, Halil Pasic wrote:
> On Tue, 18 Nov 2025 10:39:45 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> Consider the following nested setup: An L1 host uses some virtio device
>> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
>> device through to the L3 guest. Since the L3 guest sees a virtio device,
>> it might send virtio notifications to the QEMU in L2 for that device.
> 
> Hm, but conceptually the notification is sent to the virtio device,
> regardless of hypervisors, right? But because for virtio-ccw the
> notification is an DIAG 500, we have the usual cascade of intercept
> handling. And because we have never considered this scenario up till now
> the intercept handler in L2 QEMU gets called, because it is usually the
> responsibility of L2 QEMU to emulate instructions for an L3 guest.

Right.

> I think vfio-ccw pass through was supposed to be only about DASD.

Yes. And we only noticed this bug by accident - while trying to pass through 
a DASD device, the wrong device was used for VFIO and suddenly QEMU crashed.

>> But since the QEMU in L2 defined this device as vfio-ccw, the function
>> handle_virtio_ccw_notify() cannot handle this and crashes: It calls
>> virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
>> but since "sch" belongs to a vfio-ccw device, that driver_data rather
>> points to a CcwDevice instead. So as soon as QEMU tries to use some
>> VirtioCcwDevice specific data from that device, we've lost.
>>
>> We must not take virtio notifications for such devices. Thus fix the
>> issue by adding a check to the handle_virtio_ccw_notify() handler to
>> refuse all devices that are not our own virtio devices.
> 
> I'm on board with this patch! Virtio notifications are only supported
> for virtio devices and if a guest for what ever reason attempts
> to do a virtio notification on a non-virtio device, that should be
> handled accordingly. Which would be some sort of a program exception
> I guess. Maybe you could add what kind of exception do we end up
> with to the commit message. I would guess specification exception.
> 
> But I would argue that the L3 guest didn't do anything wrong.

That's the point - the L3 guest just sees a virtio device, so we should not 
punish it with program exceptions just because it tried to send a 
notification for the device.

> Pass-through of virtio-ccw devices is simply not implemented yet
> properly. And even  if we were to swallow that notification silently,
> it would be effectively loss of initiative I guess.

I think the current patch does the right thing: It returns an error value to 
the guest (just like we're doing it in other spots in this function 
already), so the guest sees that error value and then can finally give up on 
using the device.

> So I think it would really make sense to prevent passing through
> virtio-ccw devices with vfio-ccw.

That could be a nice addition on top (in another patch), but we have to fix 
handle_virtio_ccw_notify() anyway to avoid that the L3 guest can crash QEMU, 
so it's certainly not a replacement for this patch, I think.

  Thomas

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Halil Pasic]

On Tue, 18 Nov 2025 13:28:19 +0100
Thomas Huth <thuth@redhat.com> wrote:

> > But I would argue that the L3 guest didn't do anything wrong.  
> 
> That's the point - the L3 guest just sees a virtio device, so we should not 
> punish it with program exceptions just because it tried to send a 
> notification for the device.

I understand. But if from the L3 guests perspective it looks like the
notification happened just fine, it isn't exactly good either.

> 
> > Pass-through of virtio-ccw devices is simply not implemented yet
> > properly. And even  if we were to swallow that notification silently,
> > it would be effectively loss of initiative I guess.  
> 
> I think the current patch does the right thing: It returns an error value to 
> the guest (just like we're doing it in other spots in this function 
> already), so the guest sees that error value and then can finally give up on 
> using the device.

Hm, the -EINVAL is put into GPR2 which is 'Host Cookie' according to the
virtio specification:
https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html#x1-2260002

Unfortunately, I did not find any words in the spec according to which
GPR2 can be used to indicate errors. There does seem to be handling in
the linux driver for that. It basically says negative is bad, but I can't
see that in the spec. It just says "For each notification, the driver
SHOULD use GPR4 to pass the host cookie received in GPR2 from the previous
notification."

Maybe we want to update the spec to reflect what is in the filed.

But I agree it won't get any nicer than L3 guest giving up on the device
and resetting it. Which is an impact as well.

> 
> > So I think it would really make sense to prevent passing through
> > virtio-ccw devices with vfio-ccw.  
> 
> That could be a nice addition on top (in another patch), but we have to fix 
> handle_virtio_ccw_notify() anyway to avoid that the L3 guest can crash QEMU, 
> so it's certainly not a replacement for this patch, I think.

I agree, it should be a different patch.

I think adding some detail on the error handling via GPR2 to the
commit message could benefit the cause. But I don't insist. As I have
said I'm on board with the patch.

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

Regards,
Halil

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Cornelia Huck]

On Tue, Nov 18 2025, Thomas Huth <thuth@redhat.com> wrote:

> On 18/11/2025 13.02, Halil Pasic wrote:
>> On Tue, 18 Nov 2025 10:39:45 +0100
>> Thomas Huth <thuth@redhat.com> wrote:
>> 
>>> Consider the following nested setup: An L1 host uses some virtio device
>>> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
>>> device through to the L3 guest. Since the L3 guest sees a virtio device,
>>> it might send virtio notifications to the QEMU in L2 for that device.
>> 
>> Hm, but conceptually the notification is sent to the virtio device,
>> regardless of hypervisors, right? But because for virtio-ccw the
>> notification is an DIAG 500, we have the usual cascade of intercept
>> handling. And because we have never considered this scenario up till now
>> the intercept handler in L2 QEMU gets called, because it is usually the
>> responsibility of L2 QEMU to emulate instructions for an L3 guest.
>
> Right.
>
>> I think vfio-ccw pass through was supposed to be only about DASD.
>
> Yes. And we only noticed this bug by accident - while trying to pass through 
> a DASD device, the wrong device was used for VFIO and suddenly QEMU crashed.

That boils down to the fact that we don't know (at the kernel level) if
we're dealing with a dasd or something else, as we're operating on the
subchannel level. We certainly don't want to deal with devices whose
(Linux) drivers do funky things with ccws, or devices where we need to
support side-channel notifications, like virtio-ccw.

>
>>> But since the QEMU in L2 defined this device as vfio-ccw, the function
>>> handle_virtio_ccw_notify() cannot handle this and crashes: It calls
>>> virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
>>> but since "sch" belongs to a vfio-ccw device, that driver_data rather
>>> points to a CcwDevice instead. So as soon as QEMU tries to use some
>>> VirtioCcwDevice specific data from that device, we've lost.
>>>
>>> We must not take virtio notifications for such devices. Thus fix the
>>> issue by adding a check to the handle_virtio_ccw_notify() handler to
>>> refuse all devices that are not our own virtio devices.
>> 
>> I'm on board with this patch! Virtio notifications are only supported
>> for virtio devices and if a guest for what ever reason attempts
>> to do a virtio notification on a non-virtio device, that should be
>> handled accordingly. Which would be some sort of a program exception
>> I guess. Maybe you could add what kind of exception do we end up
>> with to the commit message. I would guess specification exception.
>> 
>> But I would argue that the L3 guest didn't do anything wrong.
>
> That's the point - the L3 guest just sees a virtio device, so we should not 
> punish it with program exceptions just because it tried to send a 
> notification for the device.
>
>> Pass-through of virtio-ccw devices is simply not implemented yet
>> properly. And even  if we were to swallow that notification silently,
>> it would be effectively loss of initiative I guess.
>
> I think the current patch does the right thing: It returns an error value to 
> the guest (just like we're doing it in other spots in this function 
> already), so the guest sees that error value and then can finally give up on 
> using the device.
>
>> So I think it would really make sense to prevent passing through
>> virtio-ccw devices with vfio-ccw.
>
> That could be a nice addition on top (in another patch), but we have to fix 
> handle_virtio_ccw_notify() anyway to avoid that the L3 guest can crash QEMU, 
> so it's certainly not a replacement for this patch, I think.

"Prevent crashing" is certainly the correct first step :)

I'm not sure where we would want prevent assignment of non-dasd
devices. The kernel can't (because we're dealing with a subchannel
driver.) I think maybe management software on top of QEMU?

The other direction would be supporting things like diag500, so we could
pass through virtio-ccw devices as well. But I'm not really seeing a
use case for it, or is there one?

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Thomas Huth]

On 18/11/2025 15.25, Cornelia Huck wrote:
> On Tue, Nov 18 2025, Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 18/11/2025 13.02, Halil Pasic wrote:
>>> On Tue, 18 Nov 2025 10:39:45 +0100
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> Consider the following nested setup: An L1 host uses some virtio device
>>>> (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
>>>> device through to the L3 guest. Since the L3 guest sees a virtio device,
>>>> it might send virtio notifications to the QEMU in L2 for that device.
...
>>> So I think it would really make sense to prevent passing through
>>> virtio-ccw devices with vfio-ccw.
>>
>> That could be a nice addition on top (in another patch), but we have to fix
>> handle_virtio_ccw_notify() anyway to avoid that the L3 guest can crash QEMU,
>> so it's certainly not a replacement for this patch, I think.
> 
> "Prevent crashing" is certainly the correct first step :)
> 
> I'm not sure where we would want prevent assignment of non-dasd
> devices. The kernel can't (because we're dealing with a subchannel
> driver.) I think maybe management software on top of QEMU?
> 
> The other direction would be supporting things like diag500, so we could
> pass through virtio-ccw devices as well. But I'm not really seeing a
> use case for it, or is there one?

It could be a very nice test setup to check vfio-ccw in CI pipelines (where 
you likely don't have access to a real DASD) ... but apart from that, I also 
don't see a real usecase for it.

  Thomas

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Cornelia Huck]

On Tue, Nov 18 2025, Halil Pasic <pasic@linux.ibm.com> wrote:

> Hm, the -EINVAL is put into GPR2 which is 'Host Cookie' according to the
> virtio specification:
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html#x1-2260002
>
> Unfortunately, I did not find any words in the spec according to which
> GPR2 can be used to indicate errors. There does seem to be handling in
> the linux driver for that. It basically says negative is bad, but I can't
> see that in the spec. It just says "For each notification, the driver
> SHOULD use GPR4 to pass the host cookie received in GPR2 from the previous
> notification."
>
> Maybe we want to update the spec to reflect what is in the filed.

Saying that the driver SHOULD check GPR2 for negative error values is
probably fine, since it matches what is already out
there. (Unfortunately, we can't mandate it without either a new feature
bit or a new revision, and that would be overkill IMHO.)

For the device, we say "The device MAY return a 64-bit host cookie in
GPR2 to speed up the notification execution." -- the spec should
probably also say that the device MAY return a negative error code.

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Eric Farman]

On Tue, 2025-11-18 at 13:02 +0100, Halil Pasic wrote:
> On Tue, 18 Nov 2025 10:39:45 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > Consider the following nested setup: An L1 host uses some virtio device
> > (e.g. virtio-keyboard) for the L2 guest, and this L2 guest passes this
> > device through to the L3 guest. Since the L3 guest sees a virtio device,
> > it might send virtio notifications to the QEMU in L2 for that device.
> 
> Hm, but conceptually the notification is sent to the virtio device,
> regardless of hypervisors, right? But because for virtio-ccw the
> notification is an DIAG 500, we have the usual cascade of intercept
> handling. And because we have never considered this scenario up till now
> the intercept handler in L2 QEMU gets called, because it is usually the
> responsibility of L2 QEMU to emulate instructions for an L3 guest.
> 
> I think vfio-ccw pass through was supposed to be only about DASD. 
> 
> > But since the QEMU in L2 defined this device as vfio-ccw, the function
> > handle_virtio_ccw_notify() cannot handle this and crashes: It calls
> > virtio_ccw_get_vdev() that casts sch->driver_data into a VirtioCcwDevice,
> > but since "sch" belongs to a vfio-ccw device, that driver_data rather
> > points to a CcwDevice instead. So as soon as QEMU tries to use some
> > VirtioCcwDevice specific data from that device, we've lost.
> > 
> > We must not take virtio notifications for such devices. Thus fix the
> > issue by adding a check to the handle_virtio_ccw_notify() handler to
> > refuse all devices that are not our own virtio devices.

I agree here. I like Cornelia's suggestion in the other thread of providing the subchannel
identifiers in the message so there's a hint of -where- the (presumed) typo came from.

> 
> I'm on board with this patch! Virtio notifications are only supported
> for virtio devices and if a guest for what ever reason attempts
> to do a virtio notification on a non-virtio device, that should be
> handled accordingly. Which would be some sort of a program exception
> I guess. Maybe you could add what kind of exception do we end up
> with to the commit message. I would guess specification exception.
> 
> But I would argue that the L3 guest didn't do anything wrong.
> Pass-through of virtio-ccw devices is simply not implemented yet
> properly. And even  if we were to swallow that notification silently,
> it would be effectively loss of initiative I guess.
> 
> So I think it would really make sense to prevent passing through
> virtio-ccw devices with vfio-ccw. Eric what is your take?

It's true that the only -supported- use case is passthrough DASD, and we allow other device types to
be passed through. The only ones we fence off are what we know aren't going to work, like FCPs. IIRC
we'd talked a few years ago about a plan for a more detailed allow-list of passthrough devices and
configurations, but that fell off the back of the todo list. Short of dusting all that off, I don't
know that blocking virtio-ccw from vfio is going to buy us anything that Thomas' patch doesn't
already provide.

Re: [PATCH v2] hw/s390x: Fix a possible crash with passed-through virtio devices

[Halil Pasic]

On Tue, 18 Nov 2025 10:19:56 -0500
Eric Farman <farman@linux.ibm.com> wrote:

> > So I think it would really make sense to prevent passing through
> > virtio-ccw devices with vfio-ccw. Eric what is your take?  
> 
> It's true that the only -supported- use case is passthrough DASD, and we allow other device types to
> be passed through. The only ones we fence off are what we know aren't going to work, like FCPs.

I think we can add virto-ccw to that known to not work list ;)

> IIRC
> we'd talked a few years ago about a plan for a more detailed allow-list of passthrough devices and
> configurations, but that fell off the back of the todo list. Short of dusting all that off, I don't
> know that blocking virtio-ccw from vfio is going to buy us anything that Thomas' patch doesn't
> already provide.

See your comment form later today :)

Would prevent ending up with a most likely dysfunctional device at best.

Regards,
Halil