[RFC PATCH 00/16] arm: Run Arm CCA VMs with KVM

[RFC PATCH 00/16] arm: Run Arm CCA VMs with KVM

[Jean-Philippe Brucker]

The Arm Realm Management Extension (RME), part of the Arm Confidential
Computing Architecture (CCA), enables running confidential virtual
machines in a new "Realm" security state. While the host still manages
the resources of a guest running in a Realm, it cannot access them.

This series adds some support to QEMU for launching a guest in a Realm
with KVM. The KVM changes for CCA have been posted at [1].

Patches 2-4 introduce a new RmeGuest object that inherits from
ConfidentialGuestSupport and enable it for the virt machine. Like other
confidential guest architectures, launching a Realm VM requires two
command-line parameters:

	-object rme-guest,id=<id>,<parameters>
	-M confidential-guest-support=<id>

Patches 5-6 modify the KVM vCPU support. With CCA, KVM does not sit atop
the VM anymore but talks to a new component, the Realm Management
Monitor (RMM) which deals with the Realm stage-2 page tables and CPU
state. So KVM cannot access most vCPU registers anymore except for
passing parameters to RMM when handling VM exits. Likewise, the host
must not access any memory assigned to the guest (or else it gets a
granule protection fault). The private memfd work [2] by Chao Peng will
help with this.

Patches 8-9 deal with loading images into the Realm. Those are measured
by the RMM and part of the initial measurement, which allows a Realm
owner to attest that the Realm is running what it expects. Patches 10-14
pass parameters described in the RMM specification.


This initial posting only provides direct kernel boot with DTB, not
firmware boot. There is ongoing work to extend edk2 to run in a Realm,
which will require changes to QEMU. A few problems will come up:

* The FwCfg device provides kernel images, initrd, ACPI tables etc. This
  isn't an option for CCA because the guest does not trust what the host
  provides at runtime. I suggest to load all those things in Realm
  memory before boot, and pass their address in the device tree which is
  always present at the start of RAM. This will require new properties
  in the device-tree's chosen section.

* The guest firmware probably shouldn't be on an emulated flash device.
  For one thing, it doesn't need flash because it will store all
  variable in RAM. The flash device also relies on read-only mappings
  which are not supported by KVM RME at the moment, and trapping reads
  would break integrity. I suggest to either replace the flash device
  (address 0 of the virt machine) by RAM when RmeGuest is enabled, or
  load the firmware somewhere else in RAM.


Please see [1] for additional resource, including instructions for
building and running the CCA software stack on a model. An example
command-line:

qemu-system-aarch64
	-M virt -cpu host -enable-kvm -M gic-version=3 -smp 2 -m 256M -nographic 
	-M confidential-guest-support=rme0
	-object rme-guest,id=rme0,measurement-algo=sha512
	-kernel Image -initrd rootfs.cpio
	-append 'console=ttyAMA0 earlycon'
	-overcommit mem-lock=on

A branch with these patches is available at [3].

[1] https://lore.kernel.org/kvm/20230127112248.136810-1-suzuki.poulose@arm.com/
[2] https://lore.kernel.org/qemu-devel/20221202061347.1070246-1-chao.p.peng@linux.intel.com/
[3] https://jpbrucker.net/git/qemu cca/rfc-v1

Jean-Philippe Brucker (16):
  NOMERGE: Add KVM Arm RME definitions to Linux headers
  target/arm: Add confidential guest support
  target/arm/kvm-rme: Initialize realm
  hw/arm/virt: Add support for Arm RME
  target/arm/kvm: Split kvm_arch_get/put_registers
  target/arm/kvm-rme: Initialize vCPU
  target/arm/kvm: Select RME VM type for the scratch VM
  target/arm/kvm-rme: Populate the realm with boot images
  hw/arm/boot: Populate realm memory with boot images
  target/arm/kvm-rme: Add measurement algorithm property
  target/arm/kvm-rme: Add Realm Personalization Value parameter
  target/arm/kvm-rme: Add Realm SVE vector length
  target/arm/kvm-rme: Add breakpoints and watchpoints parameters
  target/arm/kvm-rme: Add PMU num counters parameters
  target/arm/kvm: Disable Realm reboot
  target/arm/kvm-rme: Disable readonly mappings

 docs/system/confidential-guest-support.rst |   1 +
 qapi/qom.json                              |  32 +-
 include/sysemu/kvm.h                       |   2 +
 linux-headers/asm-arm64/kvm.h              |  63 +++
 linux-headers/linux/kvm.h                  |  21 +-
 target/arm/cpu.h                           |   3 +
 target/arm/kvm_arm.h                       |  21 +
 accel/kvm/kvm-all.c                        |   8 +-
 hw/arm/boot.c                              |  10 +-
 hw/arm/virt.c                              |  48 +-
 target/arm/helper.c                        |   8 +
 target/arm/kvm-rme.c                       | 505 +++++++++++++++++++++
 target/arm/kvm.c                           |  20 +-
 target/arm/kvm64.c                         |  91 +++-
 target/arm/meson.build                     |   7 +-
 15 files changed, 822 insertions(+), 18 deletions(-)
 create mode 100644 target/arm/kvm-rme.c

-- 
2.39.0

[RFC PATCH 01/16] NOMERGE: Add KVM Arm RME definitions to Linux headers

[Jean-Philippe Brucker]

