* Re: (subset) [PATCH 1/3] KVM: arm64: vgic: Fix IIDR revision field extracted from wrong value [not found] ` <106addd3f78918a9a584c43c181a9609aef1ceca.camel@infradead.org> @ 2026-05-10 21:28 ` David Woodhouse 2026-05-11 7:56 ` Marc Zyngier 0 siblings, 1 reply; 3+ messages in thread From: David Woodhouse @ 2026-05-10 21:28 UTC (permalink / raw) To: Marc Zyngier, Oliver Upton, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Paolo Bonzini, Shuah Khan, Raghavendra Rao Ananta, Eric Auger, Kees Cook, Arnd Bergmann, Nathan Chancellor, linux-arm-kernel, kvmarm, linux-kernel, kvm, linux-kselftest [-- 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 --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: (subset) [PATCH 1/3] KVM: arm64: vgic: Fix IIDR revision field extracted from wrong value 2026-05-10 21:28 ` (subset) [PATCH 1/3] KVM: arm64: vgic: Fix IIDR revision field extracted from wrong value David Woodhouse @ 2026-05-11 7:56 ` Marc Zyngier 2026-05-11 8:13 ` David Woodhouse 0 siblings, 1 reply; 3+ messages in thread From: Marc Zyngier @ 2026-05-11 7:56 UTC (permalink / raw) To: David Woodhouse Cc: Oliver Upton, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Paolo Bonzini, Shuah Khan, Raghavendra Rao Ananta, Eric Auger, Kees Cook, Arnd Bergmann, Nathan Chancellor, linux-arm-kernel, kvmarm, linux-kernel, kvm, linux-kselftest 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. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: (subset) [PATCH 1/3] KVM: arm64: vgic: Fix IIDR revision field extracted from wrong value 2026-05-11 7:56 ` Marc Zyngier @ 2026-05-11 8:13 ` David Woodhouse 0 siblings, 0 replies; 3+ messages in thread From: David Woodhouse @ 2026-05-11 8:13 UTC (permalink / raw) To: Marc Zyngier Cc: Oliver Upton, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Paolo Bonzini, Shuah Khan, Raghavendra Rao Ananta, Eric Auger, Kees Cook, Arnd Bergmann, Nathan Chancellor, linux-arm-kernel, kvmarm, linux-kernel, kvm, linux-kselftest [-- 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 --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-11 8:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260407210949.2076251-1-dwmw2@infradead.org>
[not found] ` <20260407210949.2076251-2-dwmw2@infradead.org>
[not found] ` <177702878141.537738.13460155220731277452.b4-ty@kernel.org>
[not found] ` <106addd3f78918a9a584c43c181a9609aef1ceca.camel@infradead.org>
2026-05-10 21:28 ` (subset) [PATCH 1/3] KVM: arm64: vgic: Fix IIDR revision field extracted from wrong value David Woodhouse
2026-05-11 7:56 ` Marc Zyngier
2026-05-11 8:13 ` David Woodhouse
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox