Re: [PATCH][kvmtool] virtio/pci: Correctly handle MSI-X masking while MSI-X is disabled

From: Marc Zyngier <maz@kernel.org>
To: Andre Przywara <andre.przywara@arm.com>
Cc: kernel-team@android.com, kvm@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Will Deacon <will@kernel.org>,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH][kvmtool] virtio/pci: Correctly handle MSI-X masking while MSI-X is disabled
Date: Thu, 26 Aug 2021 18:50:22 +0100	[thread overview]
Message-ID: <87zgt4nli9.wl-maz@kernel.org> (raw)
In-Reply-To: <7c2abc6a-164a-b07e-a39e-756b83a98f0c@arm.com>

On Thu, 26 Aug 2021 18:07:55 +0100,
Andre Przywara <andre.przywara@arm.com> wrote:
> 
> On 8/25/21 6:44 PM, Marc Zyngier wrote:
> 
> >> Actually, let me correct myself. I tested the wrong configuration (why
> >> isn't --force-pci the bloody default in kvmtool?).
> 
> I guess because there is no --force-mmio, and PCI on ARM was kind of
> daunting back then ;-)

I remember that. But maybe we should change that default...

> >> Having dug further, it isn't the config space that causes problems,
> >> but the programming of the MSI-X vectors. I'm starting to suspect the
> >> layout of the MSI-X bar in kvmtool.
> > 
> > OK, this is hilarious. Sort of. The MSI-X bar sizing is bonkers: you
> > can't fit 33 MSIs there (33 being the number of MSI-X that kvmtool
> > advertises), and you will have notionally overwritten the PBA as
> > well. Amusingly, the last write ends up being misdecoded as a config
> > space access...
> 
> Ah, very good find indeed, many thanks for the debugging!
> 
> I am only halfway through the code by now, but wouldn't you need to
> adjust the PBA offset in the MSIX capability as well? This is still
> stuck at that (misnamed) PCI_IO_SIZE, in
> virtio/pci.c:virtio_pci__init(): vpci->pci_hdr.msix.pba_offset =
> And IIUC this has to match the decoding in virtio_pci__msix_mmio_callback().

Indeed. I had this:

	Region 2: Memory at 50000800 (32-bit, non-prefetchable) [size=1K]
	Capabilities: [40] MSI-X: Enable+ Count=33 Masked-
		Vector table: BAR=2 offset=00000000
		PBA: BAR=2 offset=00000100

and with the PBA fix, I get:

	Region 2: Memory at 50000800 (32-bit, non-prefetchable) [size=1K]
	Capabilities: [40] MSI-X: Enable+ Count=33 Masked-
		Vector table: BAR=2 offset=00000000
		PBA: BAR=2 offset=00000210

which looks more sane. But man, this code is crufty as hell. I'll post
an updated patch later, once I've cleaned up a couple more things.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm