* [PATCH v1 1/8] KVM: arm64: Route MOPS exceptions to EL2 when guest lacks support
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
@ 2025-11-04 12:58 ` Fuad Tabba
2025-11-04 13:44 ` Joey Gouly
2025-11-04 12:59 ` [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest Fuad Tabba
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:58 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
MOPS exceptions must be routed to the hypervisor (EL2) when the guest
does not support the feature. If the guest does support MOPS, exceptions
should be handled by the guest at EL1.
The existing logic was inverted: exceptions were trapped to EL2 (by
setting HCRX_EL2_MCE2) when the guest supported MOPS, and left to be
handled at EL1 when it did not.
Fix this by moving the setting of HCRX_EL2_MCE2 from the feature-check
block to the 'else' block, ensuring exceptions are only trapped to EL2
when the guest cannot handle them.
Fixes: 84de212d739e ("KVM: arm64: Make FEAT_MOPS UNDEF if not advertised to the guest")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_emulate.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index c9eab316398e..0f8311263edf 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -684,7 +684,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
vcpu->arch.hcrx_el2 = HCRX_EL2_SMPME;
if (kvm_has_feat(kvm, ID_AA64ISAR2_EL1, MOPS, IMP))
- vcpu->arch.hcrx_el2 |= (HCRX_EL2_MSCEn | HCRX_EL2_MCE2);
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_MSCEn;
+ else
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
if (kvm_has_tcr2(kvm))
vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 1/8] KVM: arm64: Route MOPS exceptions to EL2 when guest lacks support
2025-11-04 12:58 ` [PATCH v1 1/8] KVM: arm64: Route MOPS exceptions to EL2 when guest lacks support Fuad Tabba
@ 2025-11-04 13:44 ` Joey Gouly
2025-11-04 13:49 ` Fuad Tabba
0 siblings, 1 reply; 21+ messages in thread
From: Joey Gouly @ 2025-11-04 13:44 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, suzuki.poulose,
yuzenghui, catalin.marinas, vladimir.murzin
Hi Fuad,
On Tue, Nov 04, 2025 at 12:58:59PM +0000, Fuad Tabba wrote:
> MOPS exceptions must be routed to the hypervisor (EL2) when the guest
> does not support the feature. If the guest does support MOPS, exceptions
> should be handled by the guest at EL1.
>
> The existing logic was inverted: exceptions were trapped to EL2 (by
> setting HCRX_EL2_MCE2) when the guest supported MOPS, and left to be
> handled at EL1 when it did not.
>
This doesn't seem quite right? Check the commit message of 2de451a329:
A KVM guest may use the instructions at EL1 at times when the guest is
not able to handle the exception, expecting that the instructions will
only run on one CPU (e.g. when running UEFI boot services in the guest).
As KVM may reschedule the guest between different types of CPUs at any
time (on an asymmetric system), it needs to also handle the resulting
exception itself in case the guest is not able to. A similar situation
will also occur in the future when live migrating a guest from one type
of CPU to another.
So it's intentionally trapping MOPS to EL2.
Also with HCRX_EL2.MSCEn=0, execution of the MOPS instructions are UNDEFINED at
EL0/EL1, so you wouldn't take any MOPS exceptions to EL2 anyway.
Thanks,
Joey
> Fix this by moving the setting of HCRX_EL2_MCE2 from the feature-check
> block to the 'else' block, ensuring exceptions are only trapped to EL2
> when the guest cannot handle them.
>
> Fixes: 84de212d739e ("KVM: arm64: Make FEAT_MOPS UNDEF if not advertised to the guest")
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
> arch/arm64/include/asm/kvm_emulate.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index c9eab316398e..0f8311263edf 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -684,7 +684,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> vcpu->arch.hcrx_el2 = HCRX_EL2_SMPME;
>
> if (kvm_has_feat(kvm, ID_AA64ISAR2_EL1, MOPS, IMP))
> - vcpu->arch.hcrx_el2 |= (HCRX_EL2_MSCEn | HCRX_EL2_MCE2);
> + vcpu->arch.hcrx_el2 |= HCRX_EL2_MSCEn;
> + else
> + vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
>
> if (kvm_has_tcr2(kvm))
> vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
> --
> 2.51.2.997.g839fc31de9-goog
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 1/8] KVM: arm64: Route MOPS exceptions to EL2 when guest lacks support
2025-11-04 13:44 ` Joey Gouly
@ 2025-11-04 13:49 ` Fuad Tabba
0 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 13:49 UTC (permalink / raw)
To: Joey Gouly
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, suzuki.poulose,
yuzenghui, catalin.marinas, vladimir.murzin
Hi Joey,
On Tue, 4 Nov 2025 at 13:44, Joey Gouly <joey.gouly@arm.com> wrote:
>
> Hi Fuad,
>
> On Tue, Nov 04, 2025 at 12:58:59PM +0000, Fuad Tabba wrote:
> > MOPS exceptions must be routed to the hypervisor (EL2) when the guest
> > does not support the feature. If the guest does support MOPS, exceptions
> > should be handled by the guest at EL1.
> >
> > The existing logic was inverted: exceptions were trapped to EL2 (by
> > setting HCRX_EL2_MCE2) when the guest supported MOPS, and left to be
> > handled at EL1 when it did not.
> >
>
> This doesn't seem quite right? Check the commit message of 2de451a329:
>
> A KVM guest may use the instructions at EL1 at times when the guest is
> not able to handle the exception, expecting that the instructions will
> only run on one CPU (e.g. when running UEFI boot services in the guest).
> As KVM may reschedule the guest between different types of CPUs at any
> time (on an asymmetric system), it needs to also handle the resulting
> exception itself in case the guest is not able to. A similar situation
> will also occur in the future when live migrating a guest from one type
> of CPU to another.
>
> So it's intentionally trapping MOPS to EL2.
>
> Also with HCRX_EL2.MSCEn=0, execution of the MOPS instructions are UNDEFINED at
> EL0/EL1, so you wouldn't take any MOPS exceptions to EL2 anyway.
Thanks for the clarification, you're right. I'll drop this patch.
Cheers,
/fuad
> Thanks,
> Joey
>
> > Fix this by moving the setting of HCRX_EL2_MCE2 from the feature-check
> > block to the 'else' block, ensuring exceptions are only trapped to EL2
> > when the guest cannot handle them.
> >
> > Fixes: 84de212d739e ("KVM: arm64: Make FEAT_MOPS UNDEF if not advertised to the guest")
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> > arch/arm64/include/asm/kvm_emulate.h | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> > index c9eab316398e..0f8311263edf 100644
> > --- a/arch/arm64/include/asm/kvm_emulate.h
> > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > @@ -684,7 +684,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> > vcpu->arch.hcrx_el2 = HCRX_EL2_SMPME;
> >
> > if (kvm_has_feat(kvm, ID_AA64ISAR2_EL1, MOPS, IMP))
> > - vcpu->arch.hcrx_el2 |= (HCRX_EL2_MSCEn | HCRX_EL2_MCE2);
> > + vcpu->arch.hcrx_el2 |= HCRX_EL2_MSCEn;
> > + else
> > + vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
> >
> > if (kvm_has_tcr2(kvm))
> > vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
> > --
> > 2.51.2.997.g839fc31de9-goog
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
2025-11-04 12:58 ` [PATCH v1 1/8] KVM: arm64: Route MOPS exceptions to EL2 when guest lacks support Fuad Tabba
@ 2025-11-04 12:59 ` Fuad Tabba
2025-11-04 15:15 ` Marc Zyngier
2025-11-04 12:59 ` [PATCH v1 3/8] KVM: arm64: Enable LS64 instructions when supported by guest Fuad Tabba
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:59 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
Access to ALLINT is part of FEAT_NMI. If a guest does not support this
feature, any access to this register must be trapped to the hypervisor
(EL2).
KVM didn't configure this trap, potentially allowing a guest to toggle
all interrupt mask when it doesn't support FEAT_NMI. Fix this by
checking if the guest has FEAT_NMI support.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_emulate.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 0f8311263edf..3fc62808c548 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -688,6 +688,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
else
vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
+ if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
+
if (kvm_has_tcr2(kvm))
vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest
2025-11-04 12:59 ` [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest Fuad Tabba
@ 2025-11-04 15:15 ` Marc Zyngier
2025-11-04 15:30 ` Fuad Tabba
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2025-11-04 15:15 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, linux-arm-kernel, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Tue, 04 Nov 2025 12:59:00 +0000,
Fuad Tabba <tabba@google.com> wrote:
>
> Access to ALLINT is part of FEAT_NMI. If a guest does not support this
> feature, any access to this register must be trapped to the hypervisor
> (EL2).
>
> KVM didn't configure this trap, potentially allowing a guest to toggle
> all interrupt mask when it doesn't support FEAT_NMI. Fix this by
> checking if the guest has FEAT_NMI support.
>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
> arch/arm64/include/asm/kvm_emulate.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 0f8311263edf..3fc62808c548 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -688,6 +688,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> else
> vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
>
> + if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
> + vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
> +
> if (kvm_has_tcr2(kvm))
> vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
>
I think this is moving in the wrong direction. We have for quite some
time now tried to automatically derive these behaviours from the guest
config, as we do for FGUs.
I would like to see a similar behaviour being introduced for non-FGT
bits so that we don't have to worry about these things anymore.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest
2025-11-04 15:15 ` Marc Zyngier
@ 2025-11-04 15:30 ` Fuad Tabba
2025-11-04 17:22 ` Marc Zyngier
0 siblings, 1 reply; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 15:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, linux-arm-kernel, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
Hi Marc,
On Tue, 4 Nov 2025 at 15:15, Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 04 Nov 2025 12:59:00 +0000,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > Access to ALLINT is part of FEAT_NMI. If a guest does not support this
> > feature, any access to this register must be trapped to the hypervisor
> > (EL2).
> >
> > KVM didn't configure this trap, potentially allowing a guest to toggle
> > all interrupt mask when it doesn't support FEAT_NMI. Fix this by
> > checking if the guest has FEAT_NMI support.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> > arch/arm64/include/asm/kvm_emulate.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> > index 0f8311263edf..3fc62808c548 100644
> > --- a/arch/arm64/include/asm/kvm_emulate.h
> > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > @@ -688,6 +688,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> > else
> > vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
> >
> > + if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
> > + vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
> > +
> > if (kvm_has_tcr2(kvm))
> > vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
> >
>
> I think this is moving in the wrong direction. We have for quite some
> time now tried to automatically derive these behaviours from the guest
> config, as we do for FGUs.
>
> I would like to see a similar behaviour being introduced for non-FGT
> bits so that we don't have to worry about these things anymore.
I agree that we should make this more like FGT, but since that will
likely require a bit more time (and probably a couple of respins),
should we fix the HCRX traps as they are now?
Thanks,
/fuad
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest
2025-11-04 15:30 ` Fuad Tabba
@ 2025-11-04 17:22 ` Marc Zyngier
2025-11-04 17:30 ` Fuad Tabba
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2025-11-04 17:22 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, linux-arm-kernel, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Tue, 04 Nov 2025 15:30:34 +0000,
Fuad Tabba <tabba@google.com> wrote:
>
> Hi Marc,
>
> On Tue, 4 Nov 2025 at 15:15, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Tue, 04 Nov 2025 12:59:00 +0000,
> > Fuad Tabba <tabba@google.com> wrote:
> > >
> > > Access to ALLINT is part of FEAT_NMI. If a guest does not support this
> > > feature, any access to this register must be trapped to the hypervisor
> > > (EL2).
> > >
> > > KVM didn't configure this trap, potentially allowing a guest to toggle
> > > all interrupt mask when it doesn't support FEAT_NMI. Fix this by
> > > checking if the guest has FEAT_NMI support.
> > >
> > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > ---
> > > arch/arm64/include/asm/kvm_emulate.h | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> > > index 0f8311263edf..3fc62808c548 100644
> > > --- a/arch/arm64/include/asm/kvm_emulate.h
> > > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > > @@ -688,6 +688,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> > > else
> > > vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
> > >
> > > + if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
> > > + vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
> > > +
> > > if (kvm_has_tcr2(kvm))
> > > vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
> > >
> >
> > I think this is moving in the wrong direction. We have for quite some
> > time now tried to automatically derive these behaviours from the guest
> > config, as we do for FGUs.
> >
> > I would like to see a similar behaviour being introduced for non-FGT
> > bits so that we don't have to worry about these things anymore.
>
> I agree that we should make this more like FGT, but since that will
> likely require a bit more time (and probably a couple of respins),
> should we fix the HCRX traps as they are now?
I don't think this is fixing anything. Where does the trap go? There
is no triaging, no handling. At the very least, this needs to be
defined.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest
2025-11-04 17:22 ` Marc Zyngier
@ 2025-11-04 17:30 ` Fuad Tabba
0 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 17:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, linux-arm-kernel, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Tue, 4 Nov 2025 at 17:22, Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 04 Nov 2025 15:30:34 +0000,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > Hi Marc,
> >
> > On Tue, 4 Nov 2025 at 15:15, Marc Zyngier <maz@kernel.org> wrote:
> > >
> > > On Tue, 04 Nov 2025 12:59:00 +0000,
> > > Fuad Tabba <tabba@google.com> wrote:
> > > >
> > > > Access to ALLINT is part of FEAT_NMI. If a guest does not support this
> > > > feature, any access to this register must be trapped to the hypervisor
> > > > (EL2).
> > > >
> > > > KVM didn't configure this trap, potentially allowing a guest to toggle
> > > > all interrupt mask when it doesn't support FEAT_NMI. Fix this by
> > > > checking if the guest has FEAT_NMI support.
> > > >
> > > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > > ---
> > > > arch/arm64/include/asm/kvm_emulate.h | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> > > > index 0f8311263edf..3fc62808c548 100644
> > > > --- a/arch/arm64/include/asm/kvm_emulate.h
> > > > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > > > @@ -688,6 +688,9 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> > > > else
> > > > vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
> > > >
> > > > + if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
> > > > + vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
> > > > +
> > > > if (kvm_has_tcr2(kvm))
> > > > vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
> > > >
> > >
> > > I think this is moving in the wrong direction. We have for quite some
> > > time now tried to automatically derive these behaviours from the guest
> > > config, as we do for FGUs.
> > >
> > > I would like to see a similar behaviour being introduced for non-FGT
> > > bits so that we don't have to worry about these things anymore.
> >
> > I agree that we should make this more like FGT, but since that will
> > likely require a bit more time (and probably a couple of respins),
> > should we fix the HCRX traps as they are now?
>
> I don't think this is fixing anything. Where does the trap go? There
> is no triaging, no handling. At the very least, this needs to be
> defined.
Right... I was thinking of protected VMs, but much of that code isn't
yet upstream. I'll start working on handling these traps similar to
the FGU handling, so it's ready when we start upstreaming the rest of
pKVM (soon I hope).
Cheers,
/fuad
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 3/8] KVM: arm64: Enable LS64 instructions when supported by guest
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
2025-11-04 12:58 ` [PATCH v1 1/8] KVM: arm64: Route MOPS exceptions to EL2 when guest lacks support Fuad Tabba
2025-11-04 12:59 ` [PATCH v1 2/8] KVM: arm64: Trap access to ALLINT if FEAT_NMI not supported by the guest Fuad Tabba
@ 2025-11-04 12:59 ` Fuad Tabba
2025-11-04 15:26 ` Marc Zyngier
2025-11-04 12:59 ` [PATCH v1 4/8] KVM: arm64: Refactor vcpu_set_hcrx() to reduce indentation Fuad Tabba
` (4 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:59 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
The FEAT_LS64* family (FEAT_LS64, FEAT_LS64_V, FEAT_LS64_ACCDATA)
enables support for LD64B, ST64B, and their variants. If a guest is
advertised these features, KVM should not trap accesses to these
instructions to EL2.
This is controlled by the HCRX_EL2_EnASR, HCRX_EL2_EnALS, and
HCRX_EL2_EnAS0 bits. When clear, these bits trap the corresponding
instructions. KVM did not set them, which would cause guest-supported
instructions to trap.
This also created a state mismatch for nested virtualization, which
validates its own HCRX_EL2 value against the features advertised in the
guest's ID_AA64ISAR1_EL1 (in handle_other()).
Fix this by checking for each FEAT_LS64* variant in
vcpu_set_hcrx() and setting the corresponding HCRX_EL2 enable
bit if the guest supports the feature.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_emulate.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 3fc62808c548..7880e8290a20 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -691,6 +691,15 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
+ if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_V))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_EnASR;
+
+ if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_EnALS;
+
+ if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_ACCDATA))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_EnAS0;
+
if (kvm_has_tcr2(kvm))
vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 3/8] KVM: arm64: Enable LS64 instructions when supported by guest
2025-11-04 12:59 ` [PATCH v1 3/8] KVM: arm64: Enable LS64 instructions when supported by guest Fuad Tabba
@ 2025-11-04 15:26 ` Marc Zyngier
2025-11-04 15:31 ` Fuad Tabba
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2025-11-04 15:26 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, linux-arm-kernel, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Tue, 04 Nov 2025 12:59:01 +0000,
Fuad Tabba <tabba@google.com> wrote:
>
> The FEAT_LS64* family (FEAT_LS64, FEAT_LS64_V, FEAT_LS64_ACCDATA)
> enables support for LD64B, ST64B, and their variants. If a guest is
> advertised these features, KVM should not trap accesses to these
> instructions to EL2.
>
> This is controlled by the HCRX_EL2_EnASR, HCRX_EL2_EnALS, and
> HCRX_EL2_EnAS0 bits. When clear, these bits trap the corresponding
> instructions. KVM did not set them, which would cause guest-supported
> instructions to trap.
>
> This also created a state mismatch for nested virtualization, which
> validates its own HCRX_EL2 value against the features advertised in the
> guest's ID_AA64ISAR1_EL1 (in handle_other()).
>
> Fix this by checking for each FEAT_LS64* variant in
> vcpu_set_hcrx() and setting the corresponding HCRX_EL2 enable
> bit if the guest supports the feature.
>
> Signed-off-by: Fuad Tabba <tabba@google.com>
Please see 20240815125959.2097734-1-maz@kernel.org, which has a large
portion of what is required for this to work. Just flipping the bits
isn't quite enough.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 3/8] KVM: arm64: Enable LS64 instructions when supported by guest
2025-11-04 15:26 ` Marc Zyngier
@ 2025-11-04 15:31 ` Fuad Tabba
0 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 15:31 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, linux-arm-kernel, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Tue, 4 Nov 2025 at 15:26, Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 04 Nov 2025 12:59:01 +0000,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > The FEAT_LS64* family (FEAT_LS64, FEAT_LS64_V, FEAT_LS64_ACCDATA)
> > enables support for LD64B, ST64B, and their variants. If a guest is
> > advertised these features, KVM should not trap accesses to these
> > instructions to EL2.
> >
> > This is controlled by the HCRX_EL2_EnASR, HCRX_EL2_EnALS, and
> > HCRX_EL2_EnAS0 bits. When clear, these bits trap the corresponding
> > instructions. KVM did not set them, which would cause guest-supported
> > instructions to trap.
> >
> > This also created a state mismatch for nested virtualization, which
> > validates its own HCRX_EL2 value against the features advertised in the
> > guest's ID_AA64ISAR1_EL1 (in handle_other()).
> >
> > Fix this by checking for each FEAT_LS64* variant in
> > vcpu_set_hcrx() and setting the corresponding HCRX_EL2 enable
> > bit if the guest supports the feature.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
>
> Please see 20240815125959.2097734-1-maz@kernel.org, which has a large
> portion of what is required for this to work. Just flipping the bits
> isn't quite enough.
I see that now. Thanks.
/fuad
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 4/8] KVM: arm64: Refactor vcpu_set_hcrx() to reduce indentation
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
` (2 preceding siblings ...)
2025-11-04 12:59 ` [PATCH v1 3/8] KVM: arm64: Enable LS64 instructions when supported by guest Fuad Tabba
@ 2025-11-04 12:59 ` Fuad Tabba
2025-11-04 12:59 ` [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs Fuad Tabba
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:59 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
Invert the main conditional check in vcpu_set_hcrx() to return
immediately if the CPU does not support FEAT_HCX
(ARM64_HAS_HCX).
This refactoring pattern avoids wrapping the entire function body in
an 'if' block, reducing indentation and improving readability as the
function continues to grow.
No functional change intended.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_emulate.h | 54 ++++++++++++++--------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 7880e8290a20..034e1b39de6c 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -674,40 +674,40 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
{
struct kvm *kvm = vcpu->kvm;
- if (cpus_have_final_cap(ARM64_HAS_HCX)) {
- /*
- * In general, all HCRX_EL2 bits are gated by a feature.
- * The only reason we can set SMPME without checking any
- * feature is that its effects are not directly observable
- * from the guest.
- */
- vcpu->arch.hcrx_el2 = HCRX_EL2_SMPME;
+ if (!cpus_have_final_cap(ARM64_HAS_HCX))
+ return;
- if (kvm_has_feat(kvm, ID_AA64ISAR2_EL1, MOPS, IMP))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_MSCEn;
- else
- vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
+ /*
+ * In general, all HCRX_EL2 bits are gated by a feature.
+ * The only reason we can set SMPME without checking any feature is that
+ * its effects are not directly observable from the guest.
+ */
+ vcpu->arch.hcrx_el2 = HCRX_EL2_SMPME;
- if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
+ if (kvm_has_feat(kvm, ID_AA64ISAR2_EL1, MOPS, IMP))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_MSCEn;
+ else
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
- if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_V))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_EnASR;
+ if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, NMI, IMP))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_TALLINT;
- if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_EnALS;
+ if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_V))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_EnASR;
- if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_ACCDATA))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_EnAS0;
+ if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_EnALS;
- if (kvm_has_tcr2(kvm))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
+ if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_ACCDATA))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_EnAS0;
- if (kvm_has_fpmr(kvm))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_EnFPM;
+ if (kvm_has_tcr2(kvm))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
- if (kvm_has_sctlr2(kvm))
- vcpu->arch.hcrx_el2 |= HCRX_EL2_SCTLR2En;
- }
+ if (kvm_has_fpmr(kvm))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_EnFPM;
+
+ if (kvm_has_sctlr2(kvm))
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_SCTLR2En;
}
#endif /* __ARM64_KVM_EMULATE_H__ */
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
` (3 preceding siblings ...)
2025-11-04 12:59 ` [PATCH v1 4/8] KVM: arm64: Refactor vcpu_set_hcrx() to reduce indentation Fuad Tabba
@ 2025-11-04 12:59 ` Fuad Tabba
2025-11-04 17:50 ` Suzuki K Poulose
2025-11-04 12:59 ` [PATCH v1 6/8] KVM: arm64: Fix MTE flag initialization " Fuad Tabba
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:59 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
The E2TB bits in MDCR_EL2 control trapping of Trace Buffer system
register accesses. These accesses are trapped to EL2 when the bits are
clear.
The trap initialization logic for protected VMs in pvm_init_traps_mdcr()
had the polarity inverted. When a guest did not support the ExtTrcBuff
feature, the code was setting E2TB. This incorrectly disabled the trap,
potentially allowing a protected guest to access registers for a feature
it was not given.
Fix this by inverting the operation.
Fixes: f50758260bff ("KVM: arm64: Group setting traps for protected VMs by control register")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 43bde061b65d..df0c59a29b35 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -118,7 +118,7 @@ static void pvm_init_traps_mdcr(struct kvm_vcpu *vcpu)
val |= MDCR_EL2_TTRF;
if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP))
- val |= MDCR_EL2_E2TB_MASK;
+ val &= ~MDCR_EL2_E2TB_MASK;
/* Trap Debug Communications Channel registers */
if (!kvm_has_feat(kvm, ID_AA64MMFR0_EL1, FGT, IMP))
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs
2025-11-04 12:59 ` [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs Fuad Tabba
@ 2025-11-04 17:50 ` Suzuki K Poulose
2025-11-04 17:56 ` Fuad Tabba
0 siblings, 1 reply; 21+ messages in thread
From: Suzuki K Poulose @ 2025-11-04 17:50 UTC (permalink / raw)
To: Fuad Tabba, kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, yuzenghui, catalin.marinas,
vladimir.murzin
On 04/11/2025 12:59, Fuad Tabba wrote:
> The E2TB bits in MDCR_EL2 control trapping of Trace Buffer system
> register accesses. These accesses are trapped to EL2 when the bits are
> clear.
>
> The trap initialization logic for protected VMs in pvm_init_traps_mdcr()
> had the polarity inverted. When a guest did not support the ExtTrcBuff
> feature, the code was setting E2TB. This incorrectly disabled the trap,
> potentially allowing a protected guest to access registers for a feature
> it was not given.
>
> Fix this by inverting the operation.
>
> Fixes: f50758260bff ("KVM: arm64: Group setting traps for protected VMs by control register")
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
> arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 43bde061b65d..df0c59a29b35 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -118,7 +118,7 @@ static void pvm_init_traps_mdcr(struct kvm_vcpu *vcpu)
> val |= MDCR_EL2_TTRF;
>
> if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP))
Why are we checkin ExtTrcBuff ? Shouldn't that be TraceBuffer ?
> - val |= MDCR_EL2_E2TB_MASK;
> + val &= ~MDCR_EL2_E2TB_MASK;
This looks correct to me.
Suzuki
>
> /* Trap Debug Communications Channel registers */
> if (!kvm_has_feat(kvm, ID_AA64MMFR0_EL1, FGT, IMP))
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs
2025-11-04 17:50 ` Suzuki K Poulose
@ 2025-11-04 17:56 ` Fuad Tabba
2025-11-04 18:03 ` Suzuki K Poulose
0 siblings, 1 reply; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 17:56 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, joey.gouly,
yuzenghui, catalin.marinas, vladimir.murzin
Hi Suzuki,
On Tue, 4 Nov 2025 at 17:50, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 04/11/2025 12:59, Fuad Tabba wrote:
> > The E2TB bits in MDCR_EL2 control trapping of Trace Buffer system
> > register accesses. These accesses are trapped to EL2 when the bits are
> > clear.
> >
> > The trap initialization logic for protected VMs in pvm_init_traps_mdcr()
> > had the polarity inverted. When a guest did not support the ExtTrcBuff
> > feature, the code was setting E2TB. This incorrectly disabled the trap,
> > potentially allowing a protected guest to access registers for a feature
> > it was not given.
> >
> > Fix this by inverting the operation.
> >
> > Fixes: f50758260bff ("KVM: arm64: Group setting traps for protected VMs by control register")
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> > arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > index 43bde061b65d..df0c59a29b35 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > @@ -118,7 +118,7 @@ static void pvm_init_traps_mdcr(struct kvm_vcpu *vcpu)
> > val |= MDCR_EL2_TTRF;
> >
> > if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP))
>
> Why are we checkin ExtTrcBuff ? Shouldn't that be TraceBuffer ?
This is an existing bug that needs to be fixed. Thank for spotting it.
> > - val |= MDCR_EL2_E2TB_MASK;
> > + val &= ~MDCR_EL2_E2TB_MASK;
> This looks correct to me.
I don't thin it's correct. E2TB traps when clear, not set:
https://developer.arm.com/documentation/ddi0601/2025-09/AArch64-Registers/MDCR-EL2--Monitor-Debug-Configuration-Register--EL2-
> Suzuki
>
> >
> > /* Trap Debug Communications Channel registers */
> > if (!kvm_has_feat(kvm, ID_AA64MMFR0_EL1, FGT, IMP))
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs
2025-11-04 17:56 ` Fuad Tabba
@ 2025-11-04 18:03 ` Suzuki K Poulose
2025-11-04 19:04 ` Fuad Tabba
0 siblings, 1 reply; 21+ messages in thread
From: Suzuki K Poulose @ 2025-11-04 18:03 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, joey.gouly,
yuzenghui, catalin.marinas, vladimir.murzin
On 04/11/2025 17:56, Fuad Tabba wrote:
> Hi Suzuki,
>
> On Tue, 4 Nov 2025 at 17:50, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>> On 04/11/2025 12:59, Fuad Tabba wrote:
>>> The E2TB bits in MDCR_EL2 control trapping of Trace Buffer system
>>> register accesses. These accesses are trapped to EL2 when the bits are
>>> clear.
>>>
>>> The trap initialization logic for protected VMs in pvm_init_traps_mdcr()
>>> had the polarity inverted. When a guest did not support the ExtTrcBuff
>>> feature, the code was setting E2TB. This incorrectly disabled the trap,
>>> potentially allowing a protected guest to access registers for a feature
>>> it was not given.
>>>
>>> Fix this by inverting the operation.
>>>
>>> Fixes: f50758260bff ("KVM: arm64: Group setting traps for protected VMs by control register")
>>> Signed-off-by: Fuad Tabba <tabba@google.com>
>>> ---
>>> arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
>>> index 43bde061b65d..df0c59a29b35 100644
>>> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
>>> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
>>> @@ -118,7 +118,7 @@ static void pvm_init_traps_mdcr(struct kvm_vcpu *vcpu)
>>> val |= MDCR_EL2_TTRF;
>>>
>>> if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP))
>>
>> Why are we checkin ExtTrcBuff ? Shouldn't that be TraceBuffer ?
>
> This is an existing bug that needs to be fixed. Thank for spotting it.
>
>>> - val |= MDCR_EL2_E2TB_MASK;
>>> + val &= ~MDCR_EL2_E2TB_MASK;
>> This looks correct to me.
>
> I don't thin it's correct. E2TB traps when clear, not set:
> https://developer.arm.com/documentation/ddi0601/2025-09/AArch64-Registers/MDCR-EL2--Monitor-Debug-Configuration-Register--EL2-
I meant your change to val looks correct.
Suzuki
>
>> Suzuki
>>
>>>
>>> /* Trap Debug Communications Channel registers */
>>> if (!kvm_has_feat(kvm, ID_AA64MMFR0_EL1, FGT, IMP))
>>
>>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs
2025-11-04 18:03 ` Suzuki K Poulose
@ 2025-11-04 19:04 ` Fuad Tabba
0 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 19:04 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, joey.gouly,
yuzenghui, catalin.marinas, vladimir.murzin
On Tue, 4 Nov 2025 at 18:03, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 04/11/2025 17:56, Fuad Tabba wrote:
> > Hi Suzuki,
> >
> > On Tue, 4 Nov 2025 at 17:50, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> >>
> >> On 04/11/2025 12:59, Fuad Tabba wrote:
> >>> The E2TB bits in MDCR_EL2 control trapping of Trace Buffer system
> >>> register accesses. These accesses are trapped to EL2 when the bits are
> >>> clear.
> >>>
> >>> The trap initialization logic for protected VMs in pvm_init_traps_mdcr()
> >>> had the polarity inverted. When a guest did not support the ExtTrcBuff
> >>> feature, the code was setting E2TB. This incorrectly disabled the trap,
> >>> potentially allowing a protected guest to access registers for a feature
> >>> it was not given.
> >>>
> >>> Fix this by inverting the operation.
> >>>
> >>> Fixes: f50758260bff ("KVM: arm64: Group setting traps for protected VMs by control register")
> >>> Signed-off-by: Fuad Tabba <tabba@google.com>
> >>> ---
> >>> arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> >>> index 43bde061b65d..df0c59a29b35 100644
> >>> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> >>> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> >>> @@ -118,7 +118,7 @@ static void pvm_init_traps_mdcr(struct kvm_vcpu *vcpu)
> >>> val |= MDCR_EL2_TTRF;
> >>>
> >>> if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP))
> >>
> >> Why are we checkin ExtTrcBuff ? Shouldn't that be TraceBuffer ?
> >
> > This is an existing bug that needs to be fixed. Thank for spotting it.
> >
> >>> - val |= MDCR_EL2_E2TB_MASK;
> >>> + val &= ~MDCR_EL2_E2TB_MASK;
> >> This looks correct to me.
> >
> > I don't thin it's correct. E2TB traps when clear, not set:
> > https://developer.arm.com/documentation/ddi0601/2025-09/AArch64-Registers/MDCR-EL2--Monitor-Debug-Configuration-Register--EL2-
>
> I meant your change to val looks correct.
I see, thanks :) I'll fix the existing bug when I respin this.
Cheers,
/fuad
> Suzuki
>
> >
> >> Suzuki
> >>
> >>>
> >>> /* Trap Debug Communications Channel registers */
> >>> if (!kvm_has_feat(kvm, ID_AA64MMFR0_EL1, FGT, IMP))
> >>
> >>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 6/8] KVM: arm64: Fix MTE flag initialization for protected VMs
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
` (4 preceding siblings ...)
2025-11-04 12:59 ` [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs Fuad Tabba
@ 2025-11-04 12:59 ` Fuad Tabba
2025-11-04 12:59 ` [PATCH v1 7/8] KVM: arm64: Prevent host from managing timer offsets " Fuad Tabba
2025-11-04 12:59 ` [PATCH v1 8/8] KVM: arm64: Define FAR_MASK for faulting IPA offset Fuad Tabba
7 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:59 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
The function pkvm_init_features_from_host() initializes guest
features, propagating them from the host. The logic to propagate
KVM_ARCH_FLAG_MTE_ENABLED (Memory Tagging Extension)
has a couple of issues.
First, the check was in the common path, before the divergence for
protected and non-protected VMs. For non-protected VMs, this was
unnecessary, as 'kvm->arch.flags' is completely overwritten by
host_arch_flags immediately after, which already contains the MTE flag.
For protected VMs, this was setting the flag even if the feature is not
allowed.
Second, the check was reading 'host_kvm->arch.flags' instead of using
the local 'host_arch_flags', which is read once from the host flags.
This patch fixes these issues by moving the MTE flag check to be inside
the protected-VM-only path, checking if the feature is allowed, and
changing it to use the correct host_arch_flags local variable. This
ensures non-protected VMs get the flag via the bulk copy, and protected
VMs get it via an explicit check.
Fixes: b7f345fbc32a ("KVM: arm64: Fix FEAT_MTE in pKVM")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/kvm/hyp/nvhe/pkvm.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index df0c59a29b35..b64f278ac8cc 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -337,9 +337,6 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
/* CTR_EL0 is always under host control, even for protected VMs. */
hyp_vm->kvm.arch.ctr_el0 = host_kvm->arch.ctr_el0;
- if (test_bit(KVM_ARCH_FLAG_MTE_ENABLED, &host_kvm->arch.flags))
- set_bit(KVM_ARCH_FLAG_MTE_ENABLED, &kvm->arch.flags);
-
/* No restrictions for non-protected VMs. */
if (!kvm_vm_is_protected(kvm)) {
hyp_vm->kvm.arch.flags = host_arch_flags;
@@ -372,6 +369,11 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
kvm->arch.flags |= host_arch_flags & BIT(KVM_ARCH_FLAG_GUEST_HAS_SVE);
}
+ if (kvm_pvm_ext_allowed(KVM_CAP_ARM_MTE)) {
+ set_bit(KVM_CAP_ARM_MTE, allowed_features);
+ kvm->arch.flags |= host_arch_flags & BIT(KVM_ARCH_FLAG_MTE_ENABLED);
+ }
+
bitmap_and(kvm->arch.vcpu_features, host_kvm->arch.vcpu_features,
allowed_features, KVM_VCPU_MAX_FEATURES);
}
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 7/8] KVM: arm64: Prevent host from managing timer offsets for protected VMs
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
` (5 preceding siblings ...)
2025-11-04 12:59 ` [PATCH v1 6/8] KVM: arm64: Fix MTE flag initialization " Fuad Tabba
@ 2025-11-04 12:59 ` Fuad Tabba
2025-11-04 12:59 ` [PATCH v1 8/8] KVM: arm64: Define FAR_MASK for faulting IPA offset Fuad Tabba
7 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:59 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
For protected VMs, the guest's timer offset state is private and must
not be controlled by the host. Protected VMs must always run with a
virtual counter offset of 0.
The existing timer logic allowed the host to set and manage the timer
counter offsets (voffset and poffset) for protected VMs.
This patch disables all host-side management of timer offsets for
protected VMs by adding checks in the relevant code paths.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/kvm/arch_timer.c | 18 +++++++++++++-----
arch/arm64/kvm/sys_regs.c | 6 ++++--
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 3f675875abea..69f5631ebf84 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -1056,10 +1056,14 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)
ctxt->timer_id = timerid;
- if (timerid == TIMER_VTIMER)
- ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset;
- else
- ctxt->offset.vm_offset = &kvm->arch.timer_data.poffset;
+ if (!kvm_vm_is_protected(vcpu->kvm)) {
+ if (timerid == TIMER_VTIMER)
+ ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset;
+ else
+ ctxt->offset.vm_offset = &kvm->arch.timer_data.poffset;
+ } else {
+ ctxt->offset.vm_offset = NULL;
+ }
hrtimer_setup(&ctxt->hrtimer, kvm_hrtimer_expire, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
@@ -1083,7 +1087,8 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
timer_context_init(vcpu, i);
/* Synchronize offsets across timers of a VM if not already provided */
- if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags)) {
+ if (!vcpu_is_protected(vcpu) &&
+ !test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags)) {
timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read());
timer_set_offset(vcpu_ptimer(vcpu), 0);
}
@@ -1687,6 +1692,9 @@ int kvm_vm_ioctl_set_counter_offset(struct kvm *kvm,
if (offset->reserved)
return -EINVAL;
+ if (kvm_vm_is_protected(kvm))
+ return -EBUSY;
+
mutex_lock(&kvm->lock);
if (!kvm_trylock_all_vcpus(kvm)) {
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index e67eb39ddc11..3329a8f03436 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1606,11 +1606,13 @@ static int arch_timer_set_user(struct kvm_vcpu *vcpu,
val &= ~ARCH_TIMER_CTRL_IT_STAT;
break;
case SYS_CNTVCT_EL0:
- if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
+ if (!vcpu_is_protected(vcpu) &&
+ !test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read() - val);
return 0;
case SYS_CNTPCT_EL0:
- if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
+ if (!vcpu_is_protected(vcpu) &&
+ !test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
timer_set_offset(vcpu_ptimer(vcpu), kvm_phys_timer_read() - val);
return 0;
}
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 8/8] KVM: arm64: Define FAR_MASK for faulting IPA offset
2025-11-04 12:58 [PATCH v1 0/8] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
` (6 preceding siblings ...)
2025-11-04 12:59 ` [PATCH v1 7/8] KVM: arm64: Prevent host from managing timer offsets " Fuad Tabba
@ 2025-11-04 12:59 ` Fuad Tabba
7 siblings, 0 replies; 21+ messages in thread
From: Fuad Tabba @ 2025-11-04 12:59 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, vladimir.murzin, tabba
This 12-bit FAR mask is hard-coded as 'GENMASK(11, 0)' in several places
to reconstruct the full faulting IPA.
Define FAR_MASK for this value in the shared header and replace all
open-coded instances to improve readability.
No functional change intended.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_arm.h | 2 ++
arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c | 2 +-
arch/arm64/kvm/inject_fault.c | 2 +-
arch/arm64/kvm/mmu.c | 4 ++--
4 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 1da290aeedce..e995e9b3a0c4 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -343,6 +343,8 @@
#define PAR_TO_HPFAR(par) \
(((par) & GENMASK_ULL(52 - 1, 12)) >> 8)
+#define FAR_MASK GENMASK_ULL(11, 0)
+
#define ECN(x) { ESR_ELx_EC_##x, #x }
#define kvm_arm_exception_class \
diff --git a/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c b/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c
index 78579b31a420..22898ed4dd6f 100644
--- a/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c
+++ b/arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c
@@ -44,7 +44,7 @@ int __vgic_v2_perform_cpuif_access(struct kvm_vcpu *vcpu)
/* Build the full address */
fault_ipa = kvm_vcpu_get_fault_ipa(vcpu);
- fault_ipa |= kvm_vcpu_get_hfar(vcpu) & GENMASK(11, 0);
+ fault_ipa |= kvm_vcpu_get_hfar(vcpu) & FAR_MASK;
/* If not for GICV, move on */
if (fault_ipa < vgic->vgic_cpu_base ||
diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index dfcd66c65517..8ef3453696a2 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -258,7 +258,7 @@ void kvm_inject_size_fault(struct kvm_vcpu *vcpu)
unsigned long addr, esr;
addr = kvm_vcpu_get_fault_ipa(vcpu);
- addr |= kvm_vcpu_get_hfar(vcpu) & GENMASK(11, 0);
+ addr |= kvm_vcpu_get_hfar(vcpu) & FAR_MASK;
__kvm_inject_sea(vcpu, kvm_vcpu_trap_is_iabt(vcpu), addr);
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 7cc964af8d30..21a9442f9b1e 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1959,7 +1959,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
/* Falls between the IPA range and the PARange? */
if (fault_ipa >= BIT_ULL(VTCR_EL2_IPA(vcpu->arch.hw_mmu->vtcr))) {
- fault_ipa |= kvm_vcpu_get_hfar(vcpu) & GENMASK(11, 0);
+ fault_ipa |= kvm_vcpu_get_hfar(vcpu) & FAR_MASK;
return kvm_inject_sea(vcpu, is_iabt, fault_ipa);
}
@@ -2059,7 +2059,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
* faulting VA. This is always 12 bits, irrespective
* of the page size.
*/
- ipa |= kvm_vcpu_get_hfar(vcpu) & GENMASK(11, 0);
+ ipa |= kvm_vcpu_get_hfar(vcpu) & FAR_MASK;
ret = io_mem_abort(vcpu, ipa);
goto out_unlock;
}
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread