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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkMEt5sapZjpyBSM5oX_=k1AcefEe5D4wtX=HqtHy4AD3j_g@mail.gmail.com>
Date:   Mon, 4 Dec 2023 16:39:00 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Kunkun Jiang <jiangkunkun@...wei.com>, dongli.zhang@...cle.com,
        cohuck@...hat.com, stefanha@...hat.com, mst@...hat.com,
        Oliver Upton <oliver.upton@...ux.dev>,
        James Morse <james.morse@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Zenghui Yu <yuzenghui@...wei.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Gavin Shan <gshan@...hat.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        "open list:IRQCHIP DRIVERS" <linux-kernel@...r.kernel.org>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev,
        wanghaibin.wang@...wei.com
Subject: Re: [RFC PATCH] KVM: arm/arm64: GICv4: Support shared VLPI

On Sat, Dec 2, 2023 at 8:20 PM Marc Zyngier <maz@...nel.org> wrote:
>
> Hi Kunkun,
>
> On Wed, 08 Nov 2023 09:45:51 +0000,
> Kunkun Jiang <jiangkunkun@...wei.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@...wei.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

It looks like a bug from the driver. It should only use vector 0 and 1
in this case.

Could you please check the queue_msix_vector for each queue in this case?

> > > 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.

Let's try to see the queue_msix_vector. A correct implemented device
should not try to use a vector more than 1.

>
> > > 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.

No matter how the driver is written, the host/KVM should be able to
survive from that.

Thanks

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