[Qemu-devel] [RFC 0/6] KVM/ARM: Dynamic and larger GPA size

[Qemu-devel] [RFC 0/6] KVM/ARM: Dynamic and larger GPA size

[Eric Auger]

At the moment the guest physical address space is limited to 40b
due to KVM limitations. [1] bumps this limitation and allows to
create a VM with up to 52b GPA address space.

With this series, QEMU creates a virt VM with the max IPA range
reported by the host kernel or 40b by default.

This choice can be overriden by using the -machine kvm-type=<bits>
option with bits within [40, 52]. If <bits> are not supported by
the host, the legacy 40b value is used.

It is an RFC as [1] is not upstream. This is not material for
QEMU 3.0 due to the kernel dependency. Also the EDK2 FW has a
fixed limit at 40b PA. Note at the moment there is no device or
RAM laid out after 1TB (40b) in virt machine model address space.
This will be addressed in a separate series.

Best Regards

Eric

kernel dependency:
[1] [PATCH v2 00/17] kvm: arm64: Dynamic & 52bit IPA support
https://lkml.org/lkml/2018/3/27/437
For testing, use my kernel branch:
https://github.com/eauger/linux/tree/4.16.0-rc3-Suzuki-52b-IPA-v2

This QEMU series can be found at:
https://github.com/eauger/qemu/tree/v2.12.0-ipa-rfc

Tests:
- On Cavium Gigabyte, a 48b VM was created.
- Migration tests were performed between kernel supporting the
  feature and destination kernel not suporting it

Eric Auger (6):
  linux-headers: Partial update for KVM/ARM
    KVM_ARM_GET_MAX_VM_PHYS_SHIFT
  hw/boards: Add a MachineState parameter to kvm_type callback
  kvm: add kvm_get_max_vm_phys_shift
  hw/arm/virt: Add virt-3.0 machine type
  hw/arm/virt: support kvm_type property
  hw/arm/virt: handle max_vm_phys_shift conflicts on migration

 accel/kvm/kvm-all.c           |   9 +++-
 accel/stubs/kvm-stub.c        |   5 +++
 hw/arm/virt.c                 | 102 +++++++++++++++++++++++++++++++++++++++---
 hw/ppc/mac_newworld.c         |   2 +-
 hw/ppc/mac_oldworld.c         |   2 +-
 hw/ppc/spapr.c                |   2 +-
 include/hw/arm/virt.h         |   3 ++
 include/hw/boards.h           |   2 +-
 include/sysemu/kvm.h          |   1 +
 linux-headers/asm-arm/kvm.h   |  15 -------
 linux-headers/asm-arm64/kvm.h |   6 ---
 linux-headers/linux/kvm.h     |   6 +++
 12 files changed, 124 insertions(+), 31 deletions(-)

-- 
2.5.5

[Qemu-devel] [RFC 1/6] linux-headers: Partial update for KVM/ARM KVM_ARM_GET_MAX_VM_PHYS_SHIFT

[Eric Auger]

This is a partial header to get the KVM_ARM_GET_MAX_VM_PHYS_SHIFT
ioctl. This allows to retrieve the IPA address range KVM supports.

This is based on branch:
https://github.com/eauger/linux/tree/4.16.0-rc3-Suzuki-52b-IPA-v2
itself derived from Suzuki's branch (just incremented the IOCTL index)
git://linux-arm.org/linux-skp.git ipa52/v2

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 linux-headers/asm-arm/kvm.h   | 15 ---------------
 linux-headers/asm-arm64/kvm.h |  6 ------
 linux-headers/linux/kvm.h     |  6 ++++++
 3 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
