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

[Paolo Bonzini]

On 04/07/2018 15:12, Jingqi Liu wrote:
> Add the option split-lock-ac to control whether the #AC
> exception is generated for split locked accesses, which
> is introduced for the machine, there is an example to enable it:
> -machine split-lock-ac=on
> It is disabled on default.
> 
> When bit 29 of the MSR (33H) is set, the processor
> causes an #AC exception to be issued instead of suppressing LOCK on
> bus(during split lock access).

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).

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
>  
>