diff mbox series

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

Message ID 20230127150727.612594-5-jean-philippe@linaro.org
State New
Headers show
Series arm: Run Arm CCA VMs with KVM | expand

Commit Message

Jean-Philippe Brucker Jan. 27, 2023, 3:07 p.m. UTC
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(-)

Comments

Richard Henderson Jan. 27, 2023, 9:07 p.m. UTC | #1
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~
Jean-Philippe Brucker Feb. 8, 2023, 12:08 p.m. UTC | #2
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
diff mbox series

Patch

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)