Copy the KVM definitions for Arm RME from the development branch.
Don't merge, they will be added from the periodic Linux header sync.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 linux-headers/asm-arm64/kvm.h | 63 +++++++++++++++++++++++++++++++++++
 linux-headers/linux/kvm.h     | 21 +++++++++---
 2 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index 4bf2d7246e..8e04d6f7ff 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -108,6 +108,7 @@ struct kvm_regs {
 #define KVM_ARM_VCPU_SVE		4 /* enable SVE for this CPU */
 #define KVM_ARM_VCPU_PTRAUTH_ADDRESS	5 /* VCPU uses address authentication */
 #define KVM_ARM_VCPU_PTRAUTH_GENERIC	6 /* VCPU uses generic authentication */
+#define KVM_ARM_VCPU_REC		7 /* VCPU REC state as part of Realm */
 
 struct kvm_vcpu_init {
 	__u32 target;
@@ -391,6 +392,68 @@ enum {
 #define   KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES	3
 #define   KVM_DEV_ARM_ITS_CTRL_RESET		4
 
+/* KVM_CAP_ARM_RME on VM fd */
+#define KVM_CAP_ARM_RME_CONFIG_REALM		0
+#define KVM_CAP_ARM_RME_CREATE_RD		1
+#define KVM_CAP_ARM_RME_INIT_IPA_REALM		2
+#define KVM_CAP_ARM_RME_POPULATE_REALM		3
+#define KVM_CAP_ARM_RME_ACTIVATE_REALM		4
+
+#define KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA256		0
+#define KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA512		1
+
+#define KVM_CAP_ARM_RME_RPV_SIZE 64
+
+/* List of configuration items accepted for KVM_CAP_ARM_RME_CONFIG_REALM */
+#define KVM_CAP_ARM_RME_CFG_RPV			0
+#define KVM_CAP_ARM_RME_CFG_HASH_ALGO		1
+#define KVM_CAP_ARM_RME_CFG_SVE			2
+#define KVM_CAP_ARM_RME_CFG_DBG			3
+#define KVM_CAP_ARM_RME_CFG_PMU			4
+
+struct kvm_cap_arm_rme_config_item {
+	__u32 cfg;
+	union {
+		/* cfg == KVM_CAP_ARM_RME_CFG_RPV */
+		struct {
+			__u8	rpv[KVM_CAP_ARM_RME_RPV_SIZE];
+		};
+
+		/* cfg == KVM_CAP_ARM_RME_CFG_HASH_ALGO */
+		struct {
+			__u32	hash_algo;
+		};
+
+		/* cfg == KVM_CAP_ARM_RME_CFG_SVE */
+		struct {
+			__u32	sve_vq;
+		};
+
+		/* cfg == KVM_CAP_ARM_RME_CFG_DBG */
+		struct {
+			__u32	num_brps;
+			__u32	num_wrps;
+		};
+
+		/* cfg == KVM_CAP_ARM_RME_CFG_PMU */
+		struct {
+			__u32	num_pmu_cntrs;
+		};
+		/* Fix the size of the union */
+		__u8	reserved[256];
+	};
+};
+
+struct kvm_cap_arm_rme_populate_realm_args {
+	__u64 populate_ipa_base;
+	__u64 populate_ipa_size;
+};
+
+struct kvm_cap_arm_rme_init_ipa_args {
+	__u64 init_ipa_base;
+	__u64 init_ipa_size;
+};
+
 /* Device Control API on vcpu fd */
 #define KVM_ARM_VCPU_PMU_V3_CTRL	0
 #define   KVM_ARM_VCPU_PMU_V3_IRQ	0
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index ebdafa576d..9d5affc98a 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -901,14 +901,25 @@ struct kvm_ppc_resize_hpt {
 #define KVM_S390_SIE_PAGE_OFFSET 1
 
 /*
- * On arm64, machine type can be used to request the physical
- * address size for the VM. Bits[7-0] are reserved for the guest
- * PA size shift (i.e, log2(PA_Size)). For backward compatibility,
- * value 0 implies the default IPA size, 40bits.
+ * On arm64, machine type can be used to request both the machine type and
+ * the physical address size for the VM.
+ *
+ * Bits[11-8] are reserved for the ARM specific machine type.
+ *
+ * Bits[7-0] are reserved for the guest PA size shift (i.e, log2(PA_Size)).
+ * For backward compatibility, value 0 implies the default IPA size, 40bits.
  */
+#define KVM_VM_TYPE_ARM_SHIFT		8
+#define KVM_VM_TYPE_ARM_MASK		(0xfULL << KVM_VM_TYPE_ARM_SHIFT)
+#define KVM_VM_TYPE_ARM(_type)		\
+	(((_type) << KVM_VM_TYPE_ARM_SHIFT) & KVM_VM_TYPE_ARM_MASK)
+#define KVM_VM_TYPE_ARM_NORMAL		KVM_VM_TYPE_ARM(0)
+#define KVM_VM_TYPE_ARM_REALM		KVM_VM_TYPE_ARM(1)
+
 #define KVM_VM_TYPE_ARM_IPA_SIZE_MASK	0xffULL
 #define KVM_VM_TYPE_ARM_IPA_SIZE(x)		\
 	((x) & KVM_VM_TYPE_ARM_IPA_SIZE_MASK)
+
 /*
  * ioctls for /dev/kvm fds:
  */
@@ -1176,6 +1187,8 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_S390_ZPCI_OP 221
 #define KVM_CAP_S390_CPU_TOPOLOGY 222
 
+#define KVM_CAP_ARM_RME 300 // FIXME: Large number to prevent conflicts
+
 #ifdef KVM_CAP_IRQ_ROUTING
 
 struct kvm_irq_routing_irqchip {
-- 
2.39.0

[RFC PATCH 02/16] target/arm: Add confidential guest support

[Jean-Philippe Brucker]

Add a new RmeGuest object, inheriting from ConfidentialGuestSupport, to
support the Arm Realm Management Extension (RME). It is instantiated by
passing on the command-line:

  -M virt,confidential-guest-support=<id>
  -object guest-rme,id=<id>[,options...]

This is only the skeleton. Support will be added in following patches.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 docs/system/confidential-guest-support.rst |  1 +
 qapi/qom.json                              |  3 +-
 target/arm/kvm-rme.c                       | 48 ++++++++++++++++++++++
 target/arm/meson.build                     |  7 +++-
 4 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 target/arm/kvm-rme.c

diff --git a/docs/system/confidential-guest-support.rst b/docs/system/confidential-guest-support.rst
index 0c490dbda2..acf46d8856 100644
--- a/docs/system/confidential-guest-support.rst
+++ b/docs/system/confidential-guest-support.rst
@@ -40,5 +40,6 @@ Currently supported confidential guest mechanisms are:
 * AMD Secure Encrypted Virtualization (SEV) (see :doc:`i386/amd-memory-encryption`)
 * POWER Protected Execution Facility (PEF) (see :ref:`power-papr-protected-execution-facility-pef`)
 * s390x Protected Virtualization (PV) (see :doc:`s390x/protvirt`)
+* Arm Realm Management Extension (RME)
 
 Other mechanisms may be supported in future.
diff --git a/qapi/qom.json b/qapi/qom.json
index 30e76653ad..7ca27bb86c 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -917,7 +917,8 @@
     'tls-creds-x509',
     'tls-cipher-suites',
     { 'name': 'x-remote-object', 'features': [ 'unstable' ] },
-    { 'name': 'x-vfio-user-server', 'features': [ 'unstable' ] }
+    { 'name': 'x-vfio-user-server', 'features': [ 'unstable' ] },
+    'rme-guest'
   ] }
 
 ##
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
new file mode 100644
index 0000000000..22aa3dc712
--- /dev/null
+++ b/target/arm/kvm-rme.c
@@ -0,0 +1,48 @@
+/*
+ * QEMU Arm RME support
+ *
+ * Copyright Linaro 2022
+ */
+
+#include "qemu/osdep.h"
+
+#include "exec/confidential-guest-support.h"
+#include "hw/boards.h"
+#include "hw/core/cpu.h"
+#include "kvm_arm.h"
+#include "migration/blocker.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "sysemu/kvm.h"
+#include "sysemu/runstate.h"
+
+#define TYPE_RME_GUEST "rme-guest"
+OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
+
+typedef struct RmeGuest RmeGuest;
+
+struct RmeGuest {
+    ConfidentialGuestSupport parent_obj;
+};
+
+static void rme_guest_class_init(ObjectClass *oc, void *data)
+{
+}
+
+static const TypeInfo rme_guest_info = {
+    .parent = TYPE_CONFIDENTIAL_GUEST_SUPPORT,
+    .name = TYPE_RME_GUEST,
+    .instance_size = sizeof(struct RmeGuest),
+    .class_init = rme_guest_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_USER_CREATABLE },
+        { }
+    }
+};
+
+static void rme_register_types(void)
+{
+    type_register_static(&rme_guest_info);
+}
+
+type_init(rme_register_types);
diff --git a/target/arm/meson.build b/target/arm/meson.build
index 87e911b27f..a2224c0d23 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -40,7 +40,12 @@ arm_ss.add(files(
 ))
 arm_ss.add(zlib)
 
-arm_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c', 'kvm64.c'), if_false: files('kvm-stub.c'))
+arm_ss.add(when: 'CONFIG_KVM',
+  if_true: files(
+    'kvm.c',
+    'kvm64.c',
+    'kvm-rme.c'),
+  if_false: files('kvm-stub.c'))
 
 arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
   'cpu64.c',
-- 
2.39.0

[RFC PATCH 03/16] target/arm/kvm-rme: Initialize realm

[Jean-Philippe Brucker]

The machine code calls kvm_arm_rme_vm_type() to get the VM flag and
kvm_arm_rme_init() to issue KVM hypercalls in the required order:

* create the realm descriptor early,
* finalize the REC (vCPU) after the registers are reset,
* load images into Realm RAM (in another patch),
* activate the realm at the end, at which point the realm is sealed.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 target/arm/kvm_arm.h |  14 ++++++
 target/arm/kvm-rme.c | 101 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 99017b635c..00d3df8cac 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -369,6 +369,11 @@ void kvm_arm_pvtime_init(CPUState *cs, uint64_t ipa);
 
 int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
 
+int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp);
+int kvm_arm_rme_vm_type(MachineState *ms);
+
+bool kvm_arm_rme_enabled(void);
+
 #else
 
 /*
@@ -443,6 +448,15 @@ static inline uint32_t kvm_arm_sve_get_vls(CPUState *cs)
     g_assert_not_reached();
 }
 
+static inline int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
+{
+    return 0;
+}
+
+static inline int kvm_arm_rme_vm_type(MachineState *ms)
+{
+    return 0;
+}
 #endif
 
 static inline const char *gic_class_name(void)
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 22aa3dc712..d7cdca1cbf 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -25,6 +25,107 @@ struct RmeGuest {
     ConfidentialGuestSupport parent_obj;
 };
 
+static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
+{
+    if (!cgs) {
+        return NULL;
+    }
+    return (RmeGuest *)object_dynamic_cast(OBJECT(cgs), TYPE_RME_GUEST);
+}
+
+bool kvm_arm_rme_enabled(void)
+{
+    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
+
+    return !!cgs_to_rme(cgs);
+}
+
+static int rme_create_rd(RmeGuest *guest, Error **errp)
+{
+    int ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                                KVM_CAP_ARM_RME_CREATE_RD);
+
+    if (ret) {
+        error_setg_errno(errp, -ret, "RME: failed to create Realm Descriptor");
+    }
+    return ret;
+}
+
+static void rme_vm_state_change(void *opaque, bool running, RunState state)
+{
+    int ret;
+    CPUState *cs;
+
+    if (state != RUN_STATE_RUNNING) {
+        return;
+    }
+
+    /*
+     * Now that do_cpu_reset() initialized the boot PC and
+     * kvm_cpu_synchronize_post_reset() registered it, we can finalize the REC.
+     */
+    CPU_FOREACH(cs) {
+        ret = kvm_arm_vcpu_finalize(cs, KVM_ARM_VCPU_REC);
+        if (ret) {
+            error_setg_errno(&error_fatal, -ret,
+                             "RME: failed to finalize vCPU");
+        }
+    }
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                            KVM_CAP_ARM_RME_ACTIVATE_REALM);
+    if (ret) {
+        error_setg_errno(&error_fatal, -ret, "RME: failed to activate realm");
+    }
+}
+
+int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
+{
+    int ret;
+    static Error *rme_mig_blocker;
+    RmeGuest *guest = cgs_to_rme(cgs);
+
+    if (!guest) {
+        /* Either no cgs, or another confidential guest type */
+        return 0;
+    }
+
+    if (!kvm_enabled()) {
+        error_setg(errp, "KVM required for RME");
+        return -ENODEV;
+    }
+
+    if (!kvm_check_extension(kvm_state, KVM_CAP_ARM_RME)) {
+        error_setg(errp, "KVM does not support RME");
+        return -ENODEV;
+    }
+
+    ret = rme_create_rd(guest, errp);
+    if (ret) {
+        return ret;
+    }
+
+    error_setg(&rme_mig_blocker, "RME: migration is not implemented");
+    migrate_add_blocker(rme_mig_blocker, &error_fatal);
+
+    /*
+     * The realm activation is done last, when the VM starts, after all images
+     * have been loaded and all vcpus finalized.
+     */
+    qemu_add_vm_change_state_handler(rme_vm_state_change, guest);
+
+    cgs->ready = true;
+    return 0;
+}
+
+int kvm_arm_rme_vm_type(MachineState *ms)
+{
+    if (cgs_to_rme(ms->cgs)) {
+        return KVM_VM_TYPE_ARM_REALM;
+    }
+    return 0;
+}
+
 static void rme_guest_class_init(ObjectClass *oc, void *data)
 {
 }
-- 
2.39.0

[RFC PATCH 04/16] hw/arm/virt: Add support for Arm RME

[Jean-Philippe Brucker]

When confidential-guest-support is enabled for the virt machine, call
the RME init function, and add the RME flag to the VM type.

* The Realm differentiates non-secure from realm memory using the upper
  GPA bit. Reserve that bit when creating the memory map, to make sure
  that device MMIO located in high memory can still fit.

* pvtime is disabled for the moment. Since the hypervisor has to write
  into the shared pvtime page before scheduling a vcpu, it seems
  incompatible with confidential guests.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/arm/virt.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b871350856..df613e634a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -210,6 +210,11 @@ static const char *valid_cpus[] = {
     ARM_CPU_TYPE_NAME("max"),
 };
 
+static bool virt_machine_is_confidential(VirtMachineState *vms)
+{
+    return MACHINE(vms)->cgs;
+}
+
 static bool cpu_type_valid(const char *cpu)
 {
     int i;
@@ -247,6 +252,14 @@ static void create_fdt(VirtMachineState *vms)
         exit(1);
     }
 
+    /*
+     * Since the devicetree is included in the initial measurement, it must
+     * not contain random data.
+     */
+    if (virt_machine_is_confidential(vms)) {
+        vms->dtb_randomness = false;
+    }
+
     ms->fdt = fdt;
 
     /* Header */
@@ -1924,6 +1937,15 @@ static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem)
     steal_time = object_property_get_bool(OBJECT(first_cpu),
                                           "kvm-steal-time", NULL);
 
+    if (virt_machine_is_confidential(vms)) {
+        /*
+         * The host cannot write into a confidential guest's memory until the
+         * guest shares it. Since the host writes the pvtime region before the
+         * guest gets a chance to set it up, disable pvtime.
+         */
+        steal_time = false;
+    }
+
     if (kvm_enabled()) {
         hwaddr pvtime_reg_base = vms->memmap[VIRT_PVTIME].base;
         hwaddr pvtime_reg_size = vms->memmap[VIRT_PVTIME].size;
@@ -2053,10 +2075,11 @@ static void machvirt_init(MachineState *machine)
      * if the guest has EL2 then we will use SMC as the conduit,
      * and otherwise we will use HVC (for backwards compatibility and
      * because if we're using KVM then we must use HVC).
+     * Realm guests must also use SMC.
      */
     if (vms->secure && firmware_loaded) {
         vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
-    } else if (vms->virt) {
+    } else if (vms->virt || virt_machine_is_confidential(vms)) {
         vms->psci_conduit = QEMU_PSCI_CONDUIT_SMC;
     } else {
         vms->psci_conduit = QEMU_PSCI_CONDUIT_HVC;
@@ -2102,6 +2125,8 @@ static void machvirt_init(MachineState *machine)
         exit(1);
     }
 
+    kvm_arm_rme_init(machine->cgs, &error_fatal);
+
     create_fdt(vms);
 
     assert(possible_cpus->len == max_cpus);
@@ -2854,15 +2879,26 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
 static int virt_kvm_type(MachineState *ms, const char *type_str)
 {
     VirtMachineState *vms = VIRT_MACHINE(ms);
+    int rme_vm_type = kvm_arm_rme_vm_type(ms);
     int max_vm_pa_size, requested_pa_size;
+    int rme_reserve_bit = 0;
     bool fixed_ipa;
 
-    max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa);
+    if (rme_vm_type) {
+        /*
+         * With RME, the upper GPA bit differentiates Realm from NS memory.
+         * Reserve the upper bit to guarantee that highmem devices will fit.
+         */
+        rme_reserve_bit = 1;
+    }
+
+    max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa) -
+                     rme_reserve_bit;
 
     /* we freeze the memory map to compute the highest gpa */
     virt_set_memmap(vms, max_vm_pa_size);
 
-    requested_pa_size = 64 - clz64(vms->highest_gpa);
+    requested_pa_size = 64 - clz64(vms->highest_gpa) + rme_reserve_bit;
 
     /*
      * KVM requires the IPA size to be at least 32 bits.
@@ -2883,7 +2919,11 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
      * the implicit legacy 40b IPA setting, in which case the kvm_type
      * must be 0.
      */
-    return fixed_ipa ? 0 : requested_pa_size;
+    if (fixed_ipa) {
+        return 0;
+    }
+
+    return requested_pa_size | rme_vm_type;
 }
 
 static void virt_machine_class_init(ObjectClass *oc, void *data)
-- 
2.39.0

[RFC PATCH 05/16] target/arm/kvm: Split kvm_arch_get/put_registers

[Jean-Philippe Brucker]

The confidential guest support in KVM limits the number of registers
that we can read and write. Split the get/put_registers function to
prepare for it.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 target/arm/kvm64.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 1197253d12..55191496f3 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1080,7 +1080,7 @@ static int kvm_arch_put_sve(CPUState *cs)
     return 0;
 }
 
-int kvm_arch_put_registers(CPUState *cs, int level)
+static int kvm_arm_put_core_regs(CPUState *cs)
 {
     struct kvm_one_reg reg;
     uint64_t val;
@@ -1200,6 +1200,19 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    return 0;
+}
+
+int kvm_arch_put_registers(CPUState *cs, int level)
+{
+    int ret;
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    ret = kvm_arm_put_core_regs(cs);
+    if (ret) {
+        return ret;
+    }
+
     write_cpustate_to_list(cpu, true);
 
     if (!write_list_to_kvmstate(cpu, level)) {
@@ -1293,7 +1306,7 @@ static int kvm_arch_get_sve(CPUState *cs)
     return 0;
 }
 
-int kvm_arch_get_registers(CPUState *cs)
+static int kvm_arm_get_core_regs(CPUState *cs)
 {
     struct kvm_one_reg reg;
     uint64_t val;
@@ -1413,6 +1426,19 @@ int kvm_arch_get_registers(CPUState *cs)
     }
     vfp_set_fpcr(env, fpr);
 
+    return 0;
+}
+
+int kvm_arch_get_registers(CPUState *cs)
+{
+    int ret;
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    ret = kvm_arm_get_core_regs(cs);
+    if (ret) {
+        return ret;
+    }
+
     ret = kvm_get_vcpu_events(cpu);
     if (ret) {
         return ret;
-- 
2.39.0

[RFC PATCH 06/16] target/arm/kvm-rme: Initialize vCPU

[Jean-Philippe Brucker]

The target code calls kvm_arm_vcpu_init() to mark the vCPU as part of a
realm. RME support does not use the register lists, because the host can
only set the boot PC and registers x0-x7. The rest is private to the
Realm and saved/restored by the RMM.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 target/arm/cpu.h     |  3 ++
 target/arm/kvm_arm.h |  1 +
 target/arm/helper.c  |  8 ++++++
 target/arm/kvm-rme.c | 10 +++++++
 target/arm/kvm.c     | 12 ++++++++
 target/arm/kvm64.c   | 65 ++++++++++++++++++++++++++++++++++++++++++--
 6 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9aeed3c848..7d8397985f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -937,6 +937,9 @@ struct ArchCPU {
     /* KVM steal time */
     OnOffAuto kvm_steal_time;
 
+    /* Realm Management Extension */
+    bool kvm_rme;
+
     /* Uniprocessor system with MP extensions */
     bool mp_is_up;
 
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 00d3df8cac..e4dc7fbb8d 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -373,6 +373,7 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp);
 int kvm_arm_rme_vm_type(MachineState *ms);
 
 bool kvm_arm_rme_enabled(void);
+int kvm_arm_rme_vcpu_init(CPUState *cs);
 
 #else
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d8c8223ec3..52360ae2ff 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -126,6 +126,10 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)
     int i;
     bool ok = true;
 
+    if (cpu->kvm_rme) {
+        return ok;
+    }
+
     for (i = 0; i < cpu->cpreg_array_len; i++) {
         uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
         const ARMCPRegInfo *ri;
@@ -171,6 +175,10 @@ bool write_list_to_cpustate(ARMCPU *cpu)
     int i;
     bool ok = true;
 
+    if (cpu->kvm_rme) {
+        return ok;
+    }
+
     for (i = 0; i < cpu->cpreg_array_len; i++) {
         uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
         uint64_t v = cpu->cpreg_values[i];
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index d7cdca1cbf..3833b187f9 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -118,6 +118,16 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
     return 0;
 }
 
+int kvm_arm_rme_vcpu_init(CPUState *cs)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    if (kvm_arm_rme_enabled()) {
+        cpu->kvm_rme = true;
+    }
+    return 0;
+}
+
 int kvm_arm_rme_vm_type(MachineState *ms)
 {
     if (cgs_to_rme(ms->cgs)) {
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index f022c644d2..fcddead4fe 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -449,6 +449,10 @@ int kvm_arm_init_cpreg_list(ARMCPU *cpu)
     int i, ret, arraylen;
     CPUState *cs = CPU(cpu);
 
+    if (cpu->kvm_rme) {
+        return 0;
+    }
+
     rl.n = 0;
     ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
     if (ret != -E2BIG) {
@@ -521,6 +525,10 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
     int i;
     bool ok = true;
 
+    if (cpu->kvm_rme) {
+        return ok;
+    }
+
     for (i = 0; i < cpu->cpreg_array_len; i++) {
         struct kvm_one_reg r;
         uint64_t regidx = cpu->cpreg_indexes[i];
@@ -557,6 +565,10 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
     int i;
     bool ok = true;
 
+    if (cpu->kvm_rme) {
+        return ok;
+    }
+
     for (i = 0; i < cpu->cpreg_array_len; i++) {
         struct kvm_one_reg r;
         uint64_t regidx = cpu->cpreg_indexes[i];
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 55191496f3..b6320672b2 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -887,6 +887,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return ret;
     }
 
+    ret = kvm_arm_rme_vcpu_init(cs);
+    if (ret) {
+        return ret;
+    }
+
     if (cpu_isar_feature(aa64_sve, cpu)) {
         ret = kvm_arm_sve_set_vls(cs);
         if (ret) {
@@ -1080,6 +1085,35 @@ static int kvm_arch_put_sve(CPUState *cs)
     return 0;
 }
 
+static int kvm_arm_rme_put_core_regs(CPUState *cs, int level)
+{
+    int i, ret;
+    struct kvm_one_reg reg;
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    /*
+     * The RME ABI only allows us to set 8 GPRs and the PC
+     */
+    for (i = 0; i < 8; i++) {
+        reg.id = AARCH64_CORE_REG(regs.regs[i]);
+        reg.addr = (uintptr_t) &env->xregs[i];
+        ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
+        if (ret) {
+            return ret;
+        }
+    }
+
+    reg.id = AARCH64_CORE_REG(regs.pc);
+    reg.addr = (uintptr_t) &env->pc;
+    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
+    if (ret) {
+        return ret;
+    }
+
+    return 0;
+}
+
 static int kvm_arm_put_core_regs(CPUState *cs)
 {
     struct kvm_one_reg reg;
@@ -1208,7 +1242,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     int ret;
     ARMCPU *cpu = ARM_CPU(cs);
 
-    ret = kvm_arm_put_core_regs(cs);
+    if (cpu->kvm_rme) {
+        ret = kvm_arm_rme_put_core_regs(cs, level);
+    } else {
+        ret = kvm_arm_put_core_regs(cs);
+    }
     if (ret) {
         return ret;
     }
@@ -1306,6 +1344,25 @@ static int kvm_arch_get_sve(CPUState *cs)
     return 0;
 }
 
+static int kvm_arm_rme_get_core_regs(CPUState *cs)
+{
+    int i, ret;
+    struct kvm_one_reg reg;
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    for (i = 0; i < 8; i++) {
+        reg.id = AARCH64_CORE_REG(regs.regs[i]);
+        reg.addr = (uintptr_t) &env->xregs[i];
+        ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+        if (ret) {
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
 static int kvm_arm_get_core_regs(CPUState *cs)
 {
     struct kvm_one_reg reg;
@@ -1434,7 +1491,11 @@ int kvm_arch_get_registers(CPUState *cs)
     int ret;
     ARMCPU *cpu = ARM_CPU(cs);
 
-    ret = kvm_arm_get_core_regs(cs);
+    if (cpu->kvm_rme) {
+        ret = kvm_arm_rme_get_core_regs(cs);
+    } else {
+        ret = kvm_arm_get_core_regs(cs);
+    }
     if (ret) {
         return ret;
     }
-- 
2.39.0

[RFC PATCH 07/16] target/arm/kvm: Select RME VM type for the scratch VM

[Jean-Philippe Brucker]

Although the VM type does not affect values probed from the scratch vCPU
at the moment, it could later. Ensure we specify the right type when
creating the temporary VM.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
Does the PA size need changing as well?
---
 target/arm/kvm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index fcddead4fe..d8655d9041 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -70,6 +70,7 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
 {
     int ret = 0, kvmfd = -1, vmfd = -1, cpufd = -1;
     int max_vm_pa_size;
+    int vm_type;
 
     kvmfd = qemu_open_old("/dev/kvm", O_RDWR);
     if (kvmfd < 0) {
@@ -79,8 +80,10 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
     if (max_vm_pa_size < 0) {
         max_vm_pa_size = 0;
     }
+
+    vm_type = kvm_arm_rme_vm_type(MACHINE(qdev_get_machine()));
     do {
-        vmfd = ioctl(kvmfd, KVM_CREATE_VM, max_vm_pa_size);
+        vmfd = ioctl(kvmfd, KVM_CREATE_VM, vm_type | max_vm_pa_size);
     } while (vmfd == -1 && errno == EINTR);
     if (vmfd < 0) {
         goto err;
-- 
2.39.0

[RFC PATCH 08/16] target/arm/kvm-rme: Populate the realm with boot images

[Jean-Philippe Brucker]

Initialize the GPA space and populate it with boot images (kernel,
initrd, firmware, etc). Populating has to be done at VM start time,
because the images are loaded during reset by rom_reset()

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 target/arm/kvm_arm.h |  6 ++++
 target/arm/kvm-rme.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index e4dc7fbb8d..cec6500603 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -371,6 +371,7 @@ int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
 
 int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp);
 int kvm_arm_rme_vm_type(MachineState *ms);
+void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size, hwaddr dst_size);
 
 bool kvm_arm_rme_enabled(void);
 int kvm_arm_rme_vcpu_init(CPUState *cs);
@@ -458,6 +459,11 @@ static inline int kvm_arm_rme_vm_type(MachineState *ms)
 {
     return 0;
 }
+
+static inline void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size,
+                                        hwaddr dst_size)
+{
+}
 #endif
 
 static inline const char *gic_class_name(void)
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 3833b187f9..c8c019f78a 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -9,6 +9,7 @@
 #include "exec/confidential-guest-support.h"
 #include "hw/boards.h"
 #include "hw/core/cpu.h"
+#include "hw/loader.h"
 #include "kvm_arm.h"
 #include "migration/blocker.h"
 #include "qapi/error.h"
@@ -19,12 +20,22 @@
 #define TYPE_RME_GUEST "rme-guest"
 OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
 
+#define RME_PAGE_SIZE qemu_real_host_page_size()
+
 typedef struct RmeGuest RmeGuest;
 
 struct RmeGuest {
     ConfidentialGuestSupport parent_obj;
 };
 
+struct RmeImage {
+    hwaddr base;
+    hwaddr src_size;
+    hwaddr dst_size;
+};
+
+static GSList *rme_images;
+
 static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
 {
     if (!cgs) {
@@ -51,6 +62,38 @@ static int rme_create_rd(RmeGuest *guest, Error **errp)
     return ret;
 }
 
+static void rme_populate_realm(gpointer data, gpointer user_data)
+{
+    int ret;
+    struct RmeImage *image = data;
+    struct kvm_cap_arm_rme_init_ipa_args init_args = {
+        .init_ipa_base = image->base,
+        .init_ipa_size = image->dst_size,
+    };
+    struct kvm_cap_arm_rme_populate_realm_args populate_args = {
+        .populate_ipa_base = image->base,
+        .populate_ipa_size = image->src_size,
+    };
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                            KVM_CAP_ARM_RME_INIT_IPA_REALM,
+                            (intptr_t)&init_args);
+    if (ret) {
+        error_setg_errno(&error_fatal, -ret,
+                         "RME: failed to initialize GPA range (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
+                         image->base, image->dst_size);
+    }
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                            KVM_CAP_ARM_RME_POPULATE_REALM,
+                            (intptr_t)&populate_args);
+    if (ret) {
+        error_setg_errno(&error_fatal, -ret,
+                         "RME: failed to populate realm (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
+                         image->base, image->src_size);
+    }
+}
+
 static void rme_vm_state_change(void *opaque, bool running, RunState state)
 {
     int ret;
@@ -72,6 +115,9 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state)
         }
     }
 
+    g_slist_foreach(rme_images, rme_populate_realm, NULL);
+    g_slist_free_full(g_steal_pointer(&rme_images), g_free);
+
     ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
                             KVM_CAP_ARM_RME_ACTIVATE_REALM);
     if (ret) {
@@ -118,6 +164,39 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
     return 0;
 }
 
+/*
+ * kvm_arm_rme_add_blob - Initialize the Realm IPA range and set up the image.
+ *
+ * @src_size is the number of bytes of the source image, to be populated into
+ *   Realm memory.
+ * @dst_size is the effective image size, which may be larger than @src_size.
+ *   For a kernel @dst_size may include zero-initialized regions such as the BSS
+ *   and initial page directory.
+ */
+void kvm_arm_rme_add_blob(hwaddr base, hwaddr src_size, hwaddr dst_size)
+{
+    struct RmeImage *image;
+
+    if (!kvm_arm_rme_enabled()) {
+        return;
+    }
+
+    base = QEMU_ALIGN_DOWN(base, RME_PAGE_SIZE);
+    src_size = QEMU_ALIGN_UP(src_size, RME_PAGE_SIZE);
+    dst_size = QEMU_ALIGN_UP(dst_size, RME_PAGE_SIZE);
+
+    image = g_malloc0(sizeof(*image));
+    image->base = base;
+    image->src_size = src_size;
+    image->dst_size = dst_size;
+
+    /*
+     * The ROM loader will only load the images during reset, so postpone the
+     * populate call until VM start.
+     */
+    rme_images = g_slist_prepend(rme_images, image);
+}
+
 int kvm_arm_rme_vcpu_init(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
-- 
2.39.0

[RFC PATCH 09/16] hw/arm/boot: Populate realm memory with boot images

[Jean-Philippe Brucker]

When using the Arm RME, register the images to be loaded into Realm
memory at boot. Two operations are needed for each image:
(1) INIT_IPA_REALM: mark the load addresses (IPA) as RAM with
    RMI_RTT_INIT_RIPAS.
(2) POPULATE_REALM: move the page into the Realm with RMI_DATA_CREATE.
    Its content contributes to the initial measurement.

The reason we separate (1) from (2) is that we may need to declare more
RAM than the image size. In particular booting arm64 Linux requires
reserving additional BSS space after the loaded image.

We could declare the whole guest RAM with INIT_IPA_REALM, though that
might be wasteful in terms of stage-2 mappings if the guest is not going
to use all that RAM.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/arm/boot.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 15c2bf1867..115d3f5dcc 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -25,6 +25,7 @@
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/units.h"
+#include "kvm_arm.h"
 
 /* Kernel boot protocol is specified in the kernel docs
  * Documentation/arm/Booting and Documentation/arm64/booting.txt
@@ -192,9 +193,11 @@ static void write_bootloader(const char *name, hwaddr addr,
         code[i] = tswap32(insn);
     }
 
-    assert((len * sizeof(uint32_t)) < BOOTLOADER_MAX_SIZE);
+    len *= sizeof(uint32_t);
+    assert(len < BOOTLOADER_MAX_SIZE);
 
-    rom_add_blob_fixed_as(name, code, len * sizeof(uint32_t), addr, as);
+    rom_add_blob_fixed_as(name, code, len, addr, as);
+    kvm_arm_rme_add_blob(addr, len, len);
 
     g_free(code);
 }
@@ -683,6 +686,7 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
      * the DTB is copied again upon reset, even if addr points into RAM.
      */
     rom_add_blob_fixed_as("dtb", fdt, size, addr, as);
+    kvm_arm_rme_add_blob(addr, size, size);
     qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
                                        rom_ptr_for_as(as, addr, size));
 
@@ -964,6 +968,7 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
 
     *entry = mem_base + kernel_load_offset;
     rom_add_blob_fixed_as(filename, buffer, size, *entry, as);
+    kvm_arm_rme_add_blob(*entry, size, kernel_size);
 
     g_free(buffer);
 
@@ -1119,6 +1124,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
             initrd_size = 0;
         }
         info->initrd_size = initrd_size;
+        kvm_arm_rme_add_blob(info->initrd_start, initrd_size, initrd_size);
 
         fixupcontext[FIXUP_BOARDID] = info->board_id;
         fixupcontext[FIXUP_BOARD_SETUP] = info->board_setup_addr;
-- 
2.39.0

[RFC PATCH 10/16] target/arm/kvm-rme: Add measurement algorithm property

[Jean-Philippe Brucker]

This option selects which measurement algorithm to use for attestation.
Supported values are sha256 and sha512.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 qapi/qom.json        | 14 ++++++++-
 target/arm/kvm-rme.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 7ca27bb86c..87fe7c31fe 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -855,6 +855,17 @@
   'data': { '*cpu-affinity': ['uint16'],
             '*node-affinity': ['uint16'] } }
 
+##
+# @RmeGuestProperties:
+#
+# Properties for rme-guest objects.
+#
+# @measurement-algo: Realm measurement algorithm (default: RMM default)
+#
+# Since: FIXME
+##
+{ 'struct': 'RmeGuestProperties',
+  'data': { '*measurement-algo': 'str' } }
 
 ##
 # @ObjectType:
@@ -985,7 +996,8 @@
       'tls-creds-x509':             'TlsCredsX509Properties',
       'tls-cipher-suites':          'TlsCredsProperties',
       'x-remote-object':            'RemoteObjectProperties',
-      'x-vfio-user-server':         'VfioUserServerProperties'
+      'x-vfio-user-server':         'VfioUserServerProperties',
+      'rme-guest':                  'RmeGuestProperties'
   } }
 
 ##
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index c8c019f78a..3929b941ae 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -22,10 +22,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
 
 #define RME_PAGE_SIZE qemu_real_host_page_size()
 
+#define RME_MAX_CFG         1
+
 typedef struct RmeGuest RmeGuest;
 
 struct RmeGuest {
     ConfidentialGuestSupport parent_obj;
+    char *measurement_algo;
 };
 
 struct RmeImage {
@@ -62,6 +65,40 @@ static int rme_create_rd(RmeGuest *guest, Error **errp)
     return ret;
 }
 
+static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp)
+{
+    int ret;
+    const char *cfg_str;
+    struct kvm_cap_arm_rme_config_item args = {
+        .cfg = cfg,
+    };
+
+    switch (cfg) {
+    case KVM_CAP_ARM_RME_CFG_HASH_ALGO:
+        if (!guest->measurement_algo) {
+            return 0;
+        }
+        if (!strcmp(guest->measurement_algo, "sha256")) {
+            args.hash_algo = KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA256;
+        } else if (!strcmp(guest->measurement_algo, "sha512")) {
+            args.hash_algo = KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA512;
+        } else {
+            g_assert_not_reached();
+        }
+        cfg_str = "hash algorithm";
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                            KVM_CAP_ARM_RME_CONFIG_REALM, (intptr_t)&args);
+    if (ret) {
+        error_setg_errno(errp, -ret, "RME: failed to configure %s", cfg_str);
+    }
+    return ret;
+}
+
 static void rme_populate_realm(gpointer data, gpointer user_data)
 {
     int ret;
@@ -128,6 +165,7 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state)
 int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
 {
     int ret;
+    int cfg;
     static Error *rme_mig_blocker;
     RmeGuest *guest = cgs_to_rme(cgs);
 
@@ -146,6 +184,13 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
         return -ENODEV;
     }
 
+    for (cfg = 0; cfg < RME_MAX_CFG; cfg++) {
+        ret = rme_configure_one(guest, cfg, errp);
+        if (ret) {
+            return ret;
+        }
+    }
+
     ret = rme_create_rd(guest, errp);
     if (ret) {
         return ret;
@@ -215,8 +260,34 @@ int kvm_arm_rme_vm_type(MachineState *ms)
     return 0;
 }
 
+static char *rme_get_measurement_algo(Object *obj, Error **errp)
+{
+    RmeGuest *guest = RME_GUEST(obj);
+
+    return g_strdup(guest->measurement_algo);
+}
+
+static void rme_set_measurement_algo(Object *obj, const char *value,
+                                     Error **errp)
+{
+    RmeGuest *guest = RME_GUEST(obj);
+
+    if (strncmp(value, "sha256", 6) &&
+        strncmp(value, "sha512", 6)) {
+        error_setg(errp, "invalid Realm measurement algorithm '%s'", value);
+        return;
+    }
+    g_free(guest->measurement_algo);
+    guest->measurement_algo = g_strdup(value);
+}
+
 static void rme_guest_class_init(ObjectClass *oc, void *data)
 {
+    object_class_property_add_str(oc, "measurement-algo",
+                                  rme_get_measurement_algo,
+                                  rme_set_measurement_algo);
+    object_class_property_set_description(oc, "measurement-algo",
+            "Realm measurement algorithm ('sha256', 'sha512')");
 }
 
 static const TypeInfo rme_guest_info = {
-- 
2.39.0

[RFC PATCH 11/16] target/arm/kvm-rme: Add Realm Personalization Value parameter

[Jean-Philippe Brucker]

The Realm Personalization Value (RPV) is provided by the user to
distinguish Realms that have the same initial measurement.

The user provides a 512-bit hexadecimal number.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 qapi/qom.json        |  5 ++-
 target/arm/kvm-rme.c | 72 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 87fe7c31fe..a012281628 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -862,10 +862,13 @@
 #
 # @measurement-algo: Realm measurement algorithm (default: RMM default)
 #
+# @personalization-value: Realm personalization value (default: 0)
+#
 # Since: FIXME
 ##
 { 'struct': 'RmeGuestProperties',
-  'data': { '*measurement-algo': 'str' } }
+  'data': { '*measurement-algo': 'str',
+            '*personalization-value': 'str' } }
 
 ##
 # @ObjectType:
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 3929b941ae..e974c27e5c 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -22,13 +22,14 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
 
 #define RME_PAGE_SIZE qemu_real_host_page_size()
 
-#define RME_MAX_CFG         1
+#define RME_MAX_CFG         2
 
 typedef struct RmeGuest RmeGuest;
 
 struct RmeGuest {
     ConfidentialGuestSupport parent_obj;
     char *measurement_algo;
+    char *personalization_value;
 };
 
 struct RmeImage {
@@ -65,6 +66,45 @@ static int rme_create_rd(RmeGuest *guest, Error **errp)
     return ret;
 }
 
+static int rme_parse_rpv(uint8_t *out, const char *in, Error **errp)
+{
+    int ret;
+    size_t in_len = strlen(in);
+
+    /* Two chars per byte */
+    if (in_len > KVM_CAP_ARM_RME_RPV_SIZE * 2) {
+        error_setg(errp, "Realm Personalization Value is too large");
+        return -E2BIG;
+    }
+
+    /*
+     * Parse as big-endian hexadecimal number (most significant byte on the
+     * left), store little-endian, zero-padded on the right.
+     */
+    while (in_len) {
+        /*
+         * Do the lower nibble first to catch invalid inputs such as '2z', and
+         * to handle the last char.
+         */
+        in_len--;
+        ret = sscanf(in + in_len, "%1hhx", out);
+        if (ret != 1) {
+            error_setg(errp, "Invalid Realm Personalization Value");
+            return -EINVAL;
+        }
+        if (!in_len) {
+            break;
+        }
+        in_len--;
+        ret = sscanf(in + in_len, "%2hhx", out++);
+        if (ret != 1) {
+            error_setg(errp, "Invalid Realm Personalization Value");
+            return -EINVAL;
+        }
+    }
+    return 0;
+}
+
 static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp)
 {
     int ret;
@@ -87,6 +127,16 @@ static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp)
         }
         cfg_str = "hash algorithm";
         break;