index 670b43c..4392955 100644
--- a/linux-headers/asm-arm/kvm.h
+++ b/linux-headers/asm-arm/kvm.h
@@ -135,15 +135,6 @@ struct kvm_arch_memory_slot {
 #define KVM_REG_ARM_CRM_SHIFT		7
 #define KVM_REG_ARM_32_CRN_MASK		0x0000000000007800
 #define KVM_REG_ARM_32_CRN_SHIFT	11
-/*
- * For KVM currently all guest registers are nonsecure, but we reserve a bit
- * in the encoding to distinguish secure from nonsecure for AArch32 system
- * registers that are banked by security. This is 1 for the secure banked
- * register, and 0 for the nonsecure banked register or if the register is
- * not banked by security.
- */
-#define KVM_REG_ARM_SECURE_MASK	0x0000000010000000
-#define KVM_REG_ARM_SECURE_SHIFT	28
 
 #define ARM_CP15_REG_SHIFT_MASK(x,n) \
 	(((x) << KVM_REG_ARM_ ## n ## _SHIFT) & KVM_REG_ARM_ ## n ## _MASK)
@@ -195,12 +186,6 @@ struct kvm_arch_memory_slot {
 #define KVM_REG_ARM_VFP_FPINST		0x1009
 #define KVM_REG_ARM_VFP_FPINST2		0x100A
 
-/* KVM-as-firmware specific pseudo-registers */
-#define KVM_REG_ARM_FW			(0x0014 << KVM_REG_ARM_COPROC_SHIFT)
-#define KVM_REG_ARM_FW_REG(r)		(KVM_REG_ARM | KVM_REG_SIZE_U64 | \
-					 KVM_REG_ARM_FW | ((r) & 0xffff))
-#define KVM_REG_ARM_PSCI_VERSION	KVM_REG_ARM_FW_REG(0)
-
 /* Device Control API: ARM VGIC */
 #define KVM_DEV_ARM_VGIC_GRP_ADDR	0
 #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1
diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index 17315ab..4e80651 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -206,12 +206,6 @@ struct kvm_arch_memory_slot {
 #define KVM_REG_ARM_TIMER_CNT		ARM64_SYS_REG(3, 3, 14, 3, 2)
 #define KVM_REG_ARM_TIMER_CVAL		ARM64_SYS_REG(3, 3, 14, 0, 2)
 
-/* KVM-as-firmware specific pseudo-registers */
-#define KVM_REG_ARM_FW			(0x0014 << KVM_REG_ARM_COPROC_SHIFT)
-#define KVM_REG_ARM_FW_REG(r)		(KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
-					 KVM_REG_ARM_FW | ((r) & 0xffff))
-#define KVM_REG_ARM_PSCI_VERSION	KVM_REG_ARM_FW_REG(0)
-
 /* Device Control API: ARM VGIC */
 #define KVM_DEV_ARM_VGIC_GRP_ADDR	0
 #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index cdb148e..a6c4e24 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -774,6 +774,12 @@ struct kvm_ppc_resize_hpt {
 #define KVM_GET_EMULATED_CPUID	  _IOWR(KVMIO, 0x09, struct kvm_cpuid2)
 #define KVM_GET_MSR_FEATURE_INDEX_LIST    _IOWR(KVMIO, 0x0a, struct kvm_msr_list)
 
+ /*
+ * Get the maximum physical address size supported by the host.
+ * Returns log2(Max-Physical-Address-Size)
+ */
+#define KVM_ARM_GET_MAX_VM_PHYS_SHIFT	_IO(KVMIO, 0x0b)
+
 /*
  * Extension capability list.
  */
-- 
2.5.5

[Qemu-devel] [RFC 2/6] hw/boards: Add a MachineState parameter to kvm_type callback

[Eric Auger]

On ARM, the kvm_type will be resolved by querying the KVMState.
Let's add the MachineState handle to the callback so that we
can retrieve the  KVMState handle. in kvm_init, when the callback
is called, the kvm_state variable is not yet set.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 accel/kvm/kvm-all.c   | 2 +-
 hw/ppc/mac_newworld.c | 2 +-
 hw/ppc/mac_oldworld.c | 2 +-
 hw/ppc/spapr.c        | 2 +-
 include/hw/boards.h   | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index ffee68e..0590986 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1550,7 +1550,7 @@ static int kvm_init(MachineState *ms)
 
     kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type");
     if (mc->kvm_type) {
-        type = mc->kvm_type(kvm_type);
+        type = mc->kvm_type(ms, kvm_type);
     } else if (kvm_type) {
         ret = -EINVAL;
         fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type);
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 744acdf..1409d9e 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -492,7 +492,7 @@ static void ppc_core99_init(MachineState *machine)
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
-static int core99_kvm_type(const char *arg)
+static int core99_kvm_type(MachineState *ms, const char *arg)
 {
     /* Always force PR KVM */
     return 2;
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 4608bab..1211fcd 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -363,7 +363,7 @@ static void ppc_heathrow_init(MachineState *machine)
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
-static int heathrow_kvm_type(const char *arg)
+static int heathrow_kvm_type(MachineState *ms, const char *arg)
 {
     /* Always force PR KVM */
     return 2;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f59999d..faf078e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2834,7 +2834,7 @@ static void spapr_machine_init(MachineState *machine)
     }
 }
 
-static int spapr_kvm_type(const char *vm_type)
+static int spapr_kvm_type(MachineState *ms, const char *vm_type)
 {
     if (!vm_type) {
         return 0;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index ef7457f..78f90a1 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -170,7 +170,7 @@ struct MachineClass {
     void (*init)(MachineState *state);
     void (*reset)(void);
     void (*hot_add_cpu)(const int64_t id, Error **errp);
-    int (*kvm_type)(const char *arg);
+    int (*kvm_type)(MachineState *ms, const char *arg);
 
     BlockInterfaceType block_default_type;
     int units_per_default_bus;
-- 
2.5.5

[Qemu-devel] [RFC 3/6] kvm: add kvm_get_max_vm_phys_shift

[Eric Auger]

Add the kvm_get_max_vm_phys_shift() helper that returns the
log of the maximum IPA size supported by KVM. This capability
needs to be known to create the VM with a correct IPA max size
(kvm_type passed along KVM_CREATE_VM ioctl.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 accel/kvm/kvm-all.c    | 7 +++++++
 accel/stubs/kvm-stub.c | 5 +++++
 include/sysemu/kvm.h   | 1 +
 3 files changed, 13 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 0590986..137c38e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -280,6 +280,13 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot)
     return ret;
 }
 
+int kvm_get_max_vm_phys_shift(MachineState *ms)
+{
+    KVMState *s = KVM_STATE(ms->accelerator);
+
+    return kvm_ioctl(s, KVM_ARM_GET_MAX_VM_PHYS_SHIFT, 0);
+}
+
 int kvm_destroy_vcpu(CPUState *cpu)
 {
     KVMState *s = kvm_state;
diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
index 02d5170..7575ba7 100644
--- a/accel/stubs/kvm-stub.c
+++ b/accel/stubs/kvm-stub.c
@@ -161,6 +161,11 @@ bool kvm_has_free_slot(MachineState *ms)
     return false;
 }
 
+int kvm_get_max_vm_phys_shift(MachineState *ms)
+{
+    return 0;
+}
+
 void kvm_init_cpu_signals(CPUState *cpu)
 {
     abort();
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0b64b8e..240e3d9 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -206,6 +206,7 @@ extern KVMState *kvm_state;
 /* external API */
 
 bool kvm_has_free_slot(MachineState *ms);
+int kvm_get_max_vm_phys_shift(MachineState *ms);
 bool kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
 int kvm_has_robust_singlestep(void);
-- 
2.5.5

[Qemu-devel] [RFC 4/6] hw/arm/virt: Add virt-3.0 machine type

[Eric Auger]

Add virt-3.0 machine type.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

Due to the kernel dependency, virt 3.1 will be targetted instead.
---
 hw/arm/virt.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f0a4fa0..dd92ab9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1692,10 +1692,7 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
-#define VIRT_COMPAT_2_12 \
-    HW_COMPAT_2_12
-
-static void virt_2_12_instance_init(Object *obj)
+static void virt_3_0_instance_init(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
@@ -1763,11 +1760,25 @@ static void virt_2_12_instance_init(Object *obj)
     vms->irqmap = a15irqmap;
 }
 
+static void virt_machine_3_0_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(3, 0)
+
+#define VIRT_COMPAT_2_12 \
+    HW_COMPAT_2_12
+
+static void virt_2_12_instance_init(Object *obj)
+{
+    virt_3_0_instance_init(obj);
+}
+
 static void virt_machine_2_12_options(MachineClass *mc)
 {
+    virt_machine_3_0_options(mc);
     SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_12);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(2, 12)
+DEFINE_VIRT_MACHINE(2, 12)
 
 #define VIRT_COMPAT_2_11 \
     HW_COMPAT_2_11
-- 
2.5.5

[Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Eric Auger]

The kvm-type property currently is used to pass
a user parameter to KVM_CREATE_VM. This matches
the way KVM/ARM expects to pass the max_vm_phys_shift
parameter.

This patch adds the support or the kvm-type property in
machvirt and also implements the machine class kvm_type()
callback so that it either returns the kvm-type value
provided by the user or returns the max_vm_phys_shift
exposed by KVM.

for instance, the usespace can use the following option to
instantiate a 42b IPA guest: -machine kvm-type=42

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/virt.c         | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |  1 +
 2 files changed, 45 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index dd92ab9..1700556 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1585,6 +1585,21 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
     }
 }
 
+static char *virt_get_kvm_type(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return g_strdup(vms->kvm_type);
+}
+
+static void virt_set_kvm_type(Object *obj, const char *value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    g_free(vms->kvm_type);
+    vms->kvm_type = g_strdup(value);
+}
+
 static CpuInstanceProperties
 virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
 {
@@ -1646,6 +1661,31 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
     return NULL;
 }
 
+static int virt_kvm_type(MachineState *ms, const char *type_str)
+{
+    int max_vm_phys_shift, ret = 0;
+    uint64_t type;
+
+    if (!type_str) {
+        max_vm_phys_shift = kvm_get_max_vm_phys_shift(ms);
+        if (max_vm_phys_shift < 0) {
+            goto out;
+        }
+    } else {
+        type = g_ascii_strtoll(type_str, NULL, 0);
+        type &= 0xFF;
+        max_vm_phys_shift = (int)type;
+        if (max_vm_phys_shift < 40 || max_vm_phys_shift > 52) {
+            warn_report("valid kvm-type type values are within [40, 52]:"
+                        " option is ignored and VM is created with 40b IPA");
+            goto out;
+        }
+    }
+    ret = max_vm_phys_shift;
+out:
+    return ret;
+}
+
 static void virt_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -1668,6 +1708,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
     mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
+    mc->kvm_type = virt_kvm_type;
     assert(!mc->get_hotplug_handler);
     mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
     hc->plug = virt_machine_device_plug_cb;
@@ -1756,6 +1797,9 @@ static void virt_3_0_instance_init(Object *obj)
                                     "Valid values are none and smmuv3",
                                     NULL);
 
+    object_property_add_str(obj, "kvm-type",
+                            virt_get_kvm_type, virt_set_kvm_type, NULL);
+
     vms->memmap = a15memmap;
     vms->irqmap = a15irqmap;
 }
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 4ac7ef6..2674ce7 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -118,6 +118,7 @@ typedef struct {
     uint32_t msi_phandle;
     uint32_t iommu_phandle;
     int psci_conduit;
+    char *kvm_type;
 } VirtMachineState;
 
 #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
-- 
2.5.5

[Qemu-devel] [RFC 6/6] hw/arm/virt: handle max_vm_phys_shift conflicts on migration

[Eric Auger]

When migrating a VM, we must make sure the destination host
supports as many IPA bits as the source. Otherwise the migration
must fail.

We add a VMState infrastructure to machvirt. On pre_save(),
the current source max_vm_phys_shift is saved.

On destination, we cannot use this information when creating the
VM. The VM is created using the max value reported by the
destination host - or the kvm_type inherited value -. However on
post_load() we can check that this value is compatible with the
source saved value.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/virt.c         | 37 +++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |  2 ++
 2 files changed, 39 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1700556..667f754 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1258,6 +1258,40 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
     return arm_cpu_mp_affinity(idx, clustersz);
 }
 
+static int virt_post_load(void *opaque, int version_id)
+{
+    VirtMachineState *vms = (VirtMachineState *)opaque;
+
+    if (vms->max_vm_phys_shift < vms->source_max_vm_phys_shift) {
+        error_report("This host kernel only supports %d IPA bits whereas "
+                     "the guest requires %d GPA bits", vms->max_vm_phys_shift,
+                     vms->source_max_vm_phys_shift);
+        return -1;
+    }
+    return 0;
+}
+
+static int virt_pre_save(void *opaque)
+{
+    VirtMachineState *vms = (VirtMachineState *)opaque;
+
+    vms->source_max_vm_phys_shift = vms->max_vm_phys_shift;
+    return 0;
+}
+
+static const VMStateDescription vmstate_virt = {
+    .name = "virt",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = virt_post_load,
+    .pre_save = virt_pre_save,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(source_max_vm_phys_shift, VirtMachineState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+
 static void machvirt_init(MachineState *machine)
 {
     VirtMachineState *vms = VIRT_MACHINE(machine);
@@ -1473,6 +1507,7 @@ static void machvirt_init(MachineState *machine)
 
     vms->machine_done.notify = virt_machine_done;
     qemu_add_machine_init_done_notifier(&vms->machine_done);
+    vmstate_register(NULL, 0, &vmstate_virt, vms);
 }
 
 static bool virt_get_secure(Object *obj, Error **errp)
@@ -1663,6 +1698,7 @@ 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 max_vm_phys_shift, ret = 0;
     uint64_t type;
 
@@ -1683,6 +1719,7 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
     }
     ret = max_vm_phys_shift;
 out:
+    vms->max_vm_phys_shift = (max_vm_phys_shift > 0) ? ret : 40;
     return ret;
 }
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 2674ce7..600d96d 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -119,6 +119,8 @@ typedef struct {
     uint32_t iommu_phandle;
     int psci_conduit;
     char *kvm_type;
+    int32_t max_vm_phys_shift;
+    int32_t source_max_vm_phys_shift;
 } VirtMachineState;
 
 #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
-- 
2.5.5

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Dr. David Alan Gilbert]

* Eric Auger (eric.auger@redhat.com) wrote:
> The kvm-type property currently is used to pass
> a user parameter to KVM_CREATE_VM. This matches
> the way KVM/ARM expects to pass the max_vm_phys_shift
> parameter.
> 
> This patch adds the support or the kvm-type property in
> machvirt and also implements the machine class kvm_type()
> callback so that it either returns the kvm-type value
> provided by the user or returns the max_vm_phys_shift
> exposed by KVM.
> 
> for instance, the usespace can use the following option to
> instantiate a 42b IPA guest: -machine kvm-type=42

Without saying if this is better or worse, it is different from x86,
where we have the number of physical address bits as a -cpu parameter
rather than a machine parameter, e.g.

   qemu-system-x86_64 -M pc,accel=kvm -cpu SandyBridge,phys-bits=36
or
   qemu-system-x86_64 -M pc,accel=kvm -cpu SandyBridge,host-phys-bits=true

our machine types can override the default value though.

One other complication (that I don't know if it applies to ARM) is that
TCG only supports phys-bits=40, so we refuse a TCG run with an
explicitly set phys-bits!=40.

Dave

> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/arm/virt.c         | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/virt.h |  1 +
>  2 files changed, 45 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index dd92ab9..1700556 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1585,6 +1585,21 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
>      }
>  }
>  
> +static char *virt_get_kvm_type(Object *obj, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    return g_strdup(vms->kvm_type);
> +}
> +
> +static void virt_set_kvm_type(Object *obj, const char *value, Error **errp)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    g_free(vms->kvm_type);
> +    vms->kvm_type = g_strdup(value);
> +}
> +
>  static CpuInstanceProperties
>  virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
>  {
> @@ -1646,6 +1661,31 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>      return NULL;
>  }
>  
> +static int virt_kvm_type(MachineState *ms, const char *type_str)
> +{
> +    int max_vm_phys_shift, ret = 0;
> +    uint64_t type;
> +
> +    if (!type_str) {
> +        max_vm_phys_shift = kvm_get_max_vm_phys_shift(ms);
> +        if (max_vm_phys_shift < 0) {
> +            goto out;
> +        }
> +    } else {
> +        type = g_ascii_strtoll(type_str, NULL, 0);
> +        type &= 0xFF;
> +        max_vm_phys_shift = (int)type;
> +        if (max_vm_phys_shift < 40 || max_vm_phys_shift > 52) {
> +            warn_report("valid kvm-type type values are within [40, 52]:"
> +                        " option is ignored and VM is created with 40b IPA");
> +            goto out;
> +        }
> +    }
> +    ret = max_vm_phys_shift;
> +out:
> +    return ret;
> +}
> +
>  static void virt_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1668,6 +1708,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
>      mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
> +    mc->kvm_type = virt_kvm_type;
>      assert(!mc->get_hotplug_handler);
>      mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
>      hc->plug = virt_machine_device_plug_cb;
> @@ -1756,6 +1797,9 @@ static void virt_3_0_instance_init(Object *obj)
>                                      "Valid values are none and smmuv3",
>                                      NULL);
>  
> +    object_property_add_str(obj, "kvm-type",
> +                            virt_get_kvm_type, virt_set_kvm_type, NULL);
> +
>      vms->memmap = a15memmap;
>      vms->irqmap = a15irqmap;
>  }
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 4ac7ef6..2674ce7 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -118,6 +118,7 @@ typedef struct {
>      uint32_t msi_phandle;
>      uint32_t iommu_phandle;
>      int psci_conduit;
> +    char *kvm_type;
>  } VirtMachineState;
>  
>  #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
> -- 
> 2.5.5
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 2/6] hw/boards: Add a MachineState parameter to kvm_type callback

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3277 bytes --]

On Wed, Jun 20, 2018 at 03:07:29PM +0200, Eric Auger wrote:
1;5202;0c> On ARM, the kvm_type will be resolved by querying the KVMState.
> Let's add the MachineState handle to the callback so that we
> can retrieve the  KVMState handle. in kvm_init, when the callback
> is called, the kvm_state variable is not yet set.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

ppc parts

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  accel/kvm/kvm-all.c   | 2 +-
>  hw/ppc/mac_newworld.c | 2 +-
>  hw/ppc/mac_oldworld.c | 2 +-
>  hw/ppc/spapr.c        | 2 +-
>  include/hw/boards.h   | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index ffee68e..0590986 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1550,7 +1550,7 @@ static int kvm_init(MachineState *ms)
>  
>      kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type");
>      if (mc->kvm_type) {
> -        type = mc->kvm_type(kvm_type);
> +        type = mc->kvm_type(ms, kvm_type);
>      } else if (kvm_type) {
>          ret = -EINVAL;
>          fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type);
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 744acdf..1409d9e 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -492,7 +492,7 @@ static void ppc_core99_init(MachineState *machine)
>      qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>  }
>  
> -static int core99_kvm_type(const char *arg)
> +static int core99_kvm_type(MachineState *ms, const char *arg)
>  {
>      /* Always force PR KVM */
>      return 2;
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 4608bab..1211fcd 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -363,7 +363,7 @@ static void ppc_heathrow_init(MachineState *machine)
>      qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>  }
>  
> -static int heathrow_kvm_type(const char *arg)
> +static int heathrow_kvm_type(MachineState *ms, const char *arg)
>  {
>      /* Always force PR KVM */
>      return 2;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f59999d..faf078e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2834,7 +2834,7 @@ static void spapr_machine_init(MachineState *machine)
>      }
>  }
>  
> -static int spapr_kvm_type(const char *vm_type)
> +static int spapr_kvm_type(MachineState *ms, const char *vm_type)
>  {
>      if (!vm_type) {
>          return 0;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index ef7457f..78f90a1 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -170,7 +170,7 @@ struct MachineClass {
>      void (*init)(MachineState *state);
>      void (*reset)(void);
>      void (*hot_add_cpu)(const int64_t id, Error **errp);
> -    int (*kvm_type)(const char *arg);
> +    int (*kvm_type)(MachineState *ms, const char *arg);
>  
>      BlockInterfaceType block_default_type;
>      int units_per_default_bus;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [RFC 3/6] kvm: add kvm_get_max_vm_phys_shift

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

On Wed, Jun 20, 2018 at 03:07:30PM +0200, Eric Auger wrote:
> Add the kvm_get_max_vm_phys_shift() helper that returns the
> log of the maximum IPA size supported by KVM. This capability
> needs to be known to create the VM with a correct IPA max size
> (kvm_type passed along KVM_CREATE_VM ioctl.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  accel/kvm/kvm-all.c    | 7 +++++++
>  accel/stubs/kvm-stub.c | 5 +++++
>  include/sysemu/kvm.h   | 1 +
>  3 files changed, 13 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 0590986..137c38e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -280,6 +280,13 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot)
>      return ret;
>  }
>  
> +int kvm_get_max_vm_phys_shift(MachineState *ms)
> +{
> +    KVMState *s = KVM_STATE(ms->accelerator);
> +
> +    return kvm_ioctl(s, KVM_ARM_GET_MAX_VM_PHYS_SHIFT, 0);

From the ioctl() name, I'm assuming this is an ARM specific call.

In which case, shouldn't it be in target/arm.kvm.c, not kvm-all.c ?

> +}
> +
>  int kvm_destroy_vcpu(CPUState *cpu)
>  {
>      KVMState *s = kvm_state;
> diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
> index 02d5170..7575ba7 100644
> --- a/accel/stubs/kvm-stub.c
> +++ b/accel/stubs/kvm-stub.c
> @@ -161,6 +161,11 @@ bool kvm_has_free_slot(MachineState *ms)
>      return false;
>  }
>  
> +int kvm_get_max_vm_phys_shift(MachineState *ms)
> +{
> +    return 0;
> +}
> +
>  void kvm_init_cpu_signals(CPUState *cpu)
>  {
>      abort();
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 0b64b8e..240e3d9 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -206,6 +206,7 @@ extern KVMState *kvm_state;
>  /* external API */
>  
>  bool kvm_has_free_slot(MachineState *ms);
> +int kvm_get_max_vm_phys_shift(MachineState *ms);
>  bool kvm_has_sync_mmu(void);
>  int kvm_has_vcpu_events(void);
>  int kvm_has_robust_singlestep(void);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Auger Eric]

Hi David,

On 06/20/2018 07:15 PM, Dr. David Alan Gilbert wrote:
> * Eric Auger (eric.auger@redhat.com) wrote:
>> The kvm-type property currently is used to pass
>> a user parameter to KVM_CREATE_VM. This matches
>> the way KVM/ARM expects to pass the max_vm_phys_shift
>> parameter.
>>
>> This patch adds the support or the kvm-type property in
>> machvirt and also implements the machine class kvm_type()
>> callback so that it either returns the kvm-type value
>> provided by the user or returns the max_vm_phys_shift
>> exposed by KVM.
>>
>> for instance, the usespace can use the following option to
>> instantiate a 42b IPA guest: -machine kvm-type=42
> 
> Without saying if this is better or worse, it is different from x86,
> where we have the number of physical address bits as a -cpu parameter
> rather than a machine parameter, e.g.
> 
>    qemu-system-x86_64 -M pc,accel=kvm -cpu SandyBridge,phys-bits=36
> or
>    qemu-system-x86_64 -M pc,accel=kvm -cpu SandyBridge,host-phys-bits=true
> 
> our machine types can override the default value though.

My understanding is this phys-bits is used to refine the model of the
CPU. For a given CPU, it is implementation defined how much physical and
virtual bits are supported. So it can vary. The guest then can query the
properties of the CPU using CPUID and when setting eax=0x80000008, it
then retrieves the max physical and virtual address space supported by
this CPU.

On ARM there is such capability register, named
ID_AA64MMFR0_EL1 where PARange field indicates the implemented physical
address size. No equivalent for the virtual range.

arm/internals.h implements arm_pamax(ARMCPU *cpu)
It decodes cpu->id_aa64mmfr0. This later is hardcoded for A57, A53, ...
So as far as I understand, we don't have a way to refine this reg
through cmd line, as we have on x86. But if we were to support this
feature, then phys_bits would be cpu->id_aa64mmfr0.parange.

However what we are trying to achieve in this series is different. We
don't want to refine the vCPU itself. We want to query and force KVM to
support a given intermediate physical address (IPA) range. This means
configure KVM to support a given stage 2 MMU configuration. As a
correlate this means supporting the corresponding GPA range. KVM's
capability to support a given IPA range depends on
- physical CPU ID_AA64MMFR0_EL1.PARange
- host kernel config
- host kernel page table limitations

So the kernel API proposed in [1] allows to query the max IPA KVM
support (through a /dev/kvm iotcl) and this series then does a
KVM_CREATE_VM with a type argument that matches the returned value. So
to me this rather looks as a VM attribute, which from a qemu perspective
would turn out to be a machine option. Then whether we should directly
use the kvm_type option is arguable. on spapr kvm_type typically encodes
the HV or PR. I understand this selects different kvm modules. Here we
want to use a kvm supporting up to 48b IPA for instance.

> 
> One other complication (that I don't know if it applies to ARM) is that
> TCG only supports phys-bits=40, so we refuse a TCG run with an
> explicitly set phys-bits!=40.

The new capability to support selectable GPA range only applies to
accelerated mode. I did not check if kvm-type option can be used in TCG
mode ;-)

Thanks

Eric
> 
> Dave
> 
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  hw/arm/virt.c         | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/arm/virt.h |  1 +
>>  2 files changed, 45 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index dd92ab9..1700556 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1585,6 +1585,21 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
>>      }
>>  }
>>  
>> +static char *virt_get_kvm_type(Object *obj, Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    return g_strdup(vms->kvm_type);
>> +}
>> +
>> +static void virt_set_kvm_type(Object *obj, const char *value, Error **errp)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    g_free(vms->kvm_type);
>> +    vms->kvm_type = g_strdup(value);
>> +}
>> +
>>  static CpuInstanceProperties
>>  virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
>>  {
>> @@ -1646,6 +1661,31 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>>      return NULL;
>>  }
>>  
>> +static int virt_kvm_type(MachineState *ms, const char *type_str)
>> +{
>> +    int max_vm_phys_shift, ret = 0;
>> +    uint64_t type;
>> +
>> +    if (!type_str) {
>> +        max_vm_phys_shift = kvm_get_max_vm_phys_shift(ms);
>> +        if (max_vm_phys_shift < 0) {
>> +            goto out;
>> +        }
>> +    } else {
>> +        type = g_ascii_strtoll(type_str, NULL, 0);
>> +        type &= 0xFF;
>> +        max_vm_phys_shift = (int)type;
>> +        if (max_vm_phys_shift < 40 || max_vm_phys_shift > 52) {
>> +            warn_report("valid kvm-type type values are within [40, 52]:"
>> +                        " option is ignored and VM is created with 40b IPA");
>> +            goto out;
>> +        }
>> +    }
>> +    ret = max_vm_phys_shift;
>> +out:
>> +    return ret;
>> +}
>> +
>>  static void virt_machine_class_init(ObjectClass *oc, void *data)
>>  {
>>      MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -1668,6 +1708,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>      mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
>>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
>>      mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
>> +    mc->kvm_type = virt_kvm_type;
>>      assert(!mc->get_hotplug_handler);
>>      mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
>>      hc->plug = virt_machine_device_plug_cb;
>> @@ -1756,6 +1797,9 @@ static void virt_3_0_instance_init(Object *obj)
>>                                      "Valid values are none and smmuv3",
>>                                      NULL);
>>  
>> +    object_property_add_str(obj, "kvm-type",
>> +                            virt_get_kvm_type, virt_set_kvm_type, NULL);
>> +
>>      vms->memmap = a15memmap;
>>      vms->irqmap = a15irqmap;
>>  }
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index 4ac7ef6..2674ce7 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -118,6 +118,7 @@ typedef struct {
>>      uint32_t msi_phandle;
>>      uint32_t iommu_phandle;
>>      int psci_conduit;
>> +    char *kvm_type;
>>  } VirtMachineState;
>>  
>>  #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
>> -- 
>> 2.5.5
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

Re: [Qemu-devel] [RFC 3/6] kvm: add kvm_get_max_vm_phys_shift

[Auger Eric]

Hi David,

On 06/21/2018 02:37 AM, David Gibson wrote:
> On Wed, Jun 20, 2018 at 03:07:30PM +0200, Eric Auger wrote:
>> Add the kvm_get_max_vm_phys_shift() helper that returns the
>> log of the maximum IPA size supported by KVM. This capability
>> needs to be known to create the VM with a correct IPA max size
>> (kvm_type passed along KVM_CREATE_VM ioctl.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  accel/kvm/kvm-all.c    | 7 +++++++
>>  accel/stubs/kvm-stub.c | 5 +++++
>>  include/sysemu/kvm.h   | 1 +
>>  3 files changed, 13 insertions(+)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index 0590986..137c38e 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -280,6 +280,13 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot)
>>      return ret;
>>  }
>>  
>> +int kvm_get_max_vm_phys_shift(MachineState *ms)
>> +{
>> +    KVMState *s = KVM_STATE(ms->accelerator);
>> +
>> +    return kvm_ioctl(s, KVM_ARM_GET_MAX_VM_PHYS_SHIFT, 0);
> 
> From the ioctl() name, I'm assuming this is an ARM specific call.
> 
> In which case, shouldn't it be in target/arm.kvm.c, not kvm-all.c ?
Yes definitively.

Thanks

Eric
> 
>> +}
>> +
>>  int kvm_destroy_vcpu(CPUState *cpu)
>>  {
>>      KVMState *s = kvm_state;
>> diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
>> index 02d5170..7575ba7 100644
>> --- a/accel/stubs/kvm-stub.c
>> +++ b/accel/stubs/kvm-stub.c
>> @@ -161,6 +161,11 @@ bool kvm_has_free_slot(MachineState *ms)
>>      return false;
>>  }
>>  
>> +int kvm_get_max_vm_phys_shift(MachineState *ms)
>> +{
>> +    return 0;
>> +}
>> +
>>  void kvm_init_cpu_signals(CPUState *cpu)
>>  {
>>      abort();
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index 0b64b8e..240e3d9 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -206,6 +206,7 @@ extern KVMState *kvm_state;
>>  /* external API */
>>  
>>  bool kvm_has_free_slot(MachineState *ms);
>> +int kvm_get_max_vm_phys_shift(MachineState *ms);
>>  bool kvm_has_sync_mmu(void);
>>  int kvm_has_vcpu_events(void);
>>  int kvm_has_robust_singlestep(void);
> 

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Dr. David Alan Gilbert]

* Auger Eric (eric.auger@redhat.com) wrote:
> Hi David,
> 
> On 06/20/2018 07:15 PM, Dr. David Alan Gilbert wrote:
> > * Eric Auger (eric.auger@redhat.com) wrote:
> >> The kvm-type property currently is used to pass
> >> a user parameter to KVM_CREATE_VM. This matches
> >> the way KVM/ARM expects to pass the max_vm_phys_shift
> >> parameter.
> >>
> >> This patch adds the support or the kvm-type property in
> >> machvirt and also implements the machine class kvm_type()
> >> callback so that it either returns the kvm-type value
> >> provided by the user or returns the max_vm_phys_shift
> >> exposed by KVM.
> >>
> >> for instance, the usespace can use the following option to
> >> instantiate a 42b IPA guest: -machine kvm-type=42
> > 
> > Without saying if this is better or worse, it is different from x86,
> > where we have the number of physical address bits as a -cpu parameter
> > rather than a machine parameter, e.g.
> > 
> >    qemu-system-x86_64 -M pc,accel=kvm -cpu SandyBridge,phys-bits=36
> > or
> >    qemu-system-x86_64 -M pc,accel=kvm -cpu SandyBridge,host-phys-bits=true
> > 
> > our machine types can override the default value though.
> 
> My understanding is this phys-bits is used to refine the model of the
> CPU. For a given CPU, it is implementation defined how much physical and
> virtual bits are supported. So it can vary. The guest then can query the
> properties of the CPU using CPUID and when setting eax=0x80000008, it
> then retrieves the max physical and virtual address space supported by
> this CPU.
> 
> On ARM there is such capability register, named
> ID_AA64MMFR0_EL1 where PARange field indicates the implemented physical
> address size. No equivalent for the virtual range.
> 
> arm/internals.h implements arm_pamax(ARMCPU *cpu)
> It decodes cpu->id_aa64mmfr0. This later is hardcoded for A57, A53, ...
> So as far as I understand, we don't have a way to refine this reg
> through cmd line, as we have on x86. But if we were to support this
> feature, then phys_bits would be cpu->id_aa64mmfr0.parange.
> 
> However what we are trying to achieve in this series is different. We
> don't want to refine the vCPU itself. We want to query and force KVM to
> support a given intermediate physical address (IPA) range. This means
> configure KVM to support a given stage 2 MMU configuration. As a
> correlate this means supporting the corresponding GPA range. KVM's
> capability to support a given IPA range depends on
> - physical CPU ID_AA64MMFR0_EL1.PARange
> - host kernel config
> - host kernel page table limitations
> 
> So the kernel API proposed in [1] allows to query the max IPA KVM
> support (through a /dev/kvm iotcl) and this series then does a
> KVM_CREATE_VM with a type argument that matches the returned value. So
> to me this rather looks as a VM attribute, which from a qemu perspective
> would turn out to be a machine option. Then whether we should directly
> use the kvm_type option is arguable. on spapr kvm_type typically encodes
> the HV or PR. I understand this selects different kvm modules. Here we
> want to use a kvm supporting up to 48b IPA for instance.

OK, my only worry is that management layers are going to see this
differently for different architectures, where it feels like it's
basically the same thing.

Note on x86, while I don't think there's a host kernel restriction on
phys-bits/GPA, there is the restriction of the host CPU, so if setting
it to anything other than host-phys-bits there does need to be some
probing somewhere.

> > 
> > One other complication (that I don't know if it applies to ARM) is that
> > TCG only supports phys-bits=40, so we refuse a TCG run with an
> > explicitly set phys-bits!=40.
> 
> The new capability to support selectable GPA range only applies to
> accelerated mode. I did not check if kvm-type option can be used in TCG
> mode ;-)

Yes, I think all I did was add an error check to warn/fail if you try
and set it wrongly.

Dave

> Thanks
> 
> Eric
> > 
> > Dave
> > 
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >> ---
> >>  hw/arm/virt.c         | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >>  include/hw/arm/virt.h |  1 +
> >>  2 files changed, 45 insertions(+)
> >>
> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> index dd92ab9..1700556 100644
> >> --- a/hw/arm/virt.c
> >> +++ b/hw/arm/virt.c
> >> @@ -1585,6 +1585,21 @@ static void virt_set_iommu(Object *obj, const char *value, Error **errp)
> >>      }
> >>  }
> >>  
> >> +static char *virt_get_kvm_type(Object *obj, Error **errp)
> >> +{
> >> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> >> +
> >> +    return g_strdup(vms->kvm_type);
> >> +}
> >> +
> >> +static void virt_set_kvm_type(Object *obj, const char *value, Error **errp)
> >> +{
> >> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> >> +
> >> +    g_free(vms->kvm_type);
> >> +    vms->kvm_type = g_strdup(value);
> >> +}
> >> +
> >>  static CpuInstanceProperties
> >>  virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
> >>  {
> >> @@ -1646,6 +1661,31 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
> >>      return NULL;
> >>  }
> >>  
> >> +static int virt_kvm_type(MachineState *ms, const char *type_str)
> >> +{
> >> +    int max_vm_phys_shift, ret = 0;
> >> +    uint64_t type;
> >> +
> >> +    if (!type_str) {
> >> +        max_vm_phys_shift = kvm_get_max_vm_phys_shift(ms);
> >> +        if (max_vm_phys_shift < 0) {
> >> +            goto out;
> >> +        }
> >> +    } else {
> >> +        type = g_ascii_strtoll(type_str, NULL, 0);
> >> +        type &= 0xFF;
> >> +        max_vm_phys_shift = (int)type;
> >> +        if (max_vm_phys_shift < 40 || max_vm_phys_shift > 52) {
> >> +            warn_report("valid kvm-type type values are within [40, 52]:"
> >> +                        " option is ignored and VM is created with 40b IPA");
> >> +            goto out;
> >> +        }
> >> +    }
> >> +    ret = max_vm_phys_shift;
> >> +out:
> >> +    return ret;
> >> +}
> >> +
> >>  static void virt_machine_class_init(ObjectClass *oc, void *data)
> >>  {
> >>      MachineClass *mc = MACHINE_CLASS(oc);
> >> @@ -1668,6 +1708,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> >>      mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
> >>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
> >>      mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
> >> +    mc->kvm_type = virt_kvm_type;
> >>      assert(!mc->get_hotplug_handler);
> >>      mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
> >>      hc->plug = virt_machine_device_plug_cb;
> >> @@ -1756,6 +1797,9 @@ static void virt_3_0_instance_init(Object *obj)
> >>                                      "Valid values are none and smmuv3",
> >>                                      NULL);
> >>  
> >> +    object_property_add_str(obj, "kvm-type",
> >> +                            virt_get_kvm_type, virt_set_kvm_type, NULL);
> >> +
> >>      vms->memmap = a15memmap;
> >>      vms->irqmap = a15irqmap;
> >>  }
> >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> >> index 4ac7ef6..2674ce7 100644
> >> --- a/include/hw/arm/virt.h
> >> +++ b/include/hw/arm/virt.h
> >> @@ -118,6 +118,7 @@ typedef struct {
> >>      uint32_t msi_phandle;
> >>      uint32_t iommu_phandle;
> >>      int psci_conduit;
> >> +    char *kvm_type;
> >>  } VirtMachineState;
> >>  
> >>  #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
> >> -- 
> >> 2.5.5
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Andrew Jones]

