[PATCH v5 2/9] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs

[Fuad Tabba]

On Thu, 27 Nov 2025 at 16:06, James Clark <james.clark at linaro.org> wrote:
>
>
>
> On 27/11/2025 3:38 pm, Fuad Tabba wrote:
> > On Thu, 27 Nov 2025 at 15:26, James Clark <james.clark at linaro.org> wrote:
> >>
> >>
> >>
> >> On 26/11/2025 11:48 am, Fuad Tabba wrote:
> >>> On Wed, 26 Nov 2025 at 11:47, Marc Zyngier <maz at kernel.org> wrote:
> >>>>
> >>>> On Wed, 26 Nov 2025 10:37:57 +0000,
> >>>> Fuad Tabba <tabba at google.com> wrote:
> >>>>>
> >>>>> Hi Marc,
> >>>>>
> >>>>> On Wed, 26 Nov 2025 at 10:23, Marc Zyngier <maz at kernel.org> wrote:
> >>>>>>
> >>>>>> On Tue, 18 Nov 2025 10:37:59 +0000,
> >>>>>> Fuad Tabba <tabba at google.com> 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 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 at 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;
> >>>>>>
> >>>>>> This does not only change the trapping logic (bit 24). It also change
> >>>>>> the ownership of the buffer (bit 25). I wonder whether you should do
> >>>>>> something for that, maybe by clearing TRBLIMITR_EL1.E, because
> >>>>>> otherwise, you keep tracing, but using an EL2 VA. What could possibly
> >>>>>> go wrong?
> >>>>>>
> >>>>>> Overall, I'm very uneasy about TRBE in the context of pKVM.
> >>>>>
> >>>>> So should we clear/restore TRBLIMITR_EL1.E on guest entry/exit in
> >>>>> protected mode?
> >>>>
> >>>> I think you need something of the sort, yes. Overall, SPE and TRBE
> >>>> should be aligned on what they are allowed to do, as they are two
> >>>> sides of the same ugly coin.
> >>>
> >>> I'll fix this (or add a patch to do this, depending on how it looks)
> >>> when I respin.
> >>>
> >>> Cheers,
> >>> /fuad
> >>>
> >>
> >> To understand this, is it just belt-and-braces because you don't trust
> >> the current filter clearing in TRFCR_EL1? And to reduce the potential
> >> for mistakes leading to trace leakages? I suppose even the hardware
> >> itself could have some errata where it tries to access the buffer, even
> >> if we've completely filtered out trace and flushed before going into the
> >> guest.
> >
> > Yes, we don't trust the host. The pKVM threat model is to protect
> > against a malicious host.
> >
> > Getting into hardware errata... that's a can of worms. I haven't
> > considered that.
> >
> > Cheers,
> > /fuad
> >
> >
>
> But the hypervisor already clears the filters, equivalent to clearing
> TRBLIMITR_EL1.E. Which is why I was wondering if there was something
> extra on top of the model where the host isn't trusted. I would assume
> that without anything more specific that just clearing the filters is
> enough. If we trust the hardware then it is anyway.

We trust the hardware, yes. Marc, what do you think?

/fuad

> >> Thanks
> >> James
> >>
> >>>>           M.
> >>>>
> >>>> --
> >>>> Without deviation from the norm, progress is not possible.
> >>>
> >>
>