+    case KVM_CAP_ARM_RME_CFG_RPV:
+        if (!guest->personalization_value) {
+            return 0;
+        }
+        ret = rme_parse_rpv(args.rpv, guest->personalization_value, errp);
+        if (ret) {
+            return ret;
+        }
+        cfg_str = "personalization value";
+        break;
     default:
         g_assert_not_reached();
     }
@@ -281,6 +331,21 @@ static void rme_set_measurement_algo(Object *obj, const char *value,
     guest->measurement_algo = g_strdup(value);
 }
 
+static char *rme_get_rpv(Object *obj, Error **errp)
+{
+    RmeGuest *guest = RME_GUEST(obj);
+
+    return g_strdup(guest->personalization_value);
+}
+
+static void rme_set_rpv(Object *obj, const char *value, Error **errp)
+{
+    RmeGuest *guest = RME_GUEST(obj);
+
+    g_free(guest->personalization_value);
+    guest->personalization_value = g_strdup(value);
+}
+
 static void rme_guest_class_init(ObjectClass *oc, void *data)
 {
     object_class_property_add_str(oc, "measurement-algo",
@@ -288,6 +353,11 @@ static void rme_guest_class_init(ObjectClass *oc, void *data)
                                   rme_set_measurement_algo);
     object_class_property_set_description(oc, "measurement-algo",
             "Realm measurement algorithm ('sha256', 'sha512')");
+
+    object_class_property_add_str(oc, "personalization-value", rme_get_rpv,
+                                  rme_set_rpv);
+    object_class_property_set_description(oc, "personalization-value",
+            "Realm personalization value (512-bit hexadecimal number)");
 }
 
 static const TypeInfo rme_guest_info = {
-- 
2.39.0

[RFC PATCH 12/16] target/arm/kvm-rme: Add Realm SVE vector length

[Jean-Philippe Brucker]

The Realm configuration takes a SVE enable and vector length parameter.

We cannot reuse the -cpu SVE parameters for this because that
information is needed at Realm Descriptor creation which must happen
before VCPU creation.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 qapi/qom.json        |  5 +++-
 target/arm/kvm-rme.c | 68 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index a012281628..94ecb87f6f 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -864,11 +864,14 @@
 #
 # @personalization-value: Realm personalization value (default: 0)
 #
+# @sve-vector-length: SVE vector length (default: 0, SVE disabled)
+#
 # Since: FIXME
 ##
 { 'struct': 'RmeGuestProperties',
   'data': { '*measurement-algo': 'str',
-            '*personalization-value': 'str' } }
+            '*personalization-value': 'str',
+            '*sve-vector-length': 'uint32' } }
 
 ##
 # @ObjectType:
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index e974c27e5c..0b2153a45c 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -22,7 +22,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
 
 #define RME_PAGE_SIZE qemu_real_host_page_size()
 
