Re: [RFC PATCH] KVM: arm/arm64: GICv4: Support shared VLPI

[Marc Zyngier <maz@kernel.org>]

Hi Kunkun,

On Wed, 08 Nov 2023 09:45:51 +0000,
Kunkun Jiang <jiangkunkun@huawei.com> wrote:
> 
> Hi Marc,
> 
> On 2023/11/6 23:33, Marc Zyngier wrote:
> > On Mon, 06 Nov 2023 14:59:01 +0000,
> > Kunkun Jiang <jiangkunkun@huawei.com> wrote:
> >> The virtio-pci driver write entry1-6
> >> massage.data in the msix-table and trap to QEMU for processing. The
> >> massage.data is as follow:
> >>> entry-0 0
> >>> entry-1 1
> >>> entry-2 1
> >>> entry-3 1
> >>> entry-4 1
> >>> entry-5 1
> >>> entry-6 1
> > Urgh... is vp_modern_queue_vector() used in your configuration? This
> > is ... terrible.
> I encountered this problem using the 4.19 version kernel, but not the
> 5.10 version. This vp_modern_queue_vector() function does not exist
> in 4.19, but it uses 'vp_iowrite16(msix_vec, &cfg->queue_msix_vector)',
> the same as vp_modern_queue_vector().
> 
> In the past two days, I learned about the virtio driver and made some
> new discoveries. When 'num_queues' is greater than maxcpus, it will
> fall back into MSI-X with one shared for queues. The two patches[1],
> submitted by Dongli, limits the number of hw queues used by
> virtio-blk/virtio-scsi by 'nr_cpu_ids'. The two patches were merged
> in 5.1-rc2. And the patch related virtio-blk was merged into the 4.19
> stable branch.The patch related virtio-scsi was not merged.
> [1]
> https://lore.kernel.org/all/1553682995-5682-1-git-send-email-dongli.zhang@oracle.com/
> 
> This is the earliest discussion.
> https://lore.kernel.org/all/e4afe4c5-0262-4500-aeec-60f30734b4fc@default/
> 
> I don't know if there are other circumstances that would cause it to
> fall back into MSI-X with one shared for queues. At least the hack
> method is possible.
> > I wonder if PCIe actually allows this sort of thing.
> Do you think the virtio driver should be modified?

I think the virtio driver should stop messing with the MSI-X
configuration behind the kernel's back. For example, what happens if
the kernel needs to do a disable_irq() on the "shared" interrupt? It
will mask the interrupt in *one* of the vectors, and the interrupt
will still be screaming.

This is terribly broken, even on x86.

> > In any case, this sort of behaviour breaks so many thing in KVM's
> > implementation that I'd recommend you disable GICv4 until we have a
> > good solution for that.
> There seems to be no restriction in the GIC specification that multiple
> host irqs cannot be mapped to the same vlpi. Or maybe I didn't notice.
> Do you think there are any risks?

Please see 5.2.10 ("Restrictions for INTID mapping rules"), which
clearly forbids the case we have here: "Maps multiple EventID-DeviceID
combinations to the same virtual LPI INTID-vPEID.".

> GICv3 does not have this issue, but is this configuration legal?

With GICv3, the ITS doesn't see multiple mappings to the same
LPI. Each DeviceID/EventID pair has its own LPI, and KVM will just see
the injection callback from VFIO.

Honestly, the virtio driver is broken (irrespective of the
architecture), and incompatible with the GIC architecture.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel