| Message ID | 1530709942-87947-1-git-send-email-jingqi.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <jingqi.liu@intel.com> > --- > 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" > " memory-encryption=@var{} 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} > @option{migration.send-configuration}=@var{on|off} instead. > @item memory-encryption=@var{} > Memory encryption object to use. The default is none. > +@item split-lock-ac=on|off > +Enables or disables #AC exception for split locked accesses. > @end table > ETEXI > >
Hi, This series failed build test on s390x host. Please find the details below. N/A. Internal error while reading log file --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On Wed, Jul 04, 2018 at 03:38:32PM +0200, Paolo Bonzini wrote: > 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). I don't think we need to block the CPU model because QEMU+KVM doesn't support some features yet, as long as kernel versions capable of running the 3.0 version of IceLake-Server will be also capable of running the 3.1 version of IceLake-Server. Now, if that condition won't be true and we have some IceLake features that will added only to more recent kernels, it might be a good idea to hold the inclusion of 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. I have the same question. Without a CPUID bit, guests may block the VM from being migrated to older hosts because the additional section for the MSR will appear.
Best Regards, Robert Hoo > -----Original Message----- > From: Paolo Bonzini <pbonzini@redhat.com> > Sent: Wednesday, July 4, 2018 21:39 > To: Liu, Jingqi <jingqi.liu@intel.com> > Cc: rth@twiddle.net; ehabkost@redhat.com; mtosatti@redhat.com; qemu- > devel@nongnu.org; Wang, Wei W <wei.w.wang@intel.com>; Hu, Robert > <robert.hu@intel.com> > 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 <jingqi.liu@intel.com> > > --- > > 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" > > " memory-encryption=@var{} 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} @option{migration.send-configuration}=@var{on|off} > instead. > > @item memory-encryption=@var{} > > Memory encryption object to use. The default is none. > > +@item split-lock-ac=on|off > > +Enables or disables #AC exception for split locked accesses. > > @end table > > ETEXI > > > >
On 04/07/2018 19:52, Eduardo Habkost wrote: >> (Robert, does IceLake have this feature? If so, we cannot create the >> CPU model until everything is in place). > I don't think we need to block the CPU model because QEMU+KVM > doesn't support some features yet, as long as kernel versions > capable of running the 3.0 version of IceLake-Server will be also > capable of running the 3.1 version of IceLake-Server. > > Now, if that condition won't be true and we have some IceLake > features that will added only to more recent kernels, it might be > a good idea to hold the inclusion of the CPU model until > everything is in place. > > Yeah, the reason for that is to wait for the kernel side of the implementation. Paolo
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 <jingqi.liu@intel.com> >>> --- >>> 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" >>> " memory-encryption=@var{} 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} @option{migration.send-configuration}=@var{on|off} >> instead. >>> @item memory-encryption=@var{} >>> Memory encryption object to use. The default is none. >>> +@item split-lock-ac=on|off >>> +Enables or disables #AC exception for split locked accesses. >>> @end table >>> ETEXI >>> >>> >
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" " memory-encryption=@var{} 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} @option{migration.send-configuration}=@var{on|off} instead. @item memory-encryption=@var{} Memory encryption object to use. The default is none. +@item split-lock-ac=on|off +Enables or disables #AC exception for split locked accesses. @end table ETEXI
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). Signed-off-by: Jingqi Liu <jingqi.liu@intel.com> --- 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(+)