* [PATCH v2 1/5] KVM: arm64: Fix Trace Buffer trapping for protected VMs
2025-11-06 14:44 [PATCH v2 0/5] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
@ 2025-11-06 14:44 ` Fuad Tabba
2025-11-06 16:28 ` Suzuki K Poulose
2025-11-06 14:44 ` [PATCH v2 2/5] KVM: arm64: Fix Trace Buffer trap polarity " Fuad Tabba
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2025-11-06 14:44 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 in pKVM, the hypervisor should trap accesses to trace
buffer system registers if Trace Buffer isn't supported by the VM.
However, the current code only traps if Trace Buffer External Mode isn't
supported.
Fix this by checking for FEAT_TRBE (Trace Buffer) rather than
FEAT_TRBE_EXT.
Fixes: 9d5261269098 ("KVM: arm64: Trap external trace for protected VMs")
Reported-by: Suzuki K Poulose <suzuki.poulose@arm.com>
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..8d06a246dfd1 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -117,7 +117,7 @@ static void pvm_init_traps_mdcr(struct kvm_vcpu *vcpu)
if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, TraceFilt, IMP))
val |= MDCR_EL2_TTRF;
- if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP))
+ if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, TraceBuffer, IMP))
val |= MDCR_EL2_E2TB_MASK;
/* Trap Debug Communications Channel registers */
--
2.51.2.1041.gc1ab5b90ca-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/5] KVM: arm64: Fix Trace Buffer trapping for protected VMs
2025-11-06 14:44 ` [PATCH v2 1/5] KVM: arm64: Fix Trace Buffer trapping for protected VMs Fuad Tabba
@ 2025-11-06 16:28 ` Suzuki K Poulose
0 siblings, 0 replies; 11+ messages in thread
From: Suzuki K Poulose @ 2025-11-06 16:28 UTC (permalink / raw)
To: Fuad Tabba, kvmarm, linux-arm-kernel
Cc: maz, oliver.upton, will, joey.gouly, yuzenghui, catalin.marinas,
vladimir.murzin
On 06/11/2025 14:44, Fuad Tabba wrote:
> For protected VMs in pKVM, the hypervisor should trap accesses to trace
> buffer system registers if Trace Buffer isn't supported by the VM.
> However, the current code only traps if Trace Buffer External Mode isn't
> supported.
>
> Fix this by checking for FEAT_TRBE (Trace Buffer) rather than
> FEAT_TRBE_EXT.
>
> Fixes: 9d5261269098 ("KVM: arm64: Trap external trace for protected VMs")
> Reported-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> 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..8d06a246dfd1 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -117,7 +117,7 @@ static void pvm_init_traps_mdcr(struct kvm_vcpu *vcpu)
> if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, TraceFilt, IMP))
> val |= MDCR_EL2_TTRF;
>
> - if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, ExtTrcBuff, IMP))
> + if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, TraceBuffer, IMP))
> val |= MDCR_EL2_E2TB_MASK;
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs
2025-11-06 14:44 [PATCH v2 0/5] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
2025-11-06 14:44 ` [PATCH v2 1/5] KVM: arm64: Fix Trace Buffer trapping for protected VMs Fuad Tabba
@ 2025-11-06 14:44 ` Fuad Tabba
2025-11-06 14:44 ` [PATCH v2 3/5] KVM: arm64: Fix MTE flag initialization " Fuad Tabba
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Fuad Tabba @ 2025-11-06 14:44 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 Trace Buffer
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 8d06a246dfd1..f6f8996c4f97 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, TraceBuffer, 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.1041.gc1ab5b90ca-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 3/5] KVM: arm64: Fix MTE flag initialization for protected VMs
2025-11-06 14:44 [PATCH v2 0/5] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
2025-11-06 14:44 ` [PATCH v2 1/5] KVM: arm64: Fix Trace Buffer trapping for protected VMs Fuad Tabba
2025-11-06 14:44 ` [PATCH v2 2/5] KVM: arm64: Fix Trace Buffer trap polarity " Fuad Tabba
@ 2025-11-06 14:44 ` Fuad Tabba
2025-11-06 14:44 ` [PATCH v2 4/5] KVM: arm64: Prevent host from managing timer offsets " Fuad Tabba
2025-11-06 14:44 ` [PATCH v2 5/5] KVM: arm64: Define FAR_MASK for fault IPA offset Fuad Tabba
4 siblings, 0 replies; 11+ messages in thread
From: Fuad Tabba @ 2025-11-06 14:44 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 f6f8996c4f97..7e370e31260d 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.1041.gc1ab5b90ca-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 4/5] KVM: arm64: Prevent host from managing timer offsets for protected VMs
2025-11-06 14:44 [PATCH v2 0/5] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
` (2 preceding siblings ...)
2025-11-06 14:44 ` [PATCH v2 3/5] KVM: arm64: Fix MTE flag initialization " Fuad Tabba
@ 2025-11-06 14:44 ` Fuad Tabba
2025-11-07 23:21 ` Oliver Upton
2025-11-06 14:44 ` [PATCH v2 5/5] KVM: arm64: Define FAR_MASK for fault IPA offset Fuad Tabba
4 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2025-11-06 14:44 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.1041.gc1ab5b90ca-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/5] KVM: arm64: Prevent host from managing timer offsets for protected VMs
2025-11-06 14:44 ` [PATCH v2 4/5] KVM: arm64: Prevent host from managing timer offsets " Fuad Tabba
@ 2025-11-07 23:21 ` Oliver Upton
2025-11-09 19:51 ` Fuad Tabba
0 siblings, 1 reply; 11+ messages in thread
From: Oliver Upton @ 2025-11-07 23:21 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Thu, Nov 06, 2025 at 02:44:16PM +0000, Fuad Tabba wrote:
> 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.
"This patch ..." is generally discouraged in changelogs, just state what
you're doing in an imperative tone.
> 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;
> +
This should be -EINVAL as pVMs do not even advertise the capability.
Since we already have a generic helper for filtering KVM_CAPs, I'd
prefer that we have a similar thing for enforcing ioctl limitations too.
For example, you could maintain the ioctl => KVM_CAP mapping in a table
and use kvm_pvm_ext_allowed() as the source of truth.
> 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);
Isn't there a general expectation that userspace not have access to the
vCPU state of a pVM? That should be the mechanism of enforcement instead
of special-casing these registers.
Thanks,
Oliver
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/5] KVM: arm64: Prevent host from managing timer offsets for protected VMs
2025-11-07 23:21 ` Oliver Upton
@ 2025-11-09 19:51 ` Fuad Tabba
0 siblings, 0 replies; 11+ messages in thread
From: Fuad Tabba @ 2025-11-09 19:51 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
Hi Oliver,
On Fri, 7 Nov 2025 at 23:21, Oliver Upton <oupton@kernel.org> wrote:
>
> On Thu, Nov 06, 2025 at 02:44:16PM +0000, Fuad Tabba wrote:
> > 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.
>
> "This patch ..." is generally discouraged in changelogs, just state what
> you're doing in an imperative tone.
Ack.
> > 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;
> > +
>
> This should be -EINVAL as pVMs do not even advertise the capability.
>
> Since we already have a generic helper for filtering KVM_CAPs, I'd
> prefer that we have a similar thing for enforcing ioctl limitations too.
>
> For example, you could maintain the ioctl => KVM_CAP mapping in a table
> and use kvm_pvm_ext_allowed() as the source of truth.
Yes, it makes more sense to consolidate these checks. Will do this
when I respin.
> > 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);
>
> Isn't there a general expectation that userspace not have access to the
> vCPU state of a pVM? That should be the mechanism of enforcement instead
> of special-casing these registers.
I thought I had a good reason for these checks, but now I cannot
remember what it was, nor can I see that there is a good reason, since
like you said, they are not accessible.
I'll remove them,
Cheers,
/fuad
> Thanks,
> Oliver
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] KVM: arm64: Define FAR_MASK for fault IPA offset
2025-11-06 14:44 [PATCH v2 0/5] KVM: arm64: Fixes for guest CPU feature trapping and enabling Fuad Tabba
` (3 preceding siblings ...)
2025-11-06 14:44 ` [PATCH v2 4/5] KVM: arm64: Prevent host from managing timer offsets " Fuad Tabba
@ 2025-11-06 14:44 ` Fuad Tabba
2025-11-07 23:12 ` Oliver Upton
4 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2025-11-06 14:44 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 fault 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.1041.gc1ab5b90ca-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 5/5] KVM: arm64: Define FAR_MASK for fault IPA offset
2025-11-06 14:44 ` [PATCH v2 5/5] KVM: arm64: Define FAR_MASK for fault IPA offset Fuad Tabba
@ 2025-11-07 23:12 ` Oliver Upton
2025-11-09 19:51 ` Fuad Tabba
0 siblings, 1 reply; 11+ messages in thread
From: Oliver Upton @ 2025-11-07 23:12 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Thu, Nov 06, 2025 at 02:44:17PM +0000, Fuad Tabba wrote:
> This 12-bit FAR mask is hard-coded as 'GENMASK(11, 0)' in several places
> to reconstruct the full fault 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)
> +
This seems really confusing to me, since FAR_MASK sounds like it
represents the meaningful bits of FAR_ELx. Perhaps something like the
following would better represent intent:
#define FAR_TO_FIPA_OFFSET(far) ((far) & GENMASK_ULL(11, 0))
Thanks,
Oliver
> #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.1041.gc1ab5b90ca-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 5/5] KVM: arm64: Define FAR_MASK for fault IPA offset
2025-11-07 23:12 ` Oliver Upton
@ 2025-11-09 19:51 ` Fuad Tabba
0 siblings, 0 replies; 11+ messages in thread
From: Fuad Tabba @ 2025-11-09 19:51 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, linux-arm-kernel, maz, oliver.upton, will, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, vladimir.murzin
On Fri, 7 Nov 2025 at 23:12, Oliver Upton <oupton@kernel.org> wrote:
>
> On Thu, Nov 06, 2025 at 02:44:17PM +0000, Fuad Tabba wrote:
> > This 12-bit FAR mask is hard-coded as 'GENMASK(11, 0)' in several places
> > to reconstruct the full fault 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)
> > +
>
> This seems really confusing to me, since FAR_MASK sounds like it
> represents the meaningful bits of FAR_ELx. Perhaps something like the
> following would better represent intent:
>
> #define FAR_TO_FIPA_OFFSET(far) ((far) & GENMASK_ULL(11, 0))
Sounds better. I'll change it.
Cheers,
/fuad
> Thanks,
> Oliver
>
> > #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.1041.gc1ab5b90ca-goog
> >
^ permalink raw reply [flat|nested] 11+ messages in thread