KVM: arm64: Ensure I-cache isolation between vcpus of a same VM

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

Quoted 4 lines · Marc Zyngier

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

Quoted 3 lines

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

Digging through the ARM ARM is hard. Do we have this behaviour with FWB as well?

Quoted 10 lines · Catalin Marinas

Quoted 7 lines · Marc Zyngier

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

Quoted 2 lines

Digging through the ARM ARM is hard. Do we have this behaviour with FWB as well?

The ARM ARM doesn't seem to mention FWB at all when it comes to instruction fetch, which is sort of expected as it only covers the D-side. I *think* we could sidestep this when CTR_EL0.DIC is set though, as the I-side would then snoop the D-side.

Quoted 12 lines · Marc Zyngier

Quoted 10 lines · Catalin Marinas

Quoted 7 lines · Marc Zyngier

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

Digging through the ARM ARM is hard. Do we have this behaviour with FWB as well?

Quoted 4 lines

The ARM ARM doesn't seem to mention FWB at all when it comes to instruction fetch, which is sort of expected as it only covers the D-side. I *think* we could sidestep this when CTR_EL0.DIC is set though, as the I-side would then snoop the D-side.

Not sure this helps. CTR_EL0.DIC refers to the need for maintenance to PoU while the SCTLR_EL1.M == 0 causes the I-cache to fetch from PoC. I don't think I-cache snooping the D-cache would happen to the PoU when the S1 MMU is off.

My reading of D4.4.4 is that when SCTLR_EL1.M == 0 both I and D accesses are Normal Non-cacheable with a note in D4.4.6 that Non-cacheable accesses may be held in the I-cache.

The FWB rules on combining S1 and S2 says that Normal Non-cacheable at S1 is "upgraded" to cacheable. This should happen irrespective of whether the S1 MMU is on or off and should apply to both I and D accesses (since it does not explicitly says). So I think we could skip this IC IALLU when FWB is present.

The same logic should apply when the VMM copies the VM text. With FWB, we probably only need D-cache maintenance to PoU and only if CTR_EL0.IDC==0. I haven't checked what the code currently does.

Hello,

It's not clear to me why this patch is needed. If one VCPU in the VM is generating code, is it not the software running in the VM responsible for keeping track of the MMU state of the other VCPUs and making sure the new code is executed correctly? Why should KVM get involved?

I don't see how this is different than running on bare metal (no hypervisor), and one CPU with the MMU on generates code that another CPU with the MMU off must execute.

Some comments below.

Quoted 13 lines · Catalin Marinas

Quoted 12 lines · Marc Zyngier

Quoted 10 lines · Catalin Marinas

Quoted 7 lines · Marc Zyngier

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

Digging through the ARM ARM is hard. Do we have this behaviour with FWB as well?

Quoted 8 lines

Quoted 4 lines

The ARM ARM doesn't seem to mention FWB at all when it comes to instruction fetch, which is sort of expected as it only covers the D-side. I *think* we could sidestep this when CTR_EL0.DIC is set though, as the I-side would then snoop the D-side.

Not sure this helps. CTR_EL0.DIC refers to the need for maintenance to PoU while the SCTLR_EL1.M == 0 causes the I-cache to fetch from PoC. I don't think I-cache snooping the D-cache would happen to the PoU when the S1 MMU is off.

FEAT_FWB requires that CLIDR_EL1.{LoUIS, LoUU} = {0, 0} which means that no dcache clean is required for instruction to data coherence (page D13-3086). I interpret that as saying that with FEAT_FWB, CTR_EL0.IDC is effectively 1, which means that dcache clean is not required for instruction generation, and icache invalidation is required only if CTR_EL0.DIC = 0 (according to B2-158).

Quoted 3 lines

My reading of D4.4.4 is that when SCTLR_EL1.M == 0 both I and D accesses are Normal Non-cacheable with a note in D4.4.6 that Non-cacheable accesses may be held in the I-cache.

Nitpicking, but SCTLR_EL1.M == 0 and SCTLR_EL1.I == 1 means that instruction fetches are to Normal Cacheable, Inner and Outer Read-Allocate memory (ARM DDI 0487G.a, pages D5-2709 and indirectly at D13-3586).

Like you've pointed out, as mentioned in D4.4.6, it is always possible that instruction fetches are held in the instruction cache, regardless of the state of the SCTLR_EL1.M bit.

Quoted 6 lines

The FWB rules on combining S1 and S2 says that Normal Non-cacheable at S1 is "upgraded" to cacheable. This should happen irrespective of whether the S1 MMU is on or off and should apply to both I and D accesses (since it does not explicitly says). So I think we could skip this IC IALLU when FWB is present.

Quoted 3 lines

The same logic should apply when the VMM copies the VM text. With FWB, we probably only need D-cache maintenance to PoU and only if CTR_EL0.IDC==0. I haven't checked what the code currently does.

When FEAT_FWB, CTR_EL0.IDC is effectively 1 (see above), so we don't need a dcache clean in this case.

Hi Alex,

Quoted 7 lines · Alexandru Elisei

Hello,

It's not clear to me why this patch is needed. If one VCPU in the VM is generating code, is it not the software running in the VM responsible for keeping track of the MMU state of the other VCPUs and making sure the new code is executed correctly? Why should KVM get involved?

Quoted 3 lines

I don't see how this is different than running on bare metal (no hypervisor), and one CPU with the MMU on generates code that another CPU with the MMU off must execute.

The difference is that so far, we have always considered i-caches to be private to each CPU. With a hypervisor that allows migration of vcpus from one physical CPU to another, the i-cache isn't private anymore from the perspective of the vcpus.

