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

[James Clark <james.clark@linaro.org>]

On 27/11/2025 3:38 pm, Fuad Tabba wrote:
> On Thu, 27 Nov 2025 at 15:26, James Clark <james.clark@linaro.org> wrote:
>>
>>
>>
>> On 26/11/2025 11:48 am, Fuad Tabba wrote:
>>> On Wed, 26 Nov 2025 at 11:47, Marc Zyngier <maz@kernel.org> wrote:
>>>>
>>>> On Wed, 26 Nov 2025 10:37:57 +0000,
>>>> Fuad Tabba <tabba@google.com> wrote:
>>>>>
>>>>> Hi Marc,
>>>>>
>>>>> On Wed, 26 Nov 2025 at 10:23, Marc Zyngier <maz@kernel.org> wrote:
>>>>>>
>>>>>> On Tue, 18 Nov 2025 10:37:59 +0000,
>>>>>> Fuad Tabba <tabba@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@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.

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