Re: [PATCH v5 07/19] arm64: rsi: Add support for checking whether an MMIO is protected
From: Steven Price <steven.price@arm.com>
Date: 2024-09-09 09:31:29
Also in:
kvm, kvmarm, linux-coco, lkml
On 09/09/2024 00:53, Gavin Shan wrote:
On 9/6/24 11:55 PM, Steven Price wrote:quoted
On 06/09/2024 05:32, Gavin Shan wrote:quoted
On 8/19/24 11:19 PM, Steven Price wrote:quoted
From: Suzuki K Poulose <suzuki.poulose@arm.com> On Arm CCA, with RMM-v1.0, all MMIO regions are shared. However, in the future, an Arm CCA-v1.0 compliant guest may be run in a lesser privileged partition in the Realm World (with Arm CCA-v1.1 Planes feature). In this case, some of the MMIO regions may be emulated by a higher privileged component in the Realm world, i.e, protected. Thus the guest must decide today, whether a given MMIO region is shared vs Protected and create the stage1 mapping accordingly. On Arm CCA, this detection is based on the "IPA State" (RIPAS == RIPAS_IO). Provide a helper to run this check on a given range of MMIO. Also, provide a arm64 helper which may be hooked in by other solutions. Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> Signed-off-by: Steven Price <steven.price@arm.com> --- New patch for v5 --- arch/arm64/include/asm/io.h | 8 ++++++++ arch/arm64/include/asm/rsi.h | 3 +++ arch/arm64/include/asm/rsi_cmds.h | 21 +++++++++++++++++++++ arch/arm64/kernel/rsi.c | 26 ++++++++++++++++++++++++++ 4 files changed, 58 insertions(+)[...]quoted
quoted
quoted
diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c index e968a5c9929e..381a5b9a5333 100644 --- a/arch/arm64/kernel/rsi.c +++ b/arch/arm64/kernel/rsi.c@@ -67,6 +67,32 @@ void __init arm64_rsi_setup_memory(void)} } +bool arm64_rsi_is_protected_mmio(phys_addr_t base, size_t size) +{ + enum ripas ripas; + phys_addr_t end, top; + + /* Overflow ? */ + if (WARN_ON(base + size < base)) + return false; + + end = ALIGN(base + size, RSI_GRANULE_SIZE); + base = ALIGN_DOWN(base, RSI_GRANULE_SIZE); + + while (base < end) { + if (WARN_ON(rsi_ipa_state_get(base, end, &ripas, &top))) + break; + if (WARN_ON(top <= base)) + break; + if (ripas != RSI_RIPAS_IO) + break; + base = top; + } + + return (size && base >= end); +}I don't understand why @size needs to be checked here. Its initial value taken from the input parameter should be larger than zero and its value is never updated in the loop. So I'm understanding @size is always larger than zero, and the condition would be something like below if I'm correct.Yes you are correct. I'm not entirely sure why it was written that way. The only change dropping 'size' as you suggest is that a zero-sized region is considered protected. But I'd consider it a bug if this is called with size=0. I'll drop 'size' here.The check 'size == 0' could be squeezed to the overflow check if you agree. /* size == 0 or overflow */ if (WARN_ON(base + size) <= base) return false; : return (base >= end);
Yes that makes sense, thanks for the suggestion.
quoted
quoted
return (base >= end); /* RSI_RIPAS_IO returned for all granules */ Another issue is @top is always zero with the latest tf-rmm. More details are provided below.That suggests that you are not actually using the 'latest' tf-rmm ;) (for some definition of 'latest' which might not be obvious!)quoted
From the cover letter:quoted
As mentioned above the new RMM specification means that corresponding changes need to be made in the RMM, at this time these changes are still in review (see 'topics/rmm-1.0-rel0-rc1'). So you'll need to fetch the changes[3] from the gerrit instance until they are pushed to the main branch. [3] https://review.trustedfirmware.org/c/TF-RMM/tf-rmm/+/30485Sorry, I should probably have made this much more prominent in the cover letter. Running something like... git fetch https://git.trustedfirmware.org/TF-RMM/tf-rmm.git \ refs/changes/85/30485/11 ... should get you the latest. Hopefully these changes will get merged to the main branch soon.My bad. I didn't check the cover letter in time. With this specific TF-RMM branch, I'm able to boot the guest with cca/host-v4 and cca/guest-v5. However, there are messages indicating unhandled system register accesses, as below.
To some extent unhandled system register accesses are expected. The kernel will probe for features, and if the RMM doesn't support them it will be emulating those registers as RAZ/WI. I believe RAZ/WI is an appropriate emulation, so Linux won't have any trouble here, and I don't think there's anything wrong with Linux probing these registers. So the question really is whether the RMM needs to have dummy handlers to silence the 'warnings'. They are currently output using 'INFO' so priority - so will only be visible in a 'debug' build (or if the log level has been explicitly set). Steve
# ./start.sh Info: # lkvm run -k Image -m 256 -c 2 --name guest-152 Info: Removed ghost socket file "/root/.lkvm//guest-152.sock". [ rmm ] SMC_RMI_REALM_CREATE 882860000 880856000 > RMI_SUCCESS [ rmm ] SMC_RMI_REC_AUX_COUNT 882860000 > RMI_SUCCESS 10 [ rmm ] SMC_RMI_REC_CREATE 882860000 88bdc5000 88bdc4000 > RMI_SUCCESS [ rmm ] SMC_RMI_REC_CREATE 882860000 88bdd7000 88bdc4000 > RMI_SUCCESS [ rmm ] SMC_RMI_REALM_ACTIVATE 882860000 > RMI_SUCCESS [ rmm ] Unhandled write S2_0_C0_C2_2 [ rmm ] Unhandled write S3_3_C9_C14_0 [ rmm ] SMC_RSI_VERSION 10000 > RSI_SUCCESS 10000 10000 [ rmm ] SMC_RSI_REALM_CONFIG 82b2b000 > RSI_SUCCESS [ rmm ] SMC_RSI_IPA_STATE_SET 80000000 90000000 1 0 > RSI_SUCCESS 90000000 0 [ rmm ] SMC_RSI_IPA_STATE_GET 1000000 1001000 > RSI_SUCCESS 1001000 0 : [ 1.835570] DMA: preallocated 128 KiB GFP_KERNEL|GFP_DMA32 pool for atomic allocations [ 1.865993] audit: initializing netlink subsys (disabled) [ 1.891218] audit: type=2000 audit(0.492:1): state=initialized audit_enabled=0 res=1 [ 1.899066] thermal_sys: Registered thermal governor 'step_wise' [ 1.920869] thermal_sys: Registered thermal governor 'power_allocator' [ 1.944151] cpuidle: using governor menu [ 1.988588] hw-breakpoint: found 16 breakpoint and 16 watchpoint registers. [ rmm ] Unhandled write S2_0_C0_C0_5 [ rmm ] Unhandled write S2_0_C0_C0_4 [ rmm ] Unhandled write S2_0_C0_C1_5 [ rmm ] Unhandled write S2_0_C0_C1_4 [ rmm ] Unhandled write S2_0_C0_C2_5 : [ rmm ] Unhandled write S2_0_C0_C13_6 [ rmm ] Unhandled write S2_0_C0_C14_7 [ rmm ] Unhandled write S2_0_C0_C14_6 [ rmm ] Unhandled write S2_0_C0_C15_7 [ rmm ] Unhandled write S2_0_C0_C15_6 Thanks, Gavin