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

[Halil Pasic <pasic@linux.ibm.com>]

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