On Wed, Jun 20, 2018 at 03:07:32PM +0200, Eric Auger wrote:
> The kvm-type property currently is used to pass
> a user parameter to KVM_CREATE_VM. This matches
> the way KVM/ARM expects to pass the max_vm_phys_shift
> parameter.
> 
> This patch adds the support or the kvm-type property in
> machvirt and also implements the machine class kvm_type()
> callback so that it either returns the kvm-type value
> provided by the user or returns the max_vm_phys_shift
> exposed by KVM.
> 
> for instance, the usespace can use the following option to
> instantiate a 42b IPA guest: -machine kvm-type=42

'kvm-type' is a very generic name. It looks like you're creating a KVM
VM of type 42 (which I assume is the ultimate KVM VM that answers the
meaning to Life, the Universe, and Everything), but it's not obvious
how it relates to physical address bits. Why not call this property
something like 'min_vm_phys_shift'? Notice the 'min' in the name,
because this is where the user is stating what the minimum number of
physical address bits required for the VM is. IIUC, if KVM supports
more, then it shouldn't be a problem.

Thanks,
drew

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Auger Eric]

Hi Drew,

On 07/03/2018 01:55 PM, Andrew Jones wrote:
> On Wed, Jun 20, 2018 at 03:07:32PM +0200, Eric Auger wrote:
>> The kvm-type property currently is used to pass
>> a user parameter to KVM_CREATE_VM. This matches
>> the way KVM/ARM expects to pass the max_vm_phys_shift
>> parameter.
>>
>> This patch adds the support or the kvm-type property in
>> machvirt and also implements the machine class kvm_type()
>> callback so that it either returns the kvm-type value
>> provided by the user or returns the max_vm_phys_shift
>> exposed by KVM.
>>
>> for instance, the usespace can use the following option to
>> instantiate a 42b IPA guest: -machine kvm-type=42
> 
> 'kvm-type' is a very generic name. It looks like you're creating a KVM
> VM of type 42 (which I assume is the ultimate KVM VM that answers the
> meaning to Life, the Universe, and Everything), but it's not obvious
> how it relates to physical address bits. Why not call this property
> something like 'min_vm_phys_shift'? Notice the 'min' in the name,
> because this is where the user is stating what the minimum number of
> physical address bits required for the VM is. IIUC, if KVM supports
> more, then it shouldn't be a problem.

Well I agree with you that using kvm-type=42 is not very nice.

On the other hand the current kernel API to pass the VM GPA address size
is though the KVM_CREATE_VM kvm_type argument.

in accel/kvm/kvm-all.c there is all the infrastructure to fetch the
generic machine kvm-type machine option and decode it into type, which
is passed to KVM_CREATE_VM.

"
    kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type");
    if (mc->kvm_type) {
        type = mc->kvm_type(ms, kvm_type);
    } else if (kvm_type) {
        ret = -EINVAL;
        fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type);
        goto err;
    }

    do {
        ret = kvm_ioctl(s, KVM_CREATE_VM, type);
    } while (ret == -EINTR);
"

This infrastructure already is used in hw/ppc/spapr.c

Whould it be better if we would pass something like kvm-type=48bGPA?
Otherwise I can decode another virt machine option (min_vm_phys_shift)
in kvm_type callback.

Thanks

Eric


> 
> Thanks,
> drew
> 

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Andrew Jones]

On Tue, Jul 03, 2018 at 02:31:02PM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 07/03/2018 01:55 PM, Andrew Jones wrote:
> > On Wed, Jun 20, 2018 at 03:07:32PM +0200, Eric Auger wrote:
> >> The kvm-type property currently is used to pass
> >> a user parameter to KVM_CREATE_VM. This matches
> >> the way KVM/ARM expects to pass the max_vm_phys_shift
> >> parameter.
> >>
> >> This patch adds the support or the kvm-type property in
> >> machvirt and also implements the machine class kvm_type()
> >> callback so that it either returns the kvm-type value
> >> provided by the user or returns the max_vm_phys_shift
> >> exposed by KVM.
> >>
> >> for instance, the usespace can use the following option to
> >> instantiate a 42b IPA guest: -machine kvm-type=42
> > 
> > 'kvm-type' is a very generic name. It looks like you're creating a KVM
> > VM of type 42 (which I assume is the ultimate KVM VM that answers the
> > meaning to Life, the Universe, and Everything), but it's not obvious
> > how it relates to physical address bits. Why not call this property
> > something like 'min_vm_phys_shift'? Notice the 'min' in the name,
> > because this is where the user is stating what the minimum number of
> > physical address bits required for the VM is. IIUC, if KVM supports
> > more, then it shouldn't be a problem.
> 
> Well I agree with you that using kvm-type=42 is not very nice.
> 
> On the other hand the current kernel API to pass the VM GPA address size
> is though the KVM_CREATE_VM kvm_type argument.
> 
> in accel/kvm/kvm-all.c there is all the infrastructure to fetch the
> generic machine kvm-type machine option and decode it into type, which
> is passed to KVM_CREATE_VM.
> 
> "
>     kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type");
>     if (mc->kvm_type) {
>         type = mc->kvm_type(ms, kvm_type);
>     } else if (kvm_type) {
>         ret = -EINVAL;
>         fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type);
>         goto err;
>     }
> 
>     do {
>         ret = kvm_ioctl(s, KVM_CREATE_VM, type);
>     } while (ret == -EINTR);
> "
> 
> This infrastructure already is used in hw/ppc/spapr.c
> 
> Whould it be better if we would pass something like kvm-type=48bGPA?
> Otherwise I can decode another virt machine option (min_vm_phys_shift)
> in kvm_type callback.