Quoted 32 lines

Some comments below.

Quoted 13 lines · Catalin Marinas

Quoted 12 lines · Marc Zyngier

Quoted 10 lines · Catalin Marinas

Quoted 7 lines · Marc Zyngier

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

Digging through the ARM ARM is hard. Do we have this behaviour with FWB as well?

Quoted 8 lines

Quoted 4 lines

The ARM ARM doesn't seem to mention FWB at all when it comes to instruction fetch, which is sort of expected as it only covers the D-side. I *think* we could sidestep this when CTR_EL0.DIC is set though, as the I-side would then snoop the D-side.

Not sure this helps. CTR_EL0.DIC refers to the need for maintenance to PoU while the SCTLR_EL1.M == 0 causes the I-cache to fetch from PoC. I don't think I-cache snooping the D-cache would happen to the PoU when the S1 MMU is off.

FEAT_FWB requires that CLIDR_EL1.{LoUIS, LoUU} = {0, 0} which means that no dcache clean is required for instruction to data coherence (page D13-3086). I interpret that as saying that with FEAT_FWB, CTR_EL0.IDC is effectively 1, which means that dcache clean is not required for instruction generation, and icache invalidation is required only if CTR_EL0.DIC = 0 (according to B2-158).

Quoted 7 lines

Quoted 3 lines

My reading of D4.4.4 is that when SCTLR_EL1.M == 0 both I and D accesses are Normal Non-cacheable with a note in D4.4.6 that Non-cacheable accesses may be held in the I-cache.

Nitpicking, but SCTLR_EL1.M == 0 and SCTLR_EL1.I == 1 means that instruction fetches are to Normal Cacheable, Inner and Outer Read-Allocate memory (ARM DDI 0487G.a, pages D5-2709 and indirectly at D13-3586).

I think that's the allocation in unified caches, and not necessarily the i-cache, given that it also mention things such as "Inner Write-Through", which makes no sense for the i-cache.

Quoted 3 lines

Like you've pointed out, as mentioned in D4.4.6, it is always possible that instruction fetches are held in the instruction cache, regardless of the state of the SCTLR_EL1.M bit.

Exactly, and that's what breaks things.

Quoted 9 lines

Quoted 9 lines

The FWB rules on combining S1 and S2 says that Normal Non-cacheable at S1 is "upgraded" to cacheable. This should happen irrespective of whether the S1 MMU is on or off and should apply to both I and D accesses (since it does not explicitly says). So I think we could skip this IC IALLU when FWB is present.

The same logic should apply when the VMM copies the VM text. With FWB, we probably only need D-cache maintenance to PoU and only if CTR_EL0.IDC==0. I haven't checked what the code currently does.

Quoted 2 lines

When FEAT_FWB, CTR_EL0.IDC is effectively 1 (see above), so we don't need a dcache clean in this case.

But that isn't what concerns me. FWB is exclusively documented in terms of d-cache, and doesn't describe how that affects the instruction fetch (which is why I'm reluctant to attribute any effect to it).

Thanks,

	M.

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

If we transpose this to vcpus sharing a single physical CPU, it is possible for a vcpu running with its MMU off to influence another vcpu running with its MMU on, as the latter is expected to fetch from the PoU (and self-patching code doesn't flush below that level).

In order to solve this, reuse the vcpu-private TLB invalidation code to apply the same policy to the I-cache, nuking it every time the vcpu runs on a physical CPU that ran another vcpu of the same VM in the past.

This involve renaming __kvm_tlb_flush_local_vmid() to __kvm_flush_cpu_context(), and inserting a local i-cache invalidation there.

Cc [email protected]

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/include/asm/kvm_asm.h   | 4 ++--
 arch/arm64/kvm/arm.c               | 7 ++++++-
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 +++---
 arch/arm64/kvm/hyp/nvhe/tlb.c      | 3 ++-
 arch/arm64/kvm/hyp/vhe/tlb.c       | 3 ++-
 5 files changed, 15 insertions(+), 8 deletions(-)

Quoted 25 lines · Marc Zyngier

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

If we transpose this to vcpus sharing a single physical CPU, it is possible for a vcpu running with its MMU off to influence another vcpu running with its MMU on, as the latter is expected to fetch from the PoU (and self-patching code doesn't flush below that level).

In order to solve this, reuse the vcpu-private TLB invalidation code to apply the same policy to the I-cache, nuking it every time the vcpu runs on a physical CPU that ran another vcpu of the same VM in the past.

This involve renaming __kvm_tlb_flush_local_vmid() to __kvm_flush_cpu_context(), and inserting a local i-cache invalidation there.

Cc [email protected]

Signed-off-by Marc Zyngier <[email protected]>

Quoted 7 lines

---
 arch/arm64/include/asm/kvm_asm.h   | 4 ++--
 arch/arm64/kvm/arm.c               | 7 ++++++-
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 +++---
 arch/arm64/kvm/hyp/nvhe/tlb.c      | 3 ++-
 arch/arm64/kvm/hyp/vhe/tlb.c       | 3 ++-
 5 files changed, 15 insertions(+), 8 deletions(-)

Since the FWB discussion doesn't affect the correctness of this patch:

Acked-by Will Deacon <[email protected]>

It recently became apparent that the ARMv8 architecture has interesting rules regarding attributes being used when fetching instructions if the MMU is off at Stage-1.

In this situation, the CPU is allowed to fetch from the PoC and allocate into the I-cache (unless the memory is mapped with the XN attribute at Stage-2).

[...]