Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

["Arnd Bergmann" <arnd@arndb.de>]

On Sat, Oct 24, 2026, at 09:14, Fuad Tabba wrote:
> On Fri, 23 Oct 2026 15:10:23 +0100, "Arnd Bergmann" <arnd@arndb.de> wrote:
>> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> [...]
>> @@ -254,6 +269,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
>>          if (prev != next)
>>                  __switch_mm(next);
>>
>> +        switch_iommu_mm(next, tsk); /* FIXME: is there a better place to do this? */
>
> switch_mm() isn't enough when the same thread also runs a vCPU. In v2,
> which I'll post soon, KVM will save and restore ACCDATA_EL1 only for
> the guest, since nothing in the host uses it today. So after KVM_RUN
> returns from a guest that has LS64_ACCDATA, the VMM thread's next
> ST64BV0 would use the guest's value, unless KVM also saves and
> restores the host's.

Right, makes sense.

I was a little unsure at first how ACCDATA_EL1 gets handled on PCIe
TLPs when there are both host and guest users that set it. As I
understand it now, this only works if the PCIe requesterID is assigned
to the guest VMID for stage2 translation, so a guest st64bv0
uses the ACCDATA_EL1 as PASID in combination with that VMID.
The host would just use ACCDATA_EL1/PASID for its own stage1
translation without stage2.

This means that with your v2 patch and the switch_mm() code I
have, this should be safe. There is a possible optimization to
skip the host ACCDATA_EL1 save/restore if current->thread.sctlr_user
does not enable SCTLR_EL1_EnAS0 (i.e. the same process that
runs the KVM guest has mapped the MMIO area of an SVA device).
This would make a difference if ACCDATA_EL1 access traps to
EL3, but otherwise likely does not much much of a difference
(I'd have to do some measurements on hardware).

>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> [...]
>> +static bool try_enable_st64bv0(struct pt_regs *regs, unsigned long esr)
> [...]
>> +        isar1 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR1_EL1);
>> +        if (!(isar1 & ID_AA64ISAR1_EL1_LS64_LS64_ACCDATA))
>> +                return false;
>
> v2 will add an ARM64_HAS_LS64_ACCDATA cpucap, so this could test that
> instead. As written it tests bits [1:0] of the register, not the LS64
> field, since ID_AA64ISAR1_EL1_LS64_LS64_ACCDATA is the unshifted field
> value.

Thanks for pointing it out. I have rebased on top of your patches now,
so this line is gone.

>> +
>> +        sysreg_clear_set(accdata_el1, ACCDATA_EL1_ACCDATA, pasid);
>> +        sctlr = current->thread.sctlr_user;
>> +        sctlr |= SCTLR_EL1_EnAS0;
>> +        current->thread.sctlr_user = sctlr;
>> +        update_sctlr_el1(sctlr);
>
> On a host that runs at EL1 (nVHE or hVHE), this also requires
> __init_el2_fgt to set nACCDATA_EL1 in HFGRTR_EL2 and HFGWTR_EL2, and
> __init_el2_hcrx to set HCRX_EL2.EnAS0. Without them, the ACCDATA_EL1
> access traps to EL2 and the hypervisor panics, and so does ST64BV0
> once SCTLR_EL1.EnAS0 is set. Happy to write those and the KVM change
> once you're ready to post this.

Thanks for the offer, that part is definitely not my area.

     Arnd