gdb: Fix the cache sync in write_instr()

[markj]

Summary

Previously this code simply wasn't compiled since it tests arm64
instead of aarch64. It was also wrong since it tried to use the
broadcast variant of "ic", which can't be executed in EL0.

Use builtin_clear_cache() instead. It clears the virtual address
range to point of unification and invalidates the corresponding range in
the icache.

[andrew]

__clear_cache may not work for this use on CPUs with a VIPT i-cache. It will only invalidate the i-cache by virtual address, if the instruction is in the guests i-cache it will not be handled correctly.

[andrew]

Having talked to a Linux dev who is familiar with the kvm code I think we need to:

1. ensure all VCPUs are stopped
2. write the instruction
3. clean the d-cache to the point of unification
4. if we have a VIPT icache and CTR_EL0.DIC == 0 then perform the i-cache maintenance in the kernel

[markj]

In D53722#1226796, @andrew wrote:

Having talked to a Linux dev who is familiar with the kvm code I think we need to:

1. ensure all VCPUs are stopped

This is guaranteed here.

1. write the instruction
2. clean the d-cache to the point of unification

Do we need to guarantee that this happens on the same host CPU? If so I think we need to move these operations into the kernel.

1. if we have a VIPT icache and CTR_EL0.DIC == 0 then perform the i-cache maintenance in the kernel

So, one CPU needs to call arm64_icache_sync_range(), I believe. (Can we provide a variant that syncs the whole cache, without providing a range? AFAICS the implementations ignore the range anyway.)

BTW, why does arm64 not provide a sysarch command for this like 32-bit arm does? Should we add it?

[andrew]

In D53722#1229570, @markj wrote:

In D53722#1226796, @andrew wrote:

Having talked to a Linux dev who is familiar with the kvm code I think we need to:

1. ensure all VCPUs are stopped

This is guaranteed here.

1. write the instruction
2. clean the d-cache to the point of unification

Do we need to guarantee that this happens on the same host CPU? If so I think we need to move these operations into the kernel.

It can be on any CPU, the d-cache must behave as if it was implemented as PIPT.

1. if we have a VIPT icache and CTR_EL0.DIC == 0 then perform the i-cache maintenance in the kernel

So, one CPU needs to call arm64_icache_sync_range(), I believe. (Can we provide a variant that syncs the whole cache, without providing a range? AFAICS the implementations ignore the range anyway.)

cpu_icache_sync_range_checked is safer as it includes a fault handler. The range is for the d-cache clean to the point of unification.

I would like to use i-cache invalidate by va, but have never measured an improvement. My guess is it's because we have more sync calls than are needed, e.g. mapping the same pa into two processes will call it twice, even if the memory hasn't changed.

BTW, why does arm64 not provide a sysarch command for this like 32-bit arm does? Should we add it?

For most users it's unneeded, they can control creating the mappings such that all they need is to invalidate by VA, which is executable from userspace.

If we handle this entirely in the kernel then cpu_icache_sync_range_checked should be enough. If not the following should be more complete logic.

if (cache_is_pipt() || CTR_EL0.DIC == 1) {
    __builtin___clear_cache(dest, dest + len);
} else {
    // The d-cache parts could be included with the i-cache handling
    if (CTR_EL0.IDC == 0)
        write_back_d_cache(dest);
    sys_invalidate_i_cache(); // Calls into the kernel to invalidate all the i-cache
}