Re: [PATCH v1 5/8] KVM: arm64: Fix Trace Buffer trap polarity for protected VMs

[Suzuki K Poulose <suzuki.poulose@arm.com>]

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))
>>
>>