Re: [PATCH v5 07/19] arm64: rsi: Add support for checking whether an MMIO is protected

From: Gavin Shan
Date: Sun Sep 08 2024 - 19:54:14 EST

On 9/6/24 11:55 PM, Steven Price wrote:
On 06/09/2024 05:32, Gavin Shan wrote:
On 8/19/24 11:19 PM, Steven Price wrote:
From: Suzuki K Poulose <suzuki.poulose@xxxxxxx>

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@xxxxxxx>
Signed-off-by: Steven Price <steven.price@xxxxxxx>
---
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(+)

[...]

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


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

From the cover letter:

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/+/30485

Sorry, 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.

# ./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