Re: [PATCH] KVM: arm64: vgic-v3: Don't load pending state when enabling LPIs on RD

[Zenghui Yu <yuzenghui@huawei.com>]

Hi Oliver,

On 2024/2/28 8:01, Oliver Upton wrote:
> Reality check: KVM's GIC/ITS emulation does not handle pending LPI state
> to the letter of the architecture!
> 
> Although the GIC spec defines LPI pending state on a per-redistributor
> basis, KVM's view of the LPIs in a VM are global by design. This is
> intentional, as it is a massive simplification to the way KVM organizes
> interrupts and deals with the interactions between the vCPUs and ITSes.
> 
> So, with that in mind, the KVM emulation of PENDBASE is completely wrong,
> as unmapped INTIDs (i.e. not mapped in any ITS) are silently ignored,
> even though the expecatation is that IRQs would be generated in real
> hardware. Even more hilarious things can happen for the LPIs that are
> actually mapped, as an update to global LPI state can pend an interrupt
> on a different vCPU.

I don't fully understand this "hilarious" part. Could you please
elaborate a bit more on it?

> vgic_add_lpi() fetches pending state when creating the global
> representation of an LPI. Both VM save/restore and guest interactions
> with the ITS find their way here, so the pending state will get loaded
> into KVM one way or another. That flow is slightly more correct, as it
> consults the pending table of the targeted redistributor.

This looks reasonable. I remember I had looked several times at this
code path (vgic_enable_lpis()/its_sync_lpi_pending_table()) but hadn't
made any improvements or progress.

E.g., it purely does *nothing* on the guest restore path, as
redistributors are restored before ITS, so there is no LPI mapping at
all when we're at its_sync_lpi_pending_table().

It's just been a long time. I'll take another look and think about it.

> Anyway, it is clear that this code can never be correct and doesn't
> serve a purpose. If bisection led you here you've probably got a *very*
> funky guest, and should bring it up on the list...

So I've tested it with kvm-unit-tests and got failure with the
its-pending-migration case:

INFO: gicv3: its-pending-migration: Migration complete
INFO: gicv3: its-pending-migration: expected 128 LPIs on PE #30, 0 observed
FAIL: gicv3: its-pending-migration: 128 LPIs on both PE0 and PE1 after 
migration
SUMMARY: 1 tests, 1 unexpected failures

where the guest SW directly writes to the pending table when
GICR_CTLR.EnableLPIs == 0. I seriously doubt there is any use case like
that in real world. But not sure whether it is a funky behaviour from
the architectural perspective.

Thanks,
Zenghui