Yes, this is what I'm thinking. I don't believe we have to expose the
details of the KVM API to the user through the QEMU command line. The
details are actually more complicated anyway, as the phys-shift is
only the lower 8-bits of KVM type[*], not the whole value.

Thanks,
drew

[*] Looks like Suzuki's series is missing the Documentation/virtual/kvm/api.txt
    update needed to specify that.

> 
> Thanks
> 
> Eric
> 
> 
> > 
> > Thanks,
> > drew
> > 
> 

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Suzuki K Poulose]

On 03/07/18 13:47, Andrew Jones wrote:
>> This infrastructure already is used in hw/ppc/spapr.c
>>
>> Whould it be better if we would pass something like kvm-type=48bGPA?
>> Otherwise I can decode another virt machine option (min_vm_phys_shift)
>> in kvm_type callback.
> 
> Yes, this is what I'm thinking. I don't believe we have to expose the
> details of the KVM API to the user through the QEMU command line. The
> details are actually more complicated anyway, as the phys-shift is
> only the lower 8-bits of KVM type[*], not the whole value.
> 
> Thanks,
> drew
> 
> [*] Looks like Suzuki's series is missing the Documentation/virtual/kvm/api.txt
>      update needed to specify that.

Thanks for spotting, I will update the documentation.

Suzuki

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Thomas Huth]

On 03.07.2018 14:31, Auger Eric wrote:
> Hi Drew,
> 
> On 07/03/2018 01:55 PM, Andrew Jones wrote:
>> On Wed, Jun 20, 2018 at 03:07:32PM +0200, Eric Auger wrote:
>>> The kvm-type property currently is used to pass
>>> a user parameter to KVM_CREATE_VM. This matches
>>> the way KVM/ARM expects to pass the max_vm_phys_shift
>>> parameter.
>>>
>>> This patch adds the support or the kvm-type property in
>>> machvirt and also implements the machine class kvm_type()
>>> callback so that it either returns the kvm-type value
>>> provided by the user or returns the max_vm_phys_shift
>>> exposed by KVM.
>>>
>>> for instance, the usespace can use the following option to
>>> instantiate a 42b IPA guest: -machine kvm-type=42
>>
>> 'kvm-type' is a very generic name. It looks like you're creating a KVM
>> VM of type 42 (which I assume is the ultimate KVM VM that answers the
>> meaning to Life, the Universe, and Everything), but it's not obvious
>> how it relates to physical address bits. Why not call this property
>> something like 'min_vm_phys_shift'? Notice the 'min' in the name,
>> because this is where the user is stating what the minimum number of
>> physical address bits required for the VM is. IIUC, if KVM supports
>> more, then it shouldn't be a problem.
> 
> Well I agree with you that using kvm-type=42 is not very nice.
> 
> On the other hand the current kernel API to pass the VM GPA address size
> is though the KVM_CREATE_VM kvm_type argument.
> 
> in accel/kvm/kvm-all.c there is all the infrastructure to fetch the
> generic machine kvm-type machine option and decode it into type, which
> is passed to KVM_CREATE_VM.
> 
> "
>     kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type");
>     if (mc->kvm_type) {
>         type = mc->kvm_type(ms, kvm_type);
>     } else if (kvm_type) {
>         ret = -EINVAL;
>         fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type);
>         goto err;
>     }
> 
>     do {
>         ret = kvm_ioctl(s, KVM_CREATE_VM, type);
>     } while (ret == -EINTR);
> "
> 
> This infrastructure already is used in hw/ppc/spapr.c

FWIW: The ppc code uses "kvm-type" to select the KVM implementation in
the kernel, since there are two implementations: kvm-pr (which is a
trap-and-emulate implementation) and kvm-hv (which is a
hardware-accelerated implementation). If you now introduce kvm-type for
ARM, too, but with a completely different meaning, I think that could
rather be confusing for the users...?

 Thomas

Re: [Qemu-devel] [RFC 5/6] hw/arm/virt: support kvm_type property

[Auger Eric]

Hi Thomas,

On 07/09/2018 07:26 PM, Thomas Huth wrote:
> On 03.07.2018 14:31, Auger Eric wrote:
>> Hi Drew,
>>
>> On 07/03/2018 01:55 PM, Andrew Jones wrote:
>>> On Wed, Jun 20, 2018 at 03:07:32PM +0200, Eric Auger wrote:
>>>> The kvm-type property currently is used to pass
>>>> a user parameter to KVM_CREATE_VM. This matches
>>>> the way KVM/ARM expects to pass the max_vm_phys_shift
>>>> parameter.
>>>>
>>>> This patch adds the support or the kvm-type property in
>>>> machvirt and also implements the machine class kvm_type()
>>>> callback so that it either returns the kvm-type value
>>>> provided by the user or returns the max_vm_phys_shift
>>>> exposed by KVM.
>>>>
>>>> for instance, the usespace can use the following option to
>>>> instantiate a 42b IPA guest: -machine kvm-type=42
>>>
>>> 'kvm-type' is a very generic name. It looks like you're creating a KVM
>>> VM of type 42 (which I assume is the ultimate KVM VM that answers the
>>> meaning to Life, the Universe, and Everything), but it's not obvious
>>> how it relates to physical address bits. Why not call this property
>>> something like 'min_vm_phys_shift'? Notice the 'min' in the name,
>>> because this is where the user is stating what the minimum number of
>>> physical address bits required for the VM is. IIUC, if KVM supports
>>> more, then it shouldn't be a problem.
>>
>> Well I agree with you that using kvm-type=42 is not very nice.
>>
>> On the other hand the current kernel API to pass the VM GPA address size
>> is though the KVM_CREATE_VM kvm_type argument.
>>
>> in accel/kvm/kvm-all.c there is all the infrastructure to fetch the
>> generic machine kvm-type machine option and decode it into type, which
>> is passed to KVM_CREATE_VM.
>>
>> "
>>     kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type");
>>     if (mc->kvm_type) {
>>         type = mc->kvm_type(ms, kvm_type);
>>     } else if (kvm_type) {
>>         ret = -EINVAL;
>>         fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type);
>>         goto err;
>>     }
>>
>>     do {
>>         ret = kvm_ioctl(s, KVM_CREATE_VM, type);
>>     } while (ret == -EINTR);
>> "
>>
>> This infrastructure already is used in hw/ppc/spapr.c
> 
> FWIW: The ppc code uses "kvm-type" to select the KVM implementation in
> the kernel, since there are two implementations: kvm-pr (which is a
> trap-and-emulate implementation) and kvm-hv (which is a
> hardware-accelerated implementation). If you now introduce kvm-type for
> ARM, too, but with a completely different meaning, I think that could
> rather be confusing for the users...?

Agreed. The kernel user API still is under discussion and from a qemu
perspective I will use either another machine option or a cpu option to
set the intermediate physical address size.

Thanks

Eric
> 
>  Thomas
>