Re: (subset) [PATCH 1/3] KVM: arm64: vgic: Fix IIDR revision field extracted from wrong value

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3091 bytes --]

On Mon, 2026-05-11 at 08:56 +0100, Marc Zyngier wrote:
> On Sun, 10 May 2026 22:28:50 +0100,
> David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > [1  <text/plain; UTF-8 (quoted-printable)>]
> > On Fri, 2026-04-24 at 13:24 +0100, David Woodhouse wrote:
> > > On Fri, 2026-04-24 at 12:07 +0100, Marc Zyngier wrote:
> > > > On Tue, 07 Apr 2026 21:27:02 +0100, David Woodhouse wrote:
> > > > > The uaccess write handlers for GICD_IIDR in both GICv2 and GICv3
> > > > > extract the revision field from 'reg' (the current IIDR value read back
> > > > > from the emulated distributor) instead of 'val' (the value userspace is
> > > > > trying to write). This means userspace can never actually change the
> > > > > implementation revision — the extracted value is always the current one.
> > > > > 
> > > > > Fix the FIELD_GET to use 'val' so that userspace can select a different
> > > > > revision for migration compatibility.
> > > > > 
> > > > > [...]
> > > > 
> > > > Applied to fixes, thanks!
> > > > 
> > > > [1/3] KVM: arm64: vgic: Fix IIDR revision field extracted from wrong value
> > > >       commit: a0e6ae45af17e8b27958830595799c702ffbab8d
> > > 
> > > There was a v2 of this series which also cleaned up the weird
> > > inconsistency of the IIDR value with the actual behaviour, and which
> > > fixed the fact that it's currently not possible to maintain guest
> > > compatibility when upgrading from a pre-d53c2c29ae0d kernel to a new
> > > one — despite the fact that that kind of compatibility is *precisely*
> > > what the revision field in the IIDR is designed for.
> > > 
> > > https://lore.kernel.org/all/20260408113256.2095505-1-dwmw2@infradead.org/
> > 
> > Is there a reason the rest of these fixes didn't make 7.1?
> 
> I already explained why these changes are neither necessary (a guest
> that used to run still runs) nor desirable (reintroducing bugs we have
> fixed is a bad idea).
> 
> I have therefore only taken the fix for the bug affecting userspace,
> as that was definitely something worth fixing.

You claimed that KVM/arm64 does not support migrating guests to older
(or even newer!!) kernels while maintaining compatibility.

That just *isn't* a cogent argument. I responded to it, in
https://lore.kernel.org/all/8cca18f332ea4bfb0c9f9d6775b2c1284816dd5f.camel@infradead.org/

KVM absolutely does need to support upgrading the kernel without
changing the environment that guests see. And sometimes it is sadly
also necessary to roll back an upgrade — which means that guests
launched on the new kernel should not see anything new until the fleet
is past the point where a rollback might happen.

Guest-visible changes need to be optional, in the case of the vGIC that
is exactly what the IIDR is *for*. There's not much point in my patch
that allows it to be *set*, if we don't allow it to be set *to* the
previous versions that are needed.

This isn't exactly rocket science, and I don't know why you claim not
to understand it.

What's going on, Marc? This behaviour isn't OK.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]