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

[James Clark]

On 26/11/2025 10:36 am, Fuad Tabba wrote:
> Hi James,
> 
> On Wed, 26 Nov 2025 at 10:29, James Clark <james.clark at linaro.org> wrote:
>>
>>
>>
>> On 26/11/2025 10:23 am, Marc Zyngier 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.
>>>
>>> Thanks,
>>>
>>>        M.
>>>
>>
>> Don't we already clear TRBLIMITR_EL1.E in __trace_needs_drain() for pKVM
>> when switching to the guest?
> 
> I don't think so:
> https://elixir.bootlin.com/linux/v6.18-rc7/source/arch/arm64/kvm/hyp/nvhe/debug-sr.c#L60
> 
> Cheers,
> /fuad
>> James
>>

Oh yeah I glanced over the TRBLIMITR_EL1_E and thought that was clearing 
it, but it's just checking if it's enabled or not.

We actually disable trace by setting the filters to 0 in 
__trace_do_switch() and leave TRBLIMITR_EL1.E enabled.