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

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

[David Woodhouse]

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

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?


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

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

[Marc Zyngier]

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.

	M.

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

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

[David Woodhouse]

[-- 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 --]