Re: [Qemu-arm] [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 (address@hidden) 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 <address@hidden>
>> ---
>>  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 / address@hidden / Manchester, UK
>