-#define RME_MAX_CFG         2
+#define RME_MAX_CFG         3
 
 typedef struct RmeGuest RmeGuest;
 
@@ -30,6 +30,7 @@ struct RmeGuest {
     ConfidentialGuestSupport parent_obj;
     char *measurement_algo;
     char *personalization_value;
+    uint32_t sve_vl;
 };
 
 struct RmeImage {
@@ -137,6 +138,13 @@ static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp)
         }
         cfg_str = "personalization value";
         break;
+    case KVM_CAP_ARM_RME_CFG_SVE:
+        if (!guest->sve_vl) {
+            return 0;
+        }
+        args.sve_vq = guest->sve_vl / 128;
+        cfg_str = "SVE";
+        break;
     default:
         g_assert_not_reached();
     }
@@ -346,6 +354,52 @@ static void rme_set_rpv(Object *obj, const char *value, Error **errp)
     guest->personalization_value = g_strdup(value);
 }
 
+static void rme_get_uint32(Object *obj, Visitor *v, const char *name,
+                           void *opaque, Error **errp)
+{
+    RmeGuest *guest = RME_GUEST(obj);
+    uint32_t value;
+
+    if (strcmp(name, "sve-vector-length") == 0) {
+        value = guest->sve_vl;
+    } else {
+        g_assert_not_reached();
+    }
+
+    visit_type_uint32(v, name, &value, errp);
+}
+
+static void rme_set_uint32(Object *obj, Visitor *v, const char *name,
+                           void *opaque, Error **errp)
+{
+    RmeGuest *guest = RME_GUEST(obj);
+    uint32_t max_value;
+    uint32_t value;
+    uint32_t *var;
+
+    if (!visit_type_uint32(v, name, &value, errp)) {
+        return;
+    }
+
+    if (strcmp(name, "sve-vector-length") == 0) {
+        max_value = ARM_MAX_VQ * 128;
+        var = &guest->sve_vl;
+        if (value & 0x7f) {
+            error_setg(errp, "invalid SVE vector length %"PRIu32, value);
+            return;
+        }
+    } else {
+        g_assert_not_reached();
+    }
+
+    if (value >= max_value) {
+        error_setg(errp, "invalid %s length %"PRIu32, name, value);
+        return;
+    }
+
+    *var = value;
+}
+
 static void rme_guest_class_init(ObjectClass *oc, void *data)
 {
     object_class_property_add_str(oc, "measurement-algo",
@@ -358,6 +412,18 @@ static void rme_guest_class_init(ObjectClass *oc, void *data)
                                   rme_set_rpv);
     object_class_property_set_description(oc, "personalization-value",
             "Realm personalization value (512-bit hexadecimal number)");
+
+    /*
+     * This is not ideal. Normally SVE parameters are given to -cpu, but the
+     * realm parameters are needed much earlier than CPU initialization. We also
+     * don't have a way to discover what is supported at the moment, the idea is
+     * that the user knows exactly what hardware it is running on because these
+     * parameters are part of the measurement and play in the attestation.
+     */
+    object_class_property_add(oc, "sve-vector-length", "uint32", rme_get_uint32,
+                              rme_set_uint32, NULL, NULL);
+    object_class_property_set_description(oc, "sve-vector-length",
+            "SVE vector length. 0 disables SVE (the default)");
 }
 
 static const TypeInfo rme_guest_info = {
-- 
2.39.0

[RFC PATCH 13/16] target/arm/kvm-rme: Add breakpoints and watchpoints parameters

[Jean-Philippe Brucker]

Pass the num_bps and num_wps parameters to Realm creation. These
parameters contribute to the initial Realm measurement.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 qapi/qom.json        |  8 +++++++-
 target/arm/kvm-rme.c | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 94ecb87f6f..86ed386f26 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -866,12 +866,18 @@
 #
 # @sve-vector-length: SVE vector length (default: 0, SVE disabled)
 #
+# @num-breakpoints: Number of breakpoints (default: 0)
+#
+# @num-watchpoints: Number of watchpoints (default: 0)
+#
 # Since: FIXME
 ##
 { 'struct': 'RmeGuestProperties',
   'data': { '*measurement-algo': 'str',
             '*personalization-value': 'str',
-            '*sve-vector-length': 'uint32' } }
+            '*sve-vector-length': 'uint32',
+            '*num-breakpoints': 'uint32',
+            '*num-watchpoints': 'uint32' } }
 
 ##
 # @ObjectType:
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 0b2153a45c..3f39f1f7ad 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -22,7 +22,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
 
 #define RME_PAGE_SIZE qemu_real_host_page_size()
 
-#define RME_MAX_CFG         3
+#define RME_MAX_BPS         0x10
+#define RME_MAX_WPS         0x10
+#define RME_MAX_CFG         4
 
 typedef struct RmeGuest RmeGuest;
 
@@ -31,6 +33,8 @@ struct RmeGuest {
     char *measurement_algo;
     char *personalization_value;
     uint32_t sve_vl;
+    uint32_t num_wps;
+    uint32_t num_bps;
 };
 
 struct RmeImage {
@@ -145,6 +149,14 @@ static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp)
         args.sve_vq = guest->sve_vl / 128;
         cfg_str = "SVE";
         break;
+    case KVM_CAP_ARM_RME_CFG_DBG:
+        if (!guest->num_bps && !guest->num_wps) {
+            return 0;
+        }
+        args.num_brps = guest->num_bps;
+        args.num_wrps = guest->num_wps;
+        cfg_str = "debug parameters";
+        break;
     default:
         g_assert_not_reached();
     }
@@ -362,6 +374,10 @@ static void rme_get_uint32(Object *obj, Visitor *v, const char *name,
 
     if (strcmp(name, "sve-vector-length") == 0) {
         value = guest->sve_vl;
+    } else if (strcmp(name, "num-breakpoints") == 0) {
+        value = guest->num_bps;
+    } else if (strcmp(name, "num-watchpoints") == 0) {
+        value = guest->num_wps;
     } else {
         g_assert_not_reached();
     }
@@ -388,6 +404,12 @@ static void rme_set_uint32(Object *obj, Visitor *v, const char *name,
             error_setg(errp, "invalid SVE vector length %"PRIu32, value);
             return;
         }
+    } else if (strcmp(name, "num-breakpoints") == 0) {
+        max_value = RME_MAX_BPS;
+        var = &guest->num_bps;
+    } else if (strcmp(name, "num-watchpoints") == 0) {
+        max_value = RME_MAX_WPS;
+        var = &guest->num_wps;
     } else {
         g_assert_not_reached();
     }
@@ -424,6 +446,16 @@ static void rme_guest_class_init(ObjectClass *oc, void *data)
                               rme_set_uint32, NULL, NULL);
     object_class_property_set_description(oc, "sve-vector-length",
             "SVE vector length. 0 disables SVE (the default)");
+
+    object_class_property_add(oc, "num-breakpoints", "uint32", rme_get_uint32,
+                              rme_set_uint32, NULL, NULL);
+    object_class_property_set_description(oc, "num-breakpoints",
+            "Number of breakpoints");
+
+    object_class_property_add(oc, "num-watchpoints", "uint32", rme_get_uint32,
+                              rme_set_uint32, NULL, NULL);
+    object_class_property_set_description(oc, "num-watchpoints",
+            "Number of watchpoints");
 }
 
 static const TypeInfo rme_guest_info = {
-- 
2.39.0

[RFC PATCH 14/16] target/arm/kvm-rme: Add PMU num counters parameters

[Jean-Philippe Brucker]

Pass the num_cntrs parameter to Realm creation. These parameters
contribute to the initial Realm measurement.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 qapi/qom.json        |  5 ++++-
 target/arm/kvm-rme.c | 21 ++++++++++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 86ed386f26..13c85abde9 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -870,6 +870,8 @@
 #
 # @num-watchpoints: Number of watchpoints (default: 0)
 #
+# @num-pmu-counters: Number of PMU counters (default: 0, PMU disabled)
+#
 # Since: FIXME
 ##
 { 'struct': 'RmeGuestProperties',
@@ -877,7 +879,8 @@
             '*personalization-value': 'str',
             '*sve-vector-length': 'uint32',
             '*num-breakpoints': 'uint32',
-            '*num-watchpoints': 'uint32' } }
+            '*num-watchpoints': 'uint32',
+            '*num-pmu-counters': 'uint32' } }
 
 ##
 # @ObjectType:
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 3f39f1f7ad..1baed79d46 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -24,7 +24,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
 
 #define RME_MAX_BPS         0x10
 #define RME_MAX_WPS         0x10
-#define RME_MAX_CFG         4
+#define RME_MAX_PMU_CTRS    0x20
+#define RME_MAX_CFG         5
 
 typedef struct RmeGuest RmeGuest;
 
@@ -35,6 +36,7 @@ struct RmeGuest {
     uint32_t sve_vl;
     uint32_t num_wps;
     uint32_t num_bps;
+    uint32_t num_pmu_cntrs;
 };
 
 struct RmeImage {
@@ -157,6 +159,13 @@ static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp)
         args.num_wrps = guest->num_wps;
         cfg_str = "debug parameters";
         break;
+    case KVM_CAP_ARM_RME_CFG_PMU:
+        if (!guest->num_pmu_cntrs) {
+            return 0;
+        }
+        args.num_pmu_cntrs = guest->num_pmu_cntrs;
+        cfg_str = "PMU";
+        break;
     default:
         g_assert_not_reached();
     }
