Hi Andre,
On Mon, 23 Aug 2021 17:48:33 +0100,
Andre Przywara <andre.przywara@xxxxxxx> wrote:
>
> On Sat, 21 Aug 2021 13:07:42 +0100
> Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> Hi Marc,
>
> > Since Linux commit 7d5ec3d36123 ("PCI/MSI: Mask all unused MSI-X
> > entries"), kvmtool segfaults when the guest boots and tries to
> > disable all the MSI-X entries of a virtio device while MSI-X itself
> > is disabled.
> >
> > What Linux does is seems perfectly correct. However, kvmtool uses
> > a different decoding depending on whether MSI-X is enabled for
> > this device or not. Which seems pretty wrong.
>
> While I really wish this would be wrong, I think this is
> indeed how this is supposed to work: The Virtio legacy spec makes the
> existence of those two virtio config fields dependent on the
> (dynamic!) enablement status of MSI-X. This is reflected in:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/virtio_pci.h#n72
> and explicitly mentioned as a footnote in the virtio 0.9.5 spec[1]:
> "3) ie. once you enable MSI-X on the device, the other fields move. If
> you turn it off again, they move back!"
Madness! What was Rusty on at the time? I really hope the bitcoin
thing is buying him better stuff...
> I agree that this looks like a bad idea, but I am afraid we are stuck
> with this. It looks like the Linux driver is at fault here, it should
> not issue the config access when MSIs are disabled. Something like this
> (untested):
>
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -103,6 +103,9 @@ static void vp_reset(struct virtio_device *vdev)
>
> static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> {
> + if (!vp_dev->msix_enabled)
> + return VIRTIO_MSI_NO_VECTOR;
> +
> /* Setup the vector used for configuration events */
> iowrite16(vector, vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
> /* Verify we had enough resources to assign the vector */
>
> This is just my first idea after looking at this, happy to stand
> corrected or hear about a better solution.
I don't think this works. It instead completely disables MSI-X, which
is a total bore. I think the only way to deal with it is to quirk it
to prevent the bulk masking to take effect before MSI-X is enabled.
Gawd, more crap. Just what we need...
M.
--
Without deviation from the norm, progress is not possible.
