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

[Hu, Robert]

Best Regards,
Robert Hoo

> -----Original Message-----
> From: Paolo Bonzini <address@hidden>
> Sent: Wednesday, July 4, 2018 21:39
> To: Liu, Jingqi <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden; qemu-
> address@hidden; Wang, Wei W <address@hidden>; Hu, Robert
> <address@hidden>
> Subject: Re: [PATCH] kvm: x86: Add support for -machine split-lock-ac
> 
> 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).

[Robert Hoo] 
Hi Paolo,
This is not an Icelake  feature.
> 
> 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
> >
> >