@@ -378,6 +387,8 @@ static void rme_get_uint32(Object *obj, Visitor *v, const char *name,
         value = guest->num_bps;
     } else if (strcmp(name, "num-watchpoints") == 0) {
         value = guest->num_wps;
+    } else if (strcmp(name, "num-pmu-counters") == 0) {
+        value = guest->num_pmu_cntrs;
     } else {
         g_assert_not_reached();
     }
@@ -410,6 +421,9 @@ static void rme_set_uint32(Object *obj, Visitor *v, const char *name,
     } else if (strcmp(name, "num-watchpoints") == 0) {
         max_value = RME_MAX_WPS;
         var = &guest->num_wps;
+    } else if (strcmp(name, "num-pmu-counters") == 0) {
+        max_value = RME_MAX_PMU_CTRS;
+        var = &guest->num_pmu_cntrs;
     } else {
         g_assert_not_reached();
     }
@@ -456,6 +470,11 @@ static void rme_guest_class_init(ObjectClass *oc, void *data)
                               rme_set_uint32, NULL, NULL);
     object_class_property_set_description(oc, "num-watchpoints",
             "Number of watchpoints");
+
+    object_class_property_add(oc, "num-pmu-counters", "uint32", rme_get_uint32,
+                              rme_set_uint32, NULL, NULL);
+    object_class_property_set_description(oc, "num-pmu-counters",
+            "Number of PMU counters");
 }
 
 static const TypeInfo rme_guest_info = {
-- 
2.39.0

[RFC PATCH 15/16] target/arm/kvm: Disable Realm reboot

[Jean-Philippe Brucker]

A realm cannot be reset, it must be recreated from scratch. The RMM
specification defines states of a Realm as NEW -> ACTIVE -> SYSTEM_OFF,
after which the Realm can only be destroyed. A PCSI_SYSTEM_RESET call,
which normally reboots the system, puts the Realm in SYSTEM_OFF state.

QEMU does not support recreating a VM. Normally, a reboot request by the
guest causes all devices to reset, which cannot work for a Realm.
Indeed, loading images into Realm memory and changing the PC is only
allowed for a Realm in NEW state. Resetting the images for a Realm in
SYSTEM_OFF state will cause QEMU to crash with a bus error.

Handle reboot requests by the guest more gracefully, by indicating to
runstate.c that the vCPUs of a Realm are not resettable, and that QEMU
should exit.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 target/arm/kvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index d8655d9041..02b5e8009f 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1071,7 +1071,8 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
 
 bool kvm_arch_cpu_check_are_resettable(void)
 {
-    return true;
+    /* A Realm cannot be reset */
+    return !kvm_arm_rme_enabled();
 }
 
 void kvm_arch_accel_class_init(ObjectClass *oc)
-- 
2.39.0

[RFC PATCH 16/16] target/arm/kvm-rme: Disable readonly mappings

[Jean-Philippe Brucker]

KVM does not support creating read-only mappings for realms at the
moment. Add an arch helper to detect whether read-only mappings are
supported.

Device ROM and flash normally use read-only mappings. Device ROM seems
limited to legacy use and does not need to be trusted by the guest, so
trapping reads should be fine. Flash on the other hand, is used for the
firmware and needs to be both executable and measured. It may be
necessary to replace flash with RAM in order to run firmwares like edk2
in realms.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/sysemu/kvm.h | 2 ++
 accel/kvm/kvm-all.c  | 8 +++++++-
 target/arm/kvm-rme.c | 9 +++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index e9a97eda8c..8d467c76c6 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -581,5 +581,7 @@ bool kvm_arch_cpu_check_are_resettable(void);
 
 bool kvm_dirty_ring_enabled(void);
 
+bool kvm_arch_readonly_mem_allowed(KVMState *s);
+
 uint32_t kvm_dirty_ring_size(void);
 #endif
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f99b0becd8..56cdd2e9e9 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2267,6 +2267,11 @@ bool kvm_dirty_ring_enabled(void)
     return kvm_state->kvm_dirty_ring_size ? true : false;
 }
 
+bool __attribute__((weak)) kvm_arch_readonly_mem_allowed(KVMState *s)
+{
+    return true;
+}
+
 static void query_stats_cb(StatsResultList **result, StatsTarget target,
                            strList *names, strList *targets, Error **errp);
 static void query_stats_schemas_cb(StatsSchemaList **result, Error **errp);
@@ -2520,7 +2525,8 @@ static int kvm_init(MachineState *ms)
     }
 
     kvm_readonly_mem_allowed =
-        (kvm_check_extension(s, KVM_CAP_READONLY_MEM) > 0);
+        (kvm_check_extension(s, KVM_CAP_READONLY_MEM) > 0) &&
+        kvm_arch_readonly_mem_allowed(s);
 
     kvm_eventfds_allowed =
         (kvm_check_extension(s, KVM_CAP_IOEVENTFD) > 0);
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 1baed79d46..2812a52aeb 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -62,6 +62,15 @@ bool kvm_arm_rme_enabled(void)
     return !!cgs_to_rme(cgs);
 }
 
