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

[Eric Farman <farman@linux.ibm.com>]

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.