Re: [Qemu-devel] [PATCH] kvm: x86: Add support for -machine split-lock-ac

[Paolo Bonzini]

On 05/07/2018 10:45, Hu, Robert wrote:
>> This should be a CPU feature, not a machine feature.  As mentioned in the
>> review of the kernel patch, please work with Robert to use the same
>> infrastructure for both MSR_TEST_CTL and MSR_IA32_ARCH_CAPABILITIES.
>>
>> (Robert, does IceLake have this feature?  If so, we cannot create the CPU 
>> model
>> until everything is in place).
> 
> [Robert Hoo] 
> Hi Paolo,
> This is not an Icelake  feature.

So why is Intel pushing operating system code for half-baked features?
This "MSRs are the new CPUID" attitude must stop.

Paolo

>> BTW, why is the availability of the feature not exposed with a CPUID bit?  It
>> doesn't make much sense.
>>
>> Paolo
>>
>>> Signed-off-by: Jingqi Liu <address@hidden>
>>> ---
>>>  accel/kvm/kvm-all.c       | 10 ++++++++++
>>>  hw/core/machine.c         | 19 +++++++++++++++++++
>>>  include/hw/boards.h       |  1 +
>>>  linux-headers/linux/kvm.h |  1 +
>>>  qemu-options.hx           |  3 +++
>>>  5 files changed, 34 insertions(+)
>>>
>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index
>>> eb7db92..e960a4d 100644
>>> --- a/accel/kvm/kvm-all.c
>>> +++ b/accel/kvm/kvm-all.c
>>> @@ -1699,6 +1699,16 @@ static int kvm_init(MachineState *ms)
>>>
>>>      s->sync_mmu = !!kvm_vm_check_extension(kvm_state,
>>> KVM_CAP_SYNC_MMU);
>>>
>>> +    if (ms->split_lock_ac &&
>>> +        kvm_check_extension(s, KVM_CAP_X86_SPLIT_LOCK_AC)) {
>>> +        ret = kvm_vm_enable_cap(s, KVM_CAP_X86_SPLIT_LOCK_AC, 0);
>>> +        if (ret) {
>>> +            fprintf(stderr, "Could not enable the feature that #AC 
>>> exception"
>>> +                    "will be generated for split lock accesses!\n");
>>> +            goto err;
>>> +        }
>>> +    }
>>> +
>>>      return 0;
>>>
>>>  err:
>>> diff --git a/hw/core/machine.c b/hw/core/machine.c index
>>> 617e5f8..e054119 100644
>>> --- a/hw/core/machine.c
>>> +++ b/hw/core/machine.c
>>> @@ -350,6 +350,22 @@ static void machine_set_memory_encryption(Object
>> *obj, const char *value,
>>>      ms->memory_encryption = g_strdup(value);  }
>>>
>>> +static void machine_set_split_lock_ac(Object *obj, bool value,
>>> +                                      Error **errp) {
>>> +    MachineState *ms = MACHINE(obj);
>>> +
>>> +    ms->split_lock_ac = value;
>>> +}
>>> +
>>> +static bool machine_get_split_lock_ac(Object *obj, Error **errp) {
>>> +    MachineState *ms = MACHINE(obj);
>>> +
>>> +    return ms->split_lock_ac;
>>> +}
>>> +
>>> +
>>>  void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const
>>> char *type)  {
>>>      strList *item = g_new0(strList, 1); @@ -634,6 +650,9 @@ static
>>> void machine_class_init(ObjectClass *oc, void *data)
>>>          &error_abort);
>>>      object_class_property_set_description(oc, "memory-encryption",
>>>          "Set memory encyption object to use", &error_abort);
>>> +    object_class_property_add_bool(oc, "split-lock-ac",
>>> +        machine_get_split_lock_ac, machine_set_split_lock_ac,
>>> +        &error_abort);
>>>  }
>>>
>>>  static void machine_class_base_init(ObjectClass *oc, void *data) diff
>>> --git a/include/hw/boards.h b/include/hw/boards.h index
>>> 79069dd..9c1e2c0 100644
>>> --- a/include/hw/boards.h
>>> +++ b/include/hw/boards.h
>>> @@ -256,6 +256,7 @@ struct MachineState {
>>>      bool suppress_vmdesc;
>>>      bool enforce_config_section;
>>>      bool enable_graphics;
>>> +    bool split_lock_ac;
>>>      char *memory_encryption;
>>>      DeviceMemoryState *device_memory;
>>>
>>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>>> index 98f389a..4bfa879 100644
>>> --- a/linux-headers/linux/kvm.h
>>> +++ b/linux-headers/linux/kvm.h
>>> @@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt {  #define
>>> KVM_CAP_GET_MSR_FEATURES 153  #define KVM_CAP_HYPERV_EVENTFD
>> 154
>>> #define KVM_CAP_HYPERV_TLBFLUSH 155
>>> +#define KVM_CAP_X86_SPLIT_LOCK_AC 156
>>>
>>>  #ifdef KVM_CAP_IRQ_ROUTING
>>>
>>> diff --git a/qemu-options.hx b/qemu-options.hx index 81b1e99..935cefe
>>> 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -44,6 +44,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>>>      "                nvdimm=on|off controls NVDIMM support (default=off)\n"
>>>      "                enforce-config-section=on|off enforce configuration 
>>> section
>> migration (default=off)\n"
>>>      "                s390-squash-mcss=on|off (deprecated) controls support 
>>> for
>> squashing into default css (default=off)\n"
>>> +    "                split-lock-ac=on|off controls support #AC exception 
>>> for split
>> locked accesses (default=off)\n"
>>>      "                address@hidden memory encryption object to use
>> (default=none)\n",
>>>      QEMU_ARCH_ALL)
>>>  STEXI
>>> @@ -113,6 +114,8 @@ NOTE: this parameter is deprecated. Please use
>>> @option{-global}  @address@hidden|off}
>> instead.
>>>  @item address@hidden
>>>  Memory encryption object to use. The default is none.
>>> address@hidden split-lock-ac=on|off
>>> +Enables or disables #AC exception for split locked accesses.
>>>  @end table
>>>  ETEXI
>>>
>>>
>