+/*
+ * KVM does not support creating read-only stage-2 mappings for realms at the
+ * moment
+ */
+bool kvm_arch_readonly_mem_allowed(KVMState *s)
+{
+    return !kvm_arm_rme_enabled();
+}
+
 static int rme_create_rd(RmeGuest *guest, Error **errp)
 {
     int ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
-- 
2.39.0

Re: [RFC PATCH 02/16] target/arm: Add confidential guest support

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> Add a new RmeGuest object, inheriting from ConfidentialGuestSupport, to
> support the Arm Realm Management Extension (RME). It is instantiated by
> passing on the command-line:
> 
>    -M virt,confidential-guest-support=<id>
>    -object guest-rme,id=<id>[,options...]
> 
> This is only the skeleton. Support will be added in following patches.
> 
> Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org>
> ---
>   docs/system/confidential-guest-support.rst |  1 +
>   qapi/qom.json                              |  3 +-
>   target/arm/kvm-rme.c                       | 48 ++++++++++++++++++++++
>   target/arm/meson.build                     |  7 +++-
>   4 files changed, 57 insertions(+), 2 deletions(-)
>   create mode 100644 target/arm/kvm-rme.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [RFC PATCH 03/16] target/arm/kvm-rme: Initialize realm

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> +static inline int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
> +{
> +    return 0;
> +}
> +
> +static inline int kvm_arm_rme_vm_type(MachineState *ms)
> +{
> +    return 0;
> +}

Should the stubs really return 0, or g_assert_not_reached()?

> +static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
> +{
> +    if (!cgs) {
> +        return NULL;
> +    }
> +    return (RmeGuest *)object_dynamic_cast(OBJECT(cgs), TYPE_RME_GUEST);
> +}

Direct usage of object_dynamic_cast is usually not correct.

Normally one would use DECLARE_INSTANCE_CHECKER to define the entire function.  But if you 
prefer to type-check the input argument as ConfidentialGuestSupport I can see defining 
your own function.  But in that case I think you probably want to use OBJECT_CHECK, which 
asserts that the cast succeeds.

But then I see that cgs_to_rme is, in all instances so far, also used as a predicate.


> +bool kvm_arm_rme_enabled(void)
> +{
> +    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
> +
> +    return !!cgs_to_rme(cgs);
> +}

Ah, hmm.  Well, probably wouldn't want an assert here.

At present I would expect exactly one object class to be present in the 
qemu-system-aarch64 binary that would pass the machine_check_confidential_guest_support 
test done by core code.  But we are hoping to move toward a heterogeneous model where e.g. 
the TYPE_SEV_GUEST object might be discoverable within the same executable.

Therefore, if cgs_to_rme above uses assert, this might use object_dynamic_cast here 
directly.  It seems like we ought to have a boolean test for this kind of thing, but no.


> +int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
> +{
> +    int ret;
> +    static Error *rme_mig_blocker;
> +    RmeGuest *guest = cgs_to_rme(cgs);
> +
> +    if (!guest) {
> +        /* Either no cgs, or another confidential guest type */
> +        return 0;
> +    }
> +
> +    if (!kvm_enabled()) {
> +        error_setg(errp, "KVM required for RME");
> +        return -ENODEV;
> +    }

I think this null check, and the kvm_enabled check, belong in virt.c.
You'll not get the error with the !CONFIG_KVM version of this function above.


r~

Re: [RFC PATCH 04/16] hw/arm/virt: Add support for Arm RME

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> When confidential-guest-support is enabled for the virt machine, call
> the RME init function, and add the RME flag to the VM type.
> 
> * The Realm differentiates non-secure from realm memory using the upper
>    GPA bit. Reserve that bit when creating the memory map, to make sure
>    that device MMIO located in high memory can still fit.
> 
> * pvtime is disabled for the moment. Since the hypervisor has to write
>    into the shared pvtime page before scheduling a vcpu, it seems
>    incompatible with confidential guests.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

This should be 3 patches:

(1) Including the rme type into the kvm type.
(2) Adjusting the pa size,
(3) Checking the steal-time and randomness flags.


> +    /*
> +     * Since the devicetree is included in the initial measurement, it must
> +     * not contain random data.
> +     */
> +    if (virt_machine_is_confidential(vms)) {
> +        vms->dtb_randomness = false;
> +    }

This property is default off, and the only way it can be on is user argument.  This should 
be an error, not a silent disable.

> +    if (virt_machine_is_confidential(vms)) {
> +        /*
> +         * The host cannot write into a confidential guest's memory until the
> +         * guest shares it. Since the host writes the pvtime region before the
> +         * guest gets a chance to set it up, disable pvtime.
> +         */
> +        steal_time = false;
> +    }

This property is default on since 5.2, so falls into a different category.  Since 5.2 it 
is auto-on for 64-bit guests.  Since it's auto-off for 32-bit guests, I don't see a 
problem with it being auto-off for RME guests.

I do wonder if we should change it to an OnOffAuto property, just to catch silly usage.


r~

Re: [RFC PATCH 05/16] target/arm/kvm: Split kvm_arch_get/put_registers

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> The confidential guest support in KVM limits the number of registers
> that we can read and write. Split the get/put_registers function to
> prepare for it.
> 
> Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org>
> ---
>   target/arm/kvm64.c | 30 ++++++++++++++++++++++++++++--
>   1 file changed, 28 insertions(+), 2 deletions(-)

Given the additional checks within write_list_to_cpustate and write_cpustate_to_list, I 
question whether this is the correct split.


r~

Re: [RFC PATCH 06/16] target/arm/kvm-rme: Initialize vCPU

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> The target code calls kvm_arm_vcpu_init() to mark the vCPU as part of a
> realm. RME support does not use the register lists, because the host can
> only set the boot PC and registers x0-x7. The rest is private to the
> Realm and saved/restored by the RMM.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   target/arm/cpu.h     |  3 ++
>   target/arm/kvm_arm.h |  1 +
>   target/arm/helper.c  |  8 ++++++
>   target/arm/kvm-rme.c | 10 +++++++
>   target/arm/kvm.c     | 12 ++++++++
>   target/arm/kvm64.c   | 65 ++++++++++++++++++++++++++++++++++++++++++--
>   6 files changed, 97 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 9aeed3c848..7d8397985f 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -937,6 +937,9 @@ struct ArchCPU {
>       /* KVM steal time */
>       OnOffAuto kvm_steal_time;
>   
> +    /* Realm Management Extension */
> +    bool kvm_rme;
> +
>       /* Uniprocessor system with MP extensions */
>       bool mp_is_up;
>   
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index 00d3df8cac..e4dc7fbb8d 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -373,6 +373,7 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp);
>   int kvm_arm_rme_vm_type(MachineState *ms);
>   
>   bool kvm_arm_rme_enabled(void);
> +int kvm_arm_rme_vcpu_init(CPUState *cs);
>   
>   #else
>   
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index d8c8223ec3..52360ae2ff 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -126,6 +126,10 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }
> +
>       for (i = 0; i < cpu->cpreg_array_len; i++) {
>           uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
>           const ARMCPRegInfo *ri;
> @@ -171,6 +175,10 @@ bool write_list_to_cpustate(ARMCPU *cpu)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }
> +
>       for (i = 0; i < cpu->cpreg_array_len; i++) {
>           uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
>           uint64_t v = cpu->cpreg_values[i];
> diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> index d7cdca1cbf..3833b187f9 100644
> --- a/target/arm/kvm-rme.c
> +++ b/target/arm/kvm-rme.c
> @@ -118,6 +118,16 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
>       return 0;
>   }
>   
> +int kvm_arm_rme_vcpu_init(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +
> +    if (kvm_arm_rme_enabled()) {
> +        cpu->kvm_rme = true;
> +    }
> +    return 0;
> +}
> +
>   int kvm_arm_rme_vm_type(MachineState *ms)
>   {
>       if (cgs_to_rme(ms->cgs)) {
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index f022c644d2..fcddead4fe 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -449,6 +449,10 @@ int kvm_arm_init_cpreg_list(ARMCPU *cpu)
>       int i, ret, arraylen;
>       CPUState *cs = CPU(cpu);
>   
> +    if (cpu->kvm_rme) {
> +        return 0;
> +    }
> +
>       rl.n = 0;
>       ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
>       if (ret != -E2BIG) {
> @@ -521,6 +525,10 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }
> +
>       for (i = 0; i < cpu->cpreg_array_len; i++) {
>           struct kvm_one_reg r;
>           uint64_t regidx = cpu->cpreg_indexes[i];
> @@ -557,6 +565,10 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }

I don't think that simply returning "ok" is best.  We shouldn't be calling this function 
at all with rme enabled.


r~

Re: [RFC PATCH 06/16] target/arm/kvm-rme: Initialize vCPU

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> +static int kvm_arm_rme_get_core_regs(CPUState *cs)
> +{
> +    int i, ret;
> +    struct kvm_one_reg reg;
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    for (i = 0; i < 8; i++) {
> +        reg.id = AARCH64_CORE_REG(regs.regs[i]);
> +        reg.addr = (uintptr_t) &env->xregs[i];
> +        ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
> +    return 0;
> +}

Wow, this is quite the restriction.

I get that this is just enough to seed the guest for boot, and take SMC traps, but I'm 
concerned that we can't do much with the machine underneath, when it comes to other things 
like "info registers" or gdbstub will be silently unusable.  I would prefer if we can 
somehow make this loudly unusable.

Pardon if I'm jumping the gun and you handle this later.


r~

Re: [RFC PATCH 07/16] target/arm/kvm: Select RME VM type for the scratch VM

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> Although the VM type does not affect values probed from the scratch vCPU
> at the moment, it could later. Ensure we specify the right type when
> creating the temporary VM.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> Does the PA size need changing as well?
> ---
>   target/arm/kvm.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index fcddead4fe..d8655d9041 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -70,6 +70,7 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
>   {
>       int ret = 0, kvmfd = -1, vmfd = -1, cpufd = -1;
>       int max_vm_pa_size;
> +    int vm_type;
>   
>       kvmfd = qemu_open_old("/dev/kvm", O_RDWR);
>       if (kvmfd < 0) {
> @@ -79,8 +80,10 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
>       if (max_vm_pa_size < 0) {
>           max_vm_pa_size = 0;
>       }
> +
> +    vm_type = kvm_arm_rme_vm_type(MACHINE(qdev_get_machine()));
>       do {
> -        vmfd = ioctl(kvmfd, KVM_CREATE_VM, max_vm_pa_size);
> +        vmfd = ioctl(kvmfd, KVM_CREATE_VM, vm_type | max_vm_pa_size);

Given the restrictions on register transfer for RME guests, this seems incorrect.  We 
won't be able to probe what we wanted at all.


r~

Re: [RFC PATCH 06/16] target/arm/kvm-rme: Initialize vCPU

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> --- a/target/arm/kvm-rme.c
> +++ b/target/arm/kvm-rme.c
> @@ -118,6 +118,16 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
>       return 0;
>   }
>   
> +int kvm_arm_rme_vcpu_init(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +
> +    if (kvm_arm_rme_enabled()) {
> +        cpu->kvm_rme = true;
> +    }
> +    return 0;
> +}

So... harking back to patch 3, how may times are we going to query the cast of 
machine->cgs to TYPE_RME_GUEST before we save this value?


r~

Re: [RFC PATCH 08/16] target/arm/kvm-rme: Populate the realm with boot images

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> Initialize the GPA space and populate it with boot images (kernel,
> initrd, firmware, etc). Populating has to be done at VM start time,
> because the images are loaded during reset by rom_reset()
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   target/arm/kvm_arm.h |  6 ++++
>   target/arm/kvm-rme.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 85 insertions(+)
> 
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index e4dc7fbb8d..cec6500603 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -371,6 +371,7 @@ int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
>   
>   int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp);
>   int kvm_arm_rme_vm_type(MachineState *ms);
> +void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size, hwaddr dst_size);
>   
>   bool kvm_arm_rme_enabled(void);
>   int kvm_arm_rme_vcpu_init(CPUState *cs);
> @@ -458,6 +459,11 @@ static inline int kvm_arm_rme_vm_type(MachineState *ms)
>   {
>       return 0;
>   }
> +
> +static inline void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size,
> +                                        hwaddr dst_size)
> +{
> +}
>   #endif
>   
>   static inline const char *gic_class_name(void)
> diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> index 3833b187f9..c8c019f78a 100644
> --- a/target/arm/kvm-rme.c
> +++ b/target/arm/kvm-rme.c
> @@ -9,6 +9,7 @@
>   #include "exec/confidential-guest-support.h"
>   #include "hw/boards.h"
>   #include "hw/core/cpu.h"
> +#include "hw/loader.h"
>   #include "kvm_arm.h"
>   #include "migration/blocker.h"
>   #include "qapi/error.h"
> @@ -19,12 +20,22 @@
>   #define TYPE_RME_GUEST "rme-guest"
>   OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
>   
> +#define RME_PAGE_SIZE qemu_real_host_page_size()
> +
>   typedef struct RmeGuest RmeGuest;
>   
>   struct RmeGuest {
>       ConfidentialGuestSupport parent_obj;
>   };
>   
> +struct RmeImage {
> +    hwaddr base;
> +    hwaddr src_size;
> +    hwaddr dst_size;
> +};
> +
> +static GSList *rme_images;
> +
>   static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
>   {
>       if (!cgs) {
> @@ -51,6 +62,38 @@ static int rme_create_rd(RmeGuest *guest, Error **errp)
>       return ret;
>   }
>   
> +static void rme_populate_realm(gpointer data, gpointer user_data)
> +{
> +    int ret;
> +    struct RmeImage *image = data;
> +    struct kvm_cap_arm_rme_init_ipa_args init_args = {
> +        .init_ipa_base = image->base,
> +        .init_ipa_size = image->dst_size,
> +    };
> +    struct kvm_cap_arm_rme_populate_realm_args populate_args = {
> +        .populate_ipa_base = image->base,
> +        .populate_ipa_size = image->src_size,
> +    };
> +
> +    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
> +                            KVM_CAP_ARM_RME_INIT_IPA_REALM,
> +                            (intptr_t)&init_args);
> +    if (ret) {
> +        error_setg_errno(&error_fatal, -ret,
> +                         "RME: failed to initialize GPA range (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
> +                         image->base, image->dst_size);
> +    }

Using error_fatal with error_setg is discouraged.
This should be error_report(), exit().


> +
> +    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
> +                            KVM_CAP_ARM_RME_POPULATE_REALM,
> +                            (intptr_t)&populate_args);
> +    if (ret) {
> +        error_setg_errno(&error_fatal, -ret,
> +                         "RME: failed to populate realm (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
> +                         image->base, image->src_size);
> +    }
> +}
> +
>   static void rme_vm_state_change(void *opaque, bool running, RunState state)
>   {
>       int ret;
> @@ -72,6 +115,9 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state)
>           }
>       }
>   
> +    g_slist_foreach(rme_images, rme_populate_realm, NULL);
> +    g_slist_free_full(g_steal_pointer(&rme_images), g_free);

I suppose this technically works because you clear the list, and thus the hook is called 
only on the first transition to RUNNING.  On all subsequent transitions the list is empty.

I see that i386 sev does this immediately during machine init, alongside the kernel setup. 
  Since kvm_init has already been called, that seems workable, rather than queuing 
anything for later.

But I think ideally this would be handled generically in (approximately) 
kvm_cpu_synchronize_post_init, looping over all blobs.  This would handle any usage of 
'-device loader,...', instead of the 4 specific things you handle in the next patch.


r~

Re: [RFC PATCH 02/16] target/arm: Add confidential guest support

[Philippe Mathieu-Daudé]

On 27/1/23 16:07, Jean-Philippe Brucker wrote:
> Add a new RmeGuest object, inheriting from ConfidentialGuestSupport, to
> support the Arm Realm Management Extension (RME). It is instantiated by
> passing on the command-line:
> 
>    -M virt,confidential-guest-support=<id>
>    -object guest-rme,id=<id>[,options...]
> 
> This is only the skeleton. Support will be added in following patches.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   docs/system/confidential-guest-support.rst |  1 +
>   qapi/qom.json                              |  3 +-
>   target/arm/kvm-rme.c                       | 48 ++++++++++++++++++++++
>   target/arm/meson.build                     |  7 +++-
>   4 files changed, 57 insertions(+), 2 deletions(-)
>   create mode 100644 target/arm/kvm-rme.c

> diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> new file mode 100644
> index 0000000000..22aa3dc712
> --- /dev/null
> +++ b/target/arm/kvm-rme.c
> @@ -0,0 +1,48 @@
> +/*
> + * QEMU Arm RME support
> + *
> + * Copyright Linaro 2022
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "exec/confidential-guest-support.h"
> +#include "hw/boards.h"
> +#include "hw/core/cpu.h"
> +#include "kvm_arm.h"
> +#include "migration/blocker.h"
> +#include "qapi/error.h"
> +#include "qom/object_interfaces.h"
> +#include "sysemu/kvm.h"
> +#include "sysemu/runstate.h"
> +
> +#define TYPE_RME_GUEST "rme-guest"
> +OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
> +
> +typedef struct RmeGuest RmeGuest;

OBJECT_DECLARE_SIMPLE_TYPE() already forward-declares RmeGuest,
otherwise:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [RFC PATCH 10/16] target/arm/kvm-rme: Add measurement algorithm property

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> +    object_class_property_add_str(oc, "measurement-algo",
> +                                  rme_get_measurement_algo,
> +                                  rme_set_measurement_algo);
> +    object_class_property_set_description(oc, "measurement-algo",
> +            "Realm measurement algorithm ('sha256', 'sha512')");

Better as object_property_add_enum, so that you don't duplicate parsing.
C.f. NetFilterDirection.


r~

Re: [RFC PATCH 11/16] target/arm/kvm-rme: Add Realm Personalization Value parameter

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> +static int rme_parse_rpv(uint8_t *out, const char *in, Error **errp)
> +{
> +    int ret;
> +    size_t in_len = strlen(in);
> +
> +    /* Two chars per byte */
> +    if (in_len > KVM_CAP_ARM_RME_RPV_SIZE * 2) {
> +        error_setg(errp, "Realm Personalization Value is too large");
> +        return -E2BIG;
> +    }
> +
> +    /*
> +     * Parse as big-endian hexadecimal number (most significant byte on the
> +     * left), store little-endian, zero-padded on the right.
> +     */
> +    while (in_len) {
> +        /*
> +         * Do the lower nibble first to catch invalid inputs such as '2z', and
> +         * to handle the last char.
> +         */
> +        in_len--;
> +        ret = sscanf(in + in_len, "%1hhx", out);
> +        if (ret != 1) {
> +            error_setg(errp, "Invalid Realm Personalization Value");
> +            return -EINVAL;
> +        }
> +        if (!in_len) {
> +            break;
> +        }
> +        in_len--;
> +        ret = sscanf(in + in_len, "%2hhx", out++);
> +        if (ret != 1) {
> +            error_setg(errp, "Invalid Realm Personalization Value");
> +            return -EINVAL;
> +        }
> +    }

I think this parsing is late, and should be done

> +static void rme_set_rpv(Object *obj, const char *value, Error **errp)
> +{
> +    RmeGuest *guest = RME_GUEST(obj);
> +
> +    g_free(guest->personalization_value);
> +    guest->personalization_value = g_strdup(value);
> +}

here, when the value is set, so that the error is produced at the proper time.


r~

Re: [RFC PATCH 12/16] target/arm/kvm-rme: Add Realm SVE vector length

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> +static void rme_get_uint32(Object *obj, Visitor *v, const char *name,
> +                           void *opaque, Error **errp)
> +{
> +    RmeGuest *guest = RME_GUEST(obj);
> +    uint32_t value;
> +
> +    if (strcmp(name, "sve-vector-length") == 0) {
> +        value = guest->sve_vl;
> +    } else {
> +        g_assert_not_reached();
> +    }
> +
> +    visit_type_uint32(v, name, &value, errp);
> +}
> +
> +static void rme_set_uint32(Object *obj, Visitor *v, const char *name,
> +                           void *opaque, Error **errp)
> +{
> +    RmeGuest *guest = RME_GUEST(obj);
> +    uint32_t max_value;
> +    uint32_t value;
> +    uint32_t *var;
> +
> +    if (!visit_type_uint32(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    if (strcmp(name, "sve-vector-length") == 0) {
> +        max_value = ARM_MAX_VQ * 128;
> +        var = &guest->sve_vl;
> +        if (value & 0x7f) {
> +            error_setg(errp, "invalid SVE vector length %"PRIu32, value);
> +            return;
> +        }
> +    } else {
> +        g_assert_not_reached();
> +    }
> +
> +    if (value >= max_value) {
> +        error_setg(errp, "invalid %s length %"PRIu32, name, value);
> +        return;
> +    }
> +
> +    *var = value;
> +}

I don't think you should try to genericize these functions, comparing the name string. 
Just rename them and let them be specific to SVE.


r~

Re: [RFC PATCH 12/16] target/arm/kvm-rme: Add Realm SVE vector length

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> +static void rme_get_uint32(Object *obj, Visitor *v, const char *name,
> +                           void *opaque, Error **errp)
> +{
> +    RmeGuest *guest = RME_GUEST(obj);
> +    uint32_t value;
> +
> +    if (strcmp(name, "sve-vector-length") == 0) {
> +        value = guest->sve_vl;
> +    } else {
> +        g_assert_not_reached();
> +    }
> +
> +    visit_type_uint32(v, name, &value, errp);
> +}

Something I've used elsewhere just recently (and should probably make generic:

static void rme_get_uint32(...)
{
     intptr_t offset = (intptr_t)opaque;
     uint32_t *ptr = (void *)obj + offset;

     visit_type_uint32(v, name, ptr, errp);
}

     object_class_property_add(oc, "sve-vector-length", "uint32",
                               rme_get_uint32, rme_set_sve, NULL,
                               (void *)(intptr_t)offsetof(RmeGuest, sve_vl));


r~

Re: [RFC PATCH 13/16] target/arm/kvm-rme: Add breakpoints and watchpoints parameters

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> Pass the num_bps and num_wps parameters to Realm creation. These
> parameters contribute to the initial Realm measurement.
> 
> Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org>
> ---
>   qapi/qom.json        |  8 +++++++-
>   target/arm/kvm-rme.c | 34 +++++++++++++++++++++++++++++++++-
>   2 files changed, 40 insertions(+), 2 deletions(-)

Modulo the comments vs sve-vl and use of more specific functions,

Acked-by: Richard Henderson <richard.henderson@linaro.org>

I see that the KVM branch you referenced defines but does not implement this.


r~

Re: [RFC PATCH 14/16] target/arm/kvm-rme: Add PMU num counters parameters

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> Pass the num_cntrs parameter to Realm creation. These parameters
> contribute to the initial Realm measurement.
> 
> Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org>
> ---
>   qapi/qom.json        |  5 ++++-
>   target/arm/kvm-rme.c | 21 ++++++++++++++++++++-
>   2 files changed, 24 insertions(+), 2 deletions(-)

Modulo the comments vs sve-vl and use of more specific functions,

Acked-by: Richard Henderson <richard.henderson@linaro.org>

I see that the KVM branch you referenced defines but does not implement this.


r~

Re: [RFC PATCH 15/16] target/arm/kvm: Disable Realm reboot

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> A realm cannot be reset, it must be recreated from scratch. The RMM
> specification defines states of a Realm as NEW -> ACTIVE -> SYSTEM_OFF,
> after which the Realm can only be destroyed. A PCSI_SYSTEM_RESET call,
> which normally reboots the system, puts the Realm in SYSTEM_OFF state.
> 
> QEMU does not support recreating a VM. Normally, a reboot request by the
> guest causes all devices to reset, which cannot work for a Realm.
> Indeed, loading images into Realm memory and changing the PC is only
> allowed for a Realm in NEW state. Resetting the images for a Realm in
> SYSTEM_OFF state will cause QEMU to crash with a bus error.
> 
> Handle reboot requests by the guest more gracefully, by indicating to
> runstate.c that the vCPUs of a Realm are not resettable, and that QEMU
> should exit.
> 
> Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org>
> ---
>   target/arm/kvm.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [RFC PATCH 16/16] target/arm/kvm-rme: Disable readonly mappings

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> KVM does not support creating read-only mappings for realms at the
> moment. Add an arch helper to detect whether read-only mappings are
> supported.
> 
> Device ROM and flash normally use read-only mappings. Device ROM seems
> limited to legacy use and does not need to be trusted by the guest, so
> trapping reads should be fine. Flash on the other hand, is used for the
> firmware and needs to be both executable and measured. It may be
> necessary to replace flash with RAM in order to run firmwares like edk2
> in realms.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   include/sysemu/kvm.h | 2 ++
>   accel/kvm/kvm-all.c  | 8 +++++++-
>   target/arm/kvm-rme.c | 9 +++++++++
>   3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index e9a97eda8c..8d467c76c6 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -581,5 +581,7 @@ bool kvm_arch_cpu_check_are_resettable(void);
>   
>   bool kvm_dirty_ring_enabled(void);
>   
> +bool kvm_arch_readonly_mem_allowed(KVMState *s);
> +
>   uint32_t kvm_dirty_ring_size(void);
>   #endif
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index f99b0becd8..56cdd2e9e9 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2267,6 +2267,11 @@ bool kvm_dirty_ring_enabled(void)
>       return kvm_state->kvm_dirty_ring_size ? true : false;
>   }
>   
> +bool __attribute__((weak)) kvm_arch_readonly_mem_allowed(KVMState *s)
> +{
> +    return true;
> +}

Not a fan of the weak.  Just populate this like kvm_arch_cpu_check_are_resettable.


r~

Re: [RFC PATCH 03/16] target/arm/kvm-rme: Initialize realm

[Jean-Philippe Brucker]

Hi Richard,

Thanks a lot for the review

On Fri, Jan 27, 2023 at 10:37:12AM -1000, Richard Henderson wrote:
> At present I would expect exactly one object class to be present in the
> qemu-system-aarch64 binary that would pass the
> machine_check_confidential_guest_support test done by core code.  But we are
> hoping to move toward a heterogeneous model where e.g. the TYPE_SEV_GUEST
> object might be discoverable within the same executable.

Yes, I'm not sure SEV can be supported on qemu-system-aarch64, but pKVM could
probably coexist with RME as another type of confidential guest support
(https://lwn.net/ml/linux-arm-kernel/20220519134204.5379-1-will@kernel.org/)

Thanks,
Jean

Re: [RFC PATCH 04/16] hw/arm/virt: Add support for Arm RME

[Jean-Philippe Brucker]

On Fri, Jan 27, 2023 at 11:07:35AM -1000, Richard Henderson wrote:
> > +    /*
> > +     * Since the devicetree is included in the initial measurement, it must
> > +     * not contain random data.
> > +     */
> > +    if (virt_machine_is_confidential(vms)) {
> > +        vms->dtb_randomness = false;
> > +    }
> 
> This property is default off, and the only way it can be on is user
> argument.  This should be an error, not a silent disable.

This one seems to default to true in virt_instance_init(), and I did need
to disable it in order to get deterministic measurements. Maybe I could
throw an error only when the user attempts to explicitly enables it.

> > +    if (virt_machine_is_confidential(vms)) {
> > +        /*
> > +         * The host cannot write into a confidential guest's memory until the
> > +         * guest shares it. Since the host writes the pvtime region before the
> > +         * guest gets a chance to set it up, disable pvtime.
> > +         */
> > +        steal_time = false;
> > +    }
> 
> This property is default on since 5.2, so falls into a different category.
> Since 5.2 it is auto-on for 64-bit guests.  Since it's auto-off for 32-bit
> guests, I don't see a problem with it being auto-off for RME guests.
> 
> I do wonder if we should change it to an OnOffAuto property, just to catch silly usage.

I'll look into that

Thanks,
Jean

Re: [RFC PATCH 06/16] target/arm/kvm-rme: Initialize vCPU

[Jean-Philippe Brucker]

On Fri, Jan 27, 2023 at 12:37:12PM -1000, Richard Henderson wrote:
> On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> > +static int kvm_arm_rme_get_core_regs(CPUState *cs)
> > +{
> > +    int i, ret;
> > +    struct kvm_one_reg reg;
> > +    ARMCPU *cpu = ARM_CPU(cs);
> > +    CPUARMState *env = &cpu->env;
> > +
> > +    for (i = 0; i < 8; i++) {
> > +        reg.id = AARCH64_CORE_REG(regs.regs[i]);
> > +        reg.addr = (uintptr_t) &env->xregs[i];
> > +        ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> > +        if (ret) {
> > +            return ret;
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> 
> Wow, this is quite the restriction.
> 
> I get that this is just enough to seed the guest for boot, and take SMC
> traps, but I'm concerned that we can't do much with the machine underneath,
> when it comes to other things like "info registers" or gdbstub will be
> silently unusable.  I would prefer if we can somehow make this loudly
> unusable.

For "info registers", which currently displays zero values for all regs,
we can instead return an error message in aarch64_cpu_dump_state().

For gdbstub, I suspect we should disable it entirely since it seems
fundamentally incompatible with confidential VMs, but I need to spend more
time on this.

Thanks,
Jean

Re: [RFC PATCH 08/16] target/arm/kvm-rme: Populate the realm with boot images

[Jean-Philippe Brucker]

On Fri, Jan 27, 2023 at 01:54:23PM -1000, Richard Henderson wrote:
> >   static void rme_vm_state_change(void *opaque, bool running, RunState state)
> >   {
> >       int ret;
> > @@ -72,6 +115,9 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state)
> >           }
> >       }
> > +    g_slist_foreach(rme_images, rme_populate_realm, NULL);
> > +    g_slist_free_full(g_steal_pointer(&rme_images), g_free);
> 
> I suppose this technically works because you clear the list, and thus the
> hook is called only on the first transition to RUNNING.  On all subsequent
> transitions the list is empty.
> 
> I see that i386 sev does this immediately during machine init, alongside the
> kernel setup.  Since kvm_init has already been called, that seems workable,
> rather than queuing anything for later.

The problem I faced was that RME_POPULATE_REALM needs to be called after
rom_reset(), which copies all the blobs into guest memory, and that
happens at device reset time, after machine init and
kvm_cpu_synchronize_post_init().

> But I think ideally this would be handled generically in (approximately)
> kvm_cpu_synchronize_post_init, looping over all blobs.  This would handle
> any usage of '-device loader,...', instead of the 4 specific things you
> handle in the next patch.

I'd definitely prefer something generic that hooks into the loader, I'll
look into that. I didn't do it right away because the arm64 Linux kernel
loading is special, requires reserving extra RAM in addition to the blob
(hence the two parameters to kvm_arm_rme_add_blob()). But we could just
have a special case for the extra memory needed by Linux and make the rest
generic.

Thanks,
Jean