Some of the later patches are RFC, for varying reasons (see there). 1: x86: determine HAVE_AS_* just once 2: x86: move back clang no integrated assembler tests 3: x86emul: support MOVDIRI insn 4: x86emul: support MOVDIR64B insn 5: x86/HVM: scale MPERF values reported to guests (on AMD) 6: x86emul: support RDPRU 7: x86/HVM: don't needlessly intercept APERF/MPERF/TSC MSR reads 8: x86emul: support MCOMMIT Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06.01.2020 17:31, Jan Beulich wrote: > Some of the later patches are RFC, for varying reasons (see there). > > 1: x86: determine HAVE_AS_* just once > 2: x86: move back clang no integrated assembler tests > 3: x86emul: support MOVDIRI insn > 4: x86emul: support MOVDIR64B insn > 5: x86/HVM: scale MPERF values reported to guests (on AMD) > 6: x86emul: support RDPRU > 7: x86/HVM: don't needlessly intercept APERF/MPERF/TSC MSR reads > 8: x86emul: support MCOMMIT Argh - all of this was meant ot be v4, not (again) v3. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
With the exception of HAVE_AS_QUOTED_SYM, populate the results into a
generated header instead of (at least once per [sub]directory) into
CFLAGS. This results in proper rebuilds (via make dependencies) in case
the compiler used changes between builds. It additionally eases
inspection of which assembler features were actually found usable.
Some trickery is needed to avoid header generation itself to try to
include the to-be/not-yet-generated header.
Since the definitions in generated/config.h, previously having been
command line options, might even affect xen/config.h or its descendants,
move adding of the -include option for the latter after inclusion of the
per-arch Rules.mk. Use the occasion to also move the most general -I
option to the common Rules.mk.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: New.
---
An alternative to the $(MAKECMDGOALS) trickery would be to make
generation of generated/config.h part of the asm-offsets.s rule, instead
of adding it as a dependency there. Not sure whether either is truly
better than the other.
--- a/Config.mk
+++ b/Config.mk
@@ -151,7 +151,7 @@ endif
# as-insn: Check whether assembler supports an instruction.
# Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no)
as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
- | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
+ | $(filter-out -M% %.d -include %/config.h,$(1)) \
-c -x c -o /dev/null - 2>&1),$(4),$(3))
# as-option-add: Conditionally add options to flags
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -59,7 +59,7 @@ endif
CFLAGS += -nostdinc -fno-builtin -fno-common
CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
$(call cc-option-add,CFLAGS,CC,-Wvla)
-CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
+CFLAGS += -pipe -D__XEN__ -I$(BASEDIR)/include
CFLAGS-$(CONFIG_DEBUG_INFO) += -g
CFLAGS += '-D__OBJECT_FILE__="$@"'
@@ -97,6 +97,9 @@ LDFLAGS += $(LDFLAGS-y)
include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
+# Allow the arch to use -include ahead of this one.
+CFLAGS += -include xen/config.h
+
DEPS = .*.d
include Makefile
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -6,8 +6,6 @@
# 'make clean' before rebuilding.
#
-CFLAGS += -I$(BASEDIR)/include
-
$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -224,7 +224,8 @@ endif
efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o
efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ;
-asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
+asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h \
+ $(BASEDIR)/include/generated/config.h
$(CC) $(filter-out -Wa$(comma)% -flto,$(CFLAGS)) -S -o $@ $<
asm-macros.i: CFLAGS += -D__ASSEMBLY__ -P
@@ -240,6 +241,45 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
echo '#endif' >>$@.new
$(call move-if-changed,$@.new,$@)
+# There are multiple invocations of this Makefile, one each for asm-offset.s,
+# $(TARGET), built_in.o, and several more from the rules building $(TARGET)
+# and $(TARGET).efi. The 2nd and 3rd may race with one another, and we don't
+# want to re-generate config.h in that case anyway, so guard the logic
+# accordingly. (We do want to have the FORCE dependency on the rule, to be
+# sure we pick up changes when the compiler used has changed.)
+ifeq ($(MAKECMDGOALS),asm-offsets.s)
+
+as-ISA-list := CLWB EPT FSGSBASE INVPCID RDRAND RDSEED SSE4_2 VMX XSAVEOPT
+
+CLWB-insn := clwb (%rax)
+EPT-insn := invept (%rax),%rax
+FSGSBASE-insn := rdfsbase %rax
+INVPCID-insn := invpcid (%rax),%rax
+RDRAND-insn := rdrand %eax
+RDSEED-insn := rdseed %eax
+SSE4_2-insn := crc32 %eax,%eax
+VMX-insn := vmcall
+XSAVEOPT-insn := xsaveopt (%rax)
+
+as-features-list := $(as-ISA-list) NEGATIVE_TRUE NOPS_DIRECTIVE
+
+# GAS's idea of true is -1. Clang's idea is 1.
+NEGATIVE_TRUE-insn := .if ((1 > 0) > 0); .error \"\"; .endif
+
+# Check to see whether the assembler supports the .nop directive.
+NOPS_DIRECTIVE-insn := .L1: .L2: .nops (.L2 - .L1),9
+
+$(BASEDIR)/include/generated/config.h: FORCE
+ echo '/* Generated header, do not edit. */' >$@.new
+ $(foreach f,$(as-features-list), \
+ $(if $($(f)-insn),,$(error $(f)-insn is empty)) \
+ echo '#$(call as-insn,$(CC) $(CFLAGS),"$($(f)-insn)", \
+ define, \
+ undef) HAVE_AS_$(f) /* $($(f)-insn) */' >>$@.new;)
+ $(call move-if-changed,$@.new,$@)
+
+endif
+
xen.lds: xen.lds.S
$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -3,7 +3,7 @@
XEN_IMG_OFFSET := 0x200000
-CFLAGS += -I$(BASEDIR)/include
+CFLAGS += $(if $(filter asm-macros.% %/generated/config.h,$@),,-include generated/config.h)
CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
@@ -38,26 +38,9 @@ endif
$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
-$(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
-$(call as-option-add,CFLAGS,CC,"crc32 %eax$$(comma)%eax",-DHAVE_AS_SSE4_2)
-$(call as-option-add,CFLAGS,CC,"invept (%rax)$$(comma)%rax",-DHAVE_AS_EPT)
-$(call as-option-add,CFLAGS,CC,"rdrand %eax",-DHAVE_AS_RDRAND)
-$(call as-option-add,CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE)
-$(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
-$(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
-$(call as-option-add,CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB)
$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
-U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \
'-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
-$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID)
-
-# GAS's idea of true is -1. Clang's idea is 1
-$(call as-option-add,CFLAGS,CC,\
- ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE)
-
-# Check to see whether the assmbler supports the .nop directive.
-$(call as-option-add,CFLAGS,CC,\
- ".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE)
CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -66,7 +66,7 @@ compat/%.h: compat/%.i Makefile $(BASEDI
mv -f $@.new $@
compat/%.i: compat/%.c Makefile
- $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
+ $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include %/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py
mkdir -p $(@D)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Mon, Jan 06, 2020 at 05:34:45PM +0100, Jan Beulich wrote:
> With the exception of HAVE_AS_QUOTED_SYM, populate the results into a
> generated header instead of (at least once per [sub]directory) into
> CFLAGS. This results in proper rebuilds (via make dependencies) in case
> the compiler used changes between builds. It additionally eases
> inspection of which assembler features were actually found usable.
>
> Some trickery is needed to avoid header generation itself to try to
> include the to-be/not-yet-generated header.
>
> Since the definitions in generated/config.h, previously having been
> command line options, might even affect xen/config.h or its descendants,
> move adding of the -include option for the latter after inclusion of the
> per-arch Rules.mk. Use the occasion to also move the most general -I
> option to the common Rules.mk.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v4: New.
> ---
> An alternative to the $(MAKECMDGOALS) trickery would be to make
> generation of generated/config.h part of the asm-offsets.s rule, instead
> of adding it as a dependency there. Not sure whether either is truly
> better than the other.
>
> --- a/Config.mk
> +++ b/Config.mk
> @@ -151,7 +151,7 @@ endif
> # as-insn: Check whether assembler supports an instruction.
> # Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no)
> as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
> - | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
> + | $(filter-out -M% %.d -include %/config.h,$(1)) \
Don't you need to filter out -include xen/config.h as added to CLFAGS
below?
> -c -x c -o /dev/null - 2>&1),$(4),$(3))
>
> # as-option-add: Conditionally add options to flags
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -59,7 +59,7 @@ endif
> CFLAGS += -nostdinc -fno-builtin -fno-common
> CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
> $(call cc-option-add,CFLAGS,CC,-Wvla)
> -CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
> +CFLAGS += -pipe -D__XEN__ -I$(BASEDIR)/include
> CFLAGS-$(CONFIG_DEBUG_INFO) += -g
> CFLAGS += '-D__OBJECT_FILE__="$@"'
>
> @@ -97,6 +97,9 @@ LDFLAGS += $(LDFLAGS-y)
>
> include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
>
> +# Allow the arch to use -include ahead of this one.
> +CFLAGS += -include xen/config.h
> +
> DEPS = .*.d
>
> include Makefile
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -6,8 +6,6 @@
> # 'make clean' before rebuilding.
> #
>
> -CFLAGS += -I$(BASEDIR)/include
> -
> $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
>
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -224,7 +224,8 @@ endif
> efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o
> efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ;
>
> -asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
> +asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h \
> + $(BASEDIR)/include/generated/config.h
> $(CC) $(filter-out -Wa$(comma)% -flto,$(CFLAGS)) -S -o $@ $<
>
> asm-macros.i: CFLAGS += -D__ASSEMBLY__ -P
> @@ -240,6 +241,45 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
> echo '#endif' >>$@.new
> $(call move-if-changed,$@.new,$@)
>
> +# There are multiple invocations of this Makefile, one each for asm-offset.s,
> +# $(TARGET), built_in.o, and several more from the rules building $(TARGET)
> +# and $(TARGET).efi. The 2nd and 3rd may race with one another, and we don't
> +# want to re-generate config.h in that case anyway, so guard the logic
> +# accordingly. (We do want to have the FORCE dependency on the rule, to be
> +# sure we pick up changes when the compiler used has changed.)
> +ifeq ($(MAKECMDGOALS),asm-offsets.s)
> +
> +as-ISA-list := CLWB EPT FSGSBASE INVPCID RDRAND RDSEED SSE4_2 VMX XSAVEOPT
> +
> +CLWB-insn := clwb (%rax)
> +EPT-insn := invept (%rax),%rax
> +FSGSBASE-insn := rdfsbase %rax
> +INVPCID-insn := invpcid (%rax),%rax
> +RDRAND-insn := rdrand %eax
> +RDSEED-insn := rdseed %eax
> +SSE4_2-insn := crc32 %eax,%eax
> +VMX-insn := vmcall
> +XSAVEOPT-insn := xsaveopt (%rax)
> +
> +as-features-list := $(as-ISA-list) NEGATIVE_TRUE NOPS_DIRECTIVE
I think it would be clearer to place this below the NEGATIVE_TRUE and
NOPS_DIRECTIVE definitions below? So that all FOO-insn are together.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 20.01.2020 13:04, Roger Pau Monné wrote:
> On Mon, Jan 06, 2020 at 05:34:45PM +0100, Jan Beulich wrote:
>> --- a/Config.mk
>> +++ b/Config.mk
>> @@ -151,7 +151,7 @@ endif
>> # as-insn: Check whether assembler supports an instruction.
>> # Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no)
>> as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
>> - | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
>> + | $(filter-out -M% %.d -include %/config.h,$(1)) \
>
> Don't you need to filter out -include xen/config.h as added to CLFAGS
> below?
The whole point of the change is to filter out _both_ config.h (as
well as any future one) - the one under include/xen/ and the one
under include/generated/. Hence the widening of what the pattern
would match.
>> @@ -240,6 +241,45 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
>> echo '#endif' >>$@.new
>> $(call move-if-changed,$@.new,$@)
>>
>> +# There are multiple invocations of this Makefile, one each for asm-offset.s,
>> +# $(TARGET), built_in.o, and several more from the rules building $(TARGET)
>> +# and $(TARGET).efi. The 2nd and 3rd may race with one another, and we don't
>> +# want to re-generate config.h in that case anyway, so guard the logic
>> +# accordingly. (We do want to have the FORCE dependency on the rule, to be
>> +# sure we pick up changes when the compiler used has changed.)
>> +ifeq ($(MAKECMDGOALS),asm-offsets.s)
>> +
>> +as-ISA-list := CLWB EPT FSGSBASE INVPCID RDRAND RDSEED SSE4_2 VMX XSAVEOPT
>> +
>> +CLWB-insn := clwb (%rax)
>> +EPT-insn := invept (%rax),%rax
>> +FSGSBASE-insn := rdfsbase %rax
>> +INVPCID-insn := invpcid (%rax),%rax
>> +RDRAND-insn := rdrand %eax
>> +RDSEED-insn := rdseed %eax
>> +SSE4_2-insn := crc32 %eax,%eax
>> +VMX-insn := vmcall
>> +XSAVEOPT-insn := xsaveopt (%rax)
>> +
>> +as-features-list := $(as-ISA-list) NEGATIVE_TRUE NOPS_DIRECTIVE
>
> I think it would be clearer to place this below the NEGATIVE_TRUE and
> NOPS_DIRECTIVE definitions below? So that all FOO-insn are together.
If your remark was about just the last line - yes, perhaps (and
looking at it again I don't even know why it ended up in the
place it's in right now). But I'm told the original mechanism is
going to be replaced now by a Kconfig one anyway. If this is
going to happen soon, the patch here would be of no further
interest.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06/01/2020 16:34, Jan Beulich wrote: > With the exception of HAVE_AS_QUOTED_SYM, populate the results into a > generated header instead of (at least once per [sub]directory) into > CFLAGS. This results in proper rebuilds (via make dependencies) in case > the compiler used changes between builds. It additionally eases > inspection of which assembler features were actually found usable. > > Some trickery is needed to avoid header generation itself to try to > include the to-be/not-yet-generated header. > > Since the definitions in generated/config.h, previously having been > command line options, might even affect xen/config.h or its descendants, > move adding of the -include option for the latter after inclusion of the > per-arch Rules.mk. Use the occasion to also move the most general -I > option to the common Rules.mk. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > v4: New. Is it v3 or v4? Also, is it intended for just backport? It is largely redundant with Anthony's Kconfig/Kbuild efforts, as moving these (and other checks) into the Kconfig step is the ultimate goal. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06.01.2020 17:41, Andrew Cooper wrote: > On 06/01/2020 16:34, Jan Beulich wrote: >> With the exception of HAVE_AS_QUOTED_SYM, populate the results into a >> generated header instead of (at least once per [sub]directory) into >> CFLAGS. This results in proper rebuilds (via make dependencies) in case >> the compiler used changes between builds. It additionally eases >> inspection of which assembler features were actually found usable. >> >> Some trickery is needed to avoid header generation itself to try to >> include the to-be/not-yet-generated header. >> >> Since the definitions in generated/config.h, previously having been >> command line options, might even affect xen/config.h or its descendants, >> move adding of the -include option for the latter after inclusion of the >> per-arch Rules.mk. Use the occasion to also move the most general -I >> option to the common Rules.mk. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> v4: New. > > Is it v3 or v4? v4, as said in reply to the cover letter. > Also, is it intended for just backport? It is largely redundant with > Anthony's Kconfig/Kbuild efforts, as moving these (and other checks) > into the Kconfig step is the ultimate goal. Is it? Looking at current Linux I still see e.g. # do binutils support CFI? cfi := $(call as-instr,.cfi_startproc\n.cfi_rel_offset $(sp-y)$(comma)0\n.cfi_endproc,-DCONFIG_AS_CFI=1) # is .cfi_signal_frame supported too? cfi-sigframe := $(call as-instr,.cfi_startproc\n.cfi_signal_frame\n.cfi_endproc,-DCONFIG_AS_CFI_SIGNAL_FRAME=1) cfi-sections := $(call as-instr,.cfi_sections .debug_frame,-DCONFIG_AS_CFI_SECTIONS=1) # does binutils support specific instructions? asinstr += $(call as-instr,pshufb %xmm0$(comma)%xmm0,-DCONFIG_AS_SSSE3=1) avx_instr := $(call as-instr,vxorps %ymm0$(comma)%ymm1$(comma)%ymm2,-DCONFIG_AS_AVX=1) avx2_instr :=$(call as-instr,vpbroadcastb %xmm0$(comma)%ymm1,-DCONFIG_AS_AVX2=1) avx512_instr :=$(call as-instr,vpmovm2b %k1$(comma)%zmm5,-DCONFIG_AS_AVX512=1) sha1_ni_instr :=$(call as-instr,sha1msg1 %xmm0$(comma)%xmm1,-DCONFIG_AS_SHA1_NI=1) sha256_ni_instr :=$(call as-instr,sha256msg1 %xmm0$(comma)%xmm1,-DCONFIG_AS_SHA256_NI=1) KBUILD_AFLAGS += $(cfi) $(cfi-sigframe) $(cfi-sections) $(asinstr) $(avx_instr) $(avx2_instr) $(avx512_instr) $(sha1_ni_instr) $(sha256_ni_instr) KBUILD_CFLAGS += $(cfi) $(cfi-sigframe) $(cfi-sections) $(asinstr) $(avx_instr) $(avx2_instr) $(avx512_instr) $(sha1_ni_instr) $(sha256_ni_instr) in arch/x86/Makefile. I am, btw, also unconvinced that such belongs into .config in the first place: The configuration doesn't change if I swap my tool chain. Of course I realized there's the grey area of user visible options depending on tool chain capabilities (if one means to allow such). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
This largely reverts f19af2f1138e ("x86: re-order clang no integrated
assembler tests"): Other CFLAGS setup would better happen first, in case
any of it affects the behavior of the integrated assembler. The comment
addition of course doesn't get undone. The only remaining as-option-add
invocation gets moved down in addition.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: New.
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -12,35 +12,8 @@ CFLAGS += '-D__OBJECT_LABEL__=$(subst /,
# Prevent floating-point variables from creeping into Xen.
CFLAGS += -msoft-float
-ifeq ($(clang),y)
-# Note: Any test which adds -no-integrated-as will cause subsequent tests to
-# succeed, and not trigger further additions.
-#
-# The tests to select whether the integrated assembler is usable need to happen
-# before testing any assembler features, or else the result of the tests would
-# be stale if the integrated assembler is not used.
-
-# Older clang's built-in assembler doesn't understand .skip with labels:
-# https://bugs.llvm.org/show_bug.cgi?id=27369
-$(call as-option-add,CFLAGS,CC,".L0: .L1: .skip (.L1 - .L0)",,\
- -no-integrated-as)
-
-# Check whether clang asm()-s support .include.
-$(call as-option-add,CFLAGS,CC,".include \"asm/indirect_thunk_asm.h\"",,\
- -no-integrated-as)
-
-# Check whether clang keeps .macro-s between asm()-s:
-# https://bugs.llvm.org/show_bug.cgi?id=36110
-$(call as-option-add,CFLAGS,CC,\
- ".macro FOO;.endm"$$(close); asm volatile $$(open)".macro FOO;.endm",\
- -no-integrated-as)
-endif
-
$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
-$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
- -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \
- '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
@@ -77,3 +50,30 @@ endif
# Set up the assembler include path properly for older toolchains.
CFLAGS += -Wa,-I$(BASEDIR)/include
+ifeq ($(clang),y)
+# Note: Any test which adds -no-integrated-as will cause subsequent tests to
+# succeed, and not trigger further additions.
+#
+# The tests to select whether the integrated assembler is usable need to happen
+# before testing any assembler features, or else the result of the tests would
+# be stale if the integrated assembler is not used.
+
+# Older clang's built-in assembler doesn't understand .skip with labels:
+# https://bugs.llvm.org/show_bug.cgi?id=27369
+$(call as-option-add,CFLAGS,CC,".L0: .L1: .skip (.L1 - .L0)",,\
+ -no-integrated-as)
+
+# Check whether clang asm()-s support .include.
+$(call as-option-add,CFLAGS,CC,".include \"asm/indirect_thunk_asm.h\"",,\
+ -no-integrated-as)
+
+# Check whether clang keeps .macro-s between asm()-s:
+# https://bugs.llvm.org/show_bug.cgi?id=36110
+$(call as-option-add,CFLAGS,CC,\
+ ".macro FOO;.endm"$$(close); asm volatile $$(open)".macro FOO;.endm",\
+ -no-integrated-as)
+endif
+
+$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
+ -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \
+ '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Mon, Jan 06, 2020 at 05:35:16PM +0100, Jan Beulich wrote:
> This largely reverts f19af2f1138e ("x86: re-order clang no integrated
> assembler tests"): Other CFLAGS setup would better happen first, in case
> any of it affects the behavior of the integrated assembler. The comment
> addition of course doesn't get undone. The only remaining as-option-add
> invocation gets moved down in addition.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Note that SDM revision 070 doesn't specify exception behavior for
ModRM.mod != 0b11; assuming #UD here.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Split MOVDIRI and MOVDIR64B. Re-base.
v3: Update description.
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -2196,6 +2196,18 @@ int main(int argc, char **argv)
goto fail;
printf("okay\n");
+ printf("%-40s", "Testing movdiri %edx,(%ecx)...");
+ instr[0] = 0x0f; instr[1] = 0x38; instr[2] = 0xf9; instr[3] = 0x11;
+ regs.eip = (unsigned long)&instr[0];
+ regs.ecx = (unsigned long)memset(res, -1, 16);
+ regs.edx = 0x44332211;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (regs.eip != (unsigned long)&instr[4]) ||
+ res[0] != 0x44332211 || ~res[1] )
+ goto fail;
+ printf("okay\n");
+
printf("%-40s", "Testing movq %mm3,(%ecx)...");
if ( stack_exec && cpu_has_mmx )
{
--- a/tools/tests/x86_emulator/x86-emulate.c
+++ b/tools/tests/x86_emulator/x86-emulate.c
@@ -76,6 +76,7 @@ bool emul_test_init(void)
cp.feat.adx = true;
cp.feat.avx512pf = cp.feat.avx512f;
cp.feat.rdpid = true;
+ cp.feat.movdiri = true;
cp.extd.clzero = true;
if ( cpu_has_xsave )
@@ -137,15 +138,15 @@ int emul_test_cpuid(
res->c |= 1U << 22;
/*
- * The emulator doesn't itself use ADCX/ADOX/RDPID nor the S/G prefetch
- * insns, so we can always run the respective tests.
+ * The emulator doesn't itself use ADCX/ADOX/RDPID/MOVDIRI nor the S/G
+ * prefetch insns, so we can always run the respective tests.
*/
if ( leaf == 7 && subleaf == 0 )
{
res->b |= (1U << 10) | (1U << 19);
if ( res->b & (1U << 16) )
res->b |= 1U << 26;
- res->c |= 1U << 22;
+ res->c |= (1U << 22) | (1U << 27);
}
/*
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -548,6 +548,7 @@ static const struct ext0f38_table {
[0xf1] = { .to_mem = 1, .two_op = 1 },
[0xf2 ... 0xf3] = {},
[0xf5 ... 0xf7] = {},
+ [0xf9] = { .to_mem = 1 },
};
/* Shift values between src and dst sizes of pmov{s,z}x{b,w,d}{w,d,q}. */
@@ -1902,6 +1903,7 @@ in_protmode(
#define vcpu_has_avx512_bitalg() (ctxt->cpuid->feat.avx512_bitalg)
#define vcpu_has_avx512_vpopcntdq() (ctxt->cpuid->feat.avx512_vpopcntdq)
#define vcpu_has_rdpid() (ctxt->cpuid->feat.rdpid)
+#define vcpu_has_movdiri() (ctxt->cpuid->feat.movdiri)
#define vcpu_has_avx512_4vnniw() (ctxt->cpuid->feat.avx512_4vnniw)
#define vcpu_has_avx512_4fmaps() (ctxt->cpuid->feat.avx512_4fmaps)
@@ -2712,10 +2714,12 @@ x86_decode_0f38(
{
case 0x00 ... 0xef:
case 0xf2 ... 0xf5:
- case 0xf7 ... 0xff:
+ case 0xf7 ... 0xf8:
+ case 0xfa ... 0xff:
op_bytes = 0;
/* fall through */
case 0xf6: /* adcx / adox */
+ case 0xf9: /* movdiri */
ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
break;
@@ -9915,6 +9919,14 @@ x86_emulate(
: "0" ((uint32_t)src.val), "rm" (_regs.edx) );
break;
+ case X86EMUL_OPC(0x0f38, 0xf9): /* movdiri mem,r */
+ vcpu_must_have(movdiri);
+ generate_exception_if(dst.type != OP_MEM, EXC_UD);
+ /* Ignore the non-temporal behavior for now. */
+ dst.val = src.val;
+ sfence = true;
+ break;
+
case X86EMUL_OPC_VEX_66(0x0f3a, 0x00): /* vpermq $imm8,ymm/m256,ymm */
case X86EMUL_OPC_VEX_66(0x0f3a, 0x01): /* vpermpd $imm8,ymm/m256,ymm */
generate_exception_if(!vex.l || !vex.w, EXC_UD);
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -237,6 +237,7 @@ XEN_CPUFEATURE(AVX512_BITALG, 6*32+12) /
XEN_CPUFEATURE(AVX512_VPOPCNTDQ, 6*32+14) /*A POPCNT for vectors of DW/QW */
XEN_CPUFEATURE(RDPID, 6*32+22) /*A RDPID instruction */
XEN_CPUFEATURE(CLDEMOTE, 6*32+25) /*A CLDEMOTE instruction */
+XEN_CPUFEATURE(MOVDIRI, 6*32+27) /*A MOVDIRI instruction */
/* AMD-defined CPU features, CPUID level 0x80000007.edx, word 7 */
XEN_CPUFEATURE(ITSC, 7*32+ 8) /* Invariant TSC */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06/01/2020 16:35, Jan Beulich wrote: > Note that SDM revision 070 doesn't specify exception behavior for > ModRM.mod != 0b11; assuming #UD here. Don't you mean mod == 11 here? My Jacobsville SDP does have MOVDIRI, and 0F 38 F9 C0 does result in #UD, whereas the example from the test emulator below (ModRM byte as 0x11) does function as expected. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06.01.2020 17:56, Andrew Cooper wrote: > On 06/01/2020 16:35, Jan Beulich wrote: >> Note that SDM revision 070 doesn't specify exception behavior for >> ModRM.mod != 0b11; assuming #UD here. > > Don't you mean mod == 11 here? Oh, yes, of course I do - corrected. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Note that SDM revision 070 doesn't specify exception behavior for
ModRM.mod != 0b11; assuming #UD here.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC: Yet to be tested (once SDE supports it).
---
v4: Split MOVDIRI and MOVDIR64B. Switch to using ->rmw(). Re-base.
v3: Update description.
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -2208,6 +2208,29 @@ int main(int argc, char **argv)
goto fail;
printf("okay\n");
+ printf("%-40s", "Testing movdir64b 144(%edx),%ecx...");
+ if ( stack_exec && cpu_has_movdir64b )
+ {
+ instr[0] = 0x66; instr[1] = 0x0f; instr[2] = 0x38; instr[3] = 0xf8;
+ instr[4] = 0x8a; instr[5] = 0x90; instr[8] = instr[7] = instr[6] = 0;
+ regs.eip = (unsigned long)&instr[0];
+ for ( i = 0; i < 64; ++i )
+ res[i] = i - 20;
+ regs.edx = (unsigned long)res;
+ regs.ecx = (unsigned long)(res + 16);
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (regs.eip != (unsigned long)&instr[9]) ||
+ res[15] != -5 || res[32] != 12 )
+ goto fail;
+ for ( i = 16; i < 32; ++i )
+ if ( res[i] != i )
+ goto fail;
+ printf("okay\n");
+ }
+ else
+ printf("skipped\n");
+
printf("%-40s", "Testing movq %mm3,(%ecx)...");
if ( stack_exec && cpu_has_mmx )
{
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -154,6 +154,7 @@ static inline bool xcr0_mask(uint64_t ma
#define cpu_has_avx512_vnni (cp.feat.avx512_vnni && xcr0_mask(0xe6))
#define cpu_has_avx512_bitalg (cp.feat.avx512_bitalg && xcr0_mask(0xe6))
#define cpu_has_avx512_vpopcntdq (cp.feat.avx512_vpopcntdq && xcr0_mask(0xe6))
+#define cpu_has_movdir64b cp.feat.movdir64b
#define cpu_has_avx512_4vnniw (cp.feat.avx512_4vnniw && xcr0_mask(0xe6))
#define cpu_has_avx512_4fmaps (cp.feat.avx512_4fmaps && xcr0_mask(0xe6))
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -249,12 +249,13 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
# sure we pick up changes when the compiler used has changed.)
ifeq ($(MAKECMDGOALS),asm-offsets.s)
-as-ISA-list := CLWB EPT FSGSBASE INVPCID RDRAND RDSEED SSE4_2 VMX XSAVEOPT
+as-ISA-list := CLWB EPT FSGSBASE INVPCID MOVDIR64B RDRAND RDSEED SSE4_2 VMX XSAVEOPT
CLWB-insn := clwb (%rax)
EPT-insn := invept (%rax),%rax
FSGSBASE-insn := rdfsbase %rax
INVPCID-insn := invpcid (%rax),%rax
+MOVDIR64B-insn := movdir64b (%rax),%rax
RDRAND-insn := rdrand %eax
RDSEED-insn := rdseed %eax
SSE4_2-insn := crc32 %eax,%eax
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -548,6 +548,7 @@ static const struct ext0f38_table {
[0xf1] = { .to_mem = 1, .two_op = 1 },
[0xf2 ... 0xf3] = {},
[0xf5 ... 0xf7] = {},
+ [0xf8] = { .simd_size = simd_other },
[0xf9] = { .to_mem = 1 },
};
@@ -834,6 +835,7 @@ struct x86_emulate_state {
rmw_bts,
rmw_dec,
rmw_inc,
+ rmw_movdir64b,
rmw_neg,
rmw_not,
rmw_or,
@@ -896,6 +898,7 @@ typedef union {
uint64_t __attribute__ ((aligned(16))) xmm[2];
uint64_t __attribute__ ((aligned(32))) ymm[4];
uint64_t __attribute__ ((aligned(64))) zmm[8];
+ uint32_t words[16];
} mmval_t;
/*
@@ -1904,6 +1907,7 @@ in_protmode(
#define vcpu_has_avx512_vpopcntdq() (ctxt->cpuid->feat.avx512_vpopcntdq)
#define vcpu_has_rdpid() (ctxt->cpuid->feat.rdpid)
#define vcpu_has_movdiri() (ctxt->cpuid->feat.movdiri)
+#define vcpu_has_movdir64b() (ctxt->cpuid->feat.movdir64b)
#define vcpu_has_avx512_4vnniw() (ctxt->cpuid->feat.avx512_4vnniw)
#define vcpu_has_avx512_4fmaps() (ctxt->cpuid->feat.avx512_4fmaps)
@@ -9919,6 +9923,23 @@ x86_emulate(
: "0" ((uint32_t)src.val), "rm" (_regs.edx) );
break;
+ case X86EMUL_OPC_66(0x0f38, 0xf8): /* movdir64b r,m512 */
+ host_and_vcpu_must_have(movdir64b);
+ generate_exception_if(ea.type != OP_MEM, EXC_UD);
+ src.val = truncate_ea(*dst.reg);
+ generate_exception_if(!is_aligned(x86_seg_es, src.val, 64, ctxt, ops),
+ EXC_GP, 0);
+ fail_if(!ops->rmw);
+ BUILD_BUG_ON(sizeof(*mmvalp) < 64);
+ state->rmw = rmw_movdir64b;
+ if ( (rc = ops->read(ea.mem.seg, ea.mem.off, mmvalp, 64,
+ ctxt)) != X86EMUL_OKAY ||
+ (rc = ops->rmw(x86_seg_es, src.val, 64, &mmvalp->words[0],
+ state, ctxt)) != X86EMUL_OKAY )
+ goto done;
+ state->simd_size = simd_none;
+ break;
+
case X86EMUL_OPC(0x0f38, 0xf9): /* movdiri mem,r */
vcpu_must_have(movdiri);
generate_exception_if(dst.type != OP_MEM, EXC_UD);
@@ -11074,6 +11095,26 @@ int x86_emul_rmw(
#undef BINOP
#undef SHIFT
+ case rmw_movdir64b:
+ if ( ((unsigned long)dst & 0x3f) )
+ {
+ ASSERT_UNREACHABLE();
+ return X86EMUL_UNHANDLEABLE;
+ }
+ /*
+ * eflags points to source data in this case. Use a memory clobber
+ * to compensate that the other operands don't properly express the
+ * (full) memory ranges covered.
+ */
+#ifdef HAVE_AS_MOVDIR64B
+ asm ( "movdir64b %0, %1" :: "m" (*dst), "r" (eflags) : "memory" );
+#else
+ /* movdir64b (%rdi), %rdx */
+ asm ( ".byte 0x66, 0x0f, 0x38, 0xf8, 0x17"
+ :: "D" (dst), "d" (eflags) : "memory" );
+#endif
+ break;
+
case rmw_not:
switch ( state->op_bytes )
{
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -120,6 +120,7 @@
#define cpu_has_avx512_bitalg boot_cpu_has(X86_FEATURE_AVX512_BITALG)
#define cpu_has_avx512_vpopcntdq boot_cpu_has(X86_FEATURE_AVX512_VPOPCNTDQ)
#define cpu_has_rdpid boot_cpu_has(X86_FEATURE_RDPID)
+#define cpu_has_movdir64b boot_cpu_has(X86_FEATURE_MOVDIR64B)
/* CPUID level 0x80000007.edx */
#define cpu_has_itsc boot_cpu_has(X86_FEATURE_ITSC)
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -238,6 +238,7 @@ XEN_CPUFEATURE(AVX512_VPOPCNTDQ, 6*32+14
XEN_CPUFEATURE(RDPID, 6*32+22) /*A RDPID instruction */
XEN_CPUFEATURE(CLDEMOTE, 6*32+25) /*A CLDEMOTE instruction */
XEN_CPUFEATURE(MOVDIRI, 6*32+27) /*A MOVDIRI instruction */
+XEN_CPUFEATURE(MOVDIR64B, 6*32+28) /*A MOVDIR64B instruction */
/* AMD-defined CPU features, CPUID level 0x80000007.edx, word 7 */
XEN_CPUFEATURE(ITSC, 7*32+ 8) /* Invariant TSC */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06/01/2020 16:36, Jan Beulich wrote:
> Note that SDM revision 070 doesn't specify exception behavior for
> ModRM.mod != 0b11; assuming #UD here.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> RFC: Yet to be tested (once SDE supports it).
Do you have any plans for how to support ENQCMD{,S} which are just
variations of MOVDIR64B using the F2 and F3 prefixes for behaviour
modification, and whether those might alter what we do here?
The underlying write out into the system is going to be largely the
same, but it can't be shared with MOVDIR64B because it generates a
different type of bus transaction. Also, they require ZF to find their
way back out of the rmw() handler.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06.01.2020 20:38, Andrew Cooper wrote:
> On 06/01/2020 16:36, Jan Beulich wrote:
>> Note that SDM revision 070 doesn't specify exception behavior for
>> ModRM.mod != 0b11; assuming #UD here.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> RFC: Yet to be tested (once SDE supports it).
>
> Do you have any plans for how to support ENQCMD{,S} which are just
> variations of MOVDIR64B using the F2 and F3 prefixes for behaviour
> modification, and whether those might alter what we do here?
>
> The underlying write out into the system is going to be largely the
> same, but it can't be shared with MOVDIR64B because it generates a
> different type of bus transaction. Also, they require ZF to find their
> way back out of the rmw() handler.
To be honest, I hadn't looked at the ENQCMD{,S} details at all yet
(and iirc they weren't even available when I first worked on this).
I will re-consider the approach - thanks for pointing this out.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
AMD's PM specifies that MPERF (and its r/o counterpart) reads are
affected by the TSC ratio. Hence when processing such reads in software
we too should scale the values. While we don't currently (yet) expose
the underlying feature flags, besides us allowing the MSRs to be read
nevertheless, RDPRU is going to expose the values even to user space.
Furthermore, due to the not exposed feature flags, this change has the
effect of making properly inaccessible (for reads) the two MSRs.
Note that writes to MPERF (and APERF) continue to be unsupported.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: New.
---
I did consider whether to put the code in guest_rdmsr() instead, but
decided that it's better to have it next to TSC handling.
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3440,6 +3440,22 @@ int hvm_msr_read_intercept(unsigned int
*msr_content = v->arch.hvm.msr_tsc_adjust;
break;
+ case MSR_MPERF_RD_ONLY:
+ if ( !d->arch.cpuid->extd.efro )
+ {
+ goto gp_fault;
+
+ case MSR_IA32_MPERF:
+ if ( !(d->arch.cpuid->basic.raw[6].c &
+ CPUID6_ECX_APERFMPERF_CAPABILITY) )
+ goto gp_fault;
+ }
+ if ( rdmsr_safe(msr, *msr_content) )
+ goto gp_fault;
+ if ( d->arch.cpuid->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ *msr_content = hvm_get_guest_tsc_fixed(v, *msr_content);
+ break;
+
case MSR_APIC_BASE:
*msr_content = vcpu_vlapic(v)->hw.apic_base_msr;
break;
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -377,6 +377,9 @@
#define MSR_IA32_MPERF 0x000000e7
#define MSR_IA32_APERF 0x000000e8
+#define MSR_MPERF_RD_ONLY 0xc00000e7
+#define MSR_APERF_RD_ONLY 0xc00000e8
+
#define MSR_IA32_THERM_CONTROL 0x0000019a
#define MSR_IA32_THERM_INTERRUPT 0x0000019b
#define MSR_IA32_THERM_STATUS 0x0000019c
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
While the PM doesn't say so, this assumes that the MPERF value read this
way gets scaled similarly to its reading through RDMSR.
Also introduce the SVM related constants at this occasion.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC: Andrew promised to take care of the CPUID side of this; will need
re-basing over his work once available.
TBD: There are indications that the CPUID field used may be just 8 bits
wide.
---
v4: Add GENERAL2_INTERCEPT_RDPRU and VMEXIT_RDPRU enumerators. Fold
handling of out of bounds indexes into switch(). Avoid
recalculate_misc() clobbering what recalculate_cpu_policy() has
done. Re-base.
v3: New.
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -259,6 +259,7 @@ int libxl_cpuid_parse_config(libxl_cpuid
{"clzero", 0x80000008, NA, CPUID_REG_EBX, 0, 1},
{"rstr-fp-err-ptrs", 0x80000008, NA, CPUID_REG_EBX, 2, 1},
+ {"rdpru", 0x80000008, NA, CPUID_REG_EBX, 4, 1},
{"wbnoinvd", 0x80000008, NA, CPUID_REG_EBX, 9, 1},
{"ibpb", 0x80000008, NA, CPUID_REG_EBX, 12, 1},
{"ppin", 0x80000008, NA, CPUID_REG_EBX, 23, 1},
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -147,6 +147,8 @@ static const char *const str_e8b[32] =
[ 0] = "clzero",
[ 2] = "rstr-fp-err-ptrs",
+ [ 4] = "rdpru",
+
/* [ 8] */ [ 9] = "wbnoinvd",
[12] = "ibpb",
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -671,6 +671,13 @@ static int read_msr(
{
switch ( reg )
{
+ case 0x000000e8: /* APERF */
+ case 0xc00000e8: /* APERF_RD_ONLY */
+#define APERF_LO_VALUE 0xAEAEAEAE
+#define APERF_HI_VALUE 0xEAEAEAEA
+ *val = ((uint64_t)APERF_HI_VALUE << 32) | APERF_LO_VALUE;
+ return X86EMUL_OKAY;
+
case 0xc0000080: /* EFER */
*val = ctxt->addr_size > 32 ? 0x500 /* LME|LMA */ : 0;
return X86EMUL_OKAY;
@@ -2231,6 +2238,30 @@ int main(int argc, char **argv)
else
printf("skipped\n");
+ printf("%-40s", "Testing rdpru...");
+ instr[0] = 0x0f; instr[1] = 0x01; instr[2] = 0xfd;
+ regs.eip = (unsigned long)&instr[0];
+ regs.ecx = 1;
+ regs.eflags = EFLAGS_ALWAYS_SET;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (regs.eax != APERF_LO_VALUE) || (regs.edx != APERF_HI_VALUE) ||
+ !(regs.eflags & X86_EFLAGS_CF) ||
+ (regs.eip != (unsigned long)&instr[3]) )
+ goto fail;
+ if ( ctxt.cpuid->extd.rdpru_max < 0xffff )
+ {
+ regs.eip = (unsigned long)&instr[0];
+ regs.ecx = ctxt.cpuid->extd.rdpru_max + 1;
+ regs.eflags = EFLAGS_ALWAYS_SET | X86_EFLAGS_CF;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) || regs.eax || regs.edx ||
+ (regs.eflags & X86_EFLAGS_CF) ||
+ (regs.eip != (unsigned long)&instr[3]) )
+ goto fail;
+ }
+ printf("okay\n");
+
printf("%-40s", "Testing movq %mm3,(%ecx)...");
if ( stack_exec && cpu_has_mmx )
{
--- a/tools/tests/x86_emulator/x86-emulate.c
+++ b/tools/tests/x86_emulator/x86-emulate.c
@@ -78,6 +78,8 @@ bool emul_test_init(void)
cp.feat.rdpid = true;
cp.feat.movdiri = true;
cp.extd.clzero = true;
+ cp.extd.rdpru = true;
+ cp.extd.rdpru_max = 1;
if ( cpu_has_xsave )
{
@@ -150,11 +152,11 @@ int emul_test_cpuid(
}
/*
- * The emulator doesn't itself use CLZERO, so we can always run the
+ * The emulator doesn't itself use CLZERO/RDPRU, so we can always run the
* respective test(s).
*/
if ( leaf == 0x80000008 )
- res->b |= 1U << 0;
+ res->b |= (1U << 0) | (1U << 4);
return X86EMUL_OKAY;
}
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -232,8 +232,6 @@ static void recalculate_misc(struct cpui
/* Most of Power/RAS hidden from guests. */
p->extd.raw[0x7].a = p->extd.raw[0x7].b = p->extd.raw[0x7].c = 0;
- p->extd.raw[0x8].d = 0;
-
switch ( p->x86_vendor )
{
case X86_VENDOR_INTEL:
@@ -252,6 +250,7 @@ static void recalculate_misc(struct cpui
p->extd.raw[0x8].a &= 0x0000ffff;
p->extd.raw[0x8].c = 0;
+ p->extd.raw[0x8].d = 0;
break;
case X86_VENDOR_AMD:
@@ -270,6 +269,7 @@ static void recalculate_misc(struct cpui
p->extd.raw[0x8].a &= 0x0000ffff; /* GuestMaxPhysAddr hidden. */
p->extd.raw[0x8].c &= 0x0003f0ff;
+ p->extd.raw[0x8].d &= 0xffff0000;
p->extd.raw[0x9] = EMPTY_LEAF;
@@ -577,6 +577,11 @@ void recalculate_cpuid_policy(struct dom
p->extd.maxlinaddr = p->extd.lm ? 48 : 32;
+ if ( p->extd.rdpru )
+ p->extd.rdpru_max = min(p->extd.rdpru_max, max->extd.rdpru_max);
+ else
+ p->extd.rdpru_max = 0;
+
recalculate_xstate(p);
recalculate_misc(p);
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1874,6 +1874,7 @@ in_protmode(
#define vcpu_has_fma4() (ctxt->cpuid->extd.fma4)
#define vcpu_has_tbm() (ctxt->cpuid->extd.tbm)
#define vcpu_has_clzero() (ctxt->cpuid->extd.clzero)
+#define vcpu_has_rdpru() (ctxt->cpuid->extd.rdpru)
#define vcpu_has_wbnoinvd() (ctxt->cpuid->extd.wbnoinvd)
#define vcpu_has_bmi1() (ctxt->cpuid->feat.bmi1)
@@ -5698,6 +5699,50 @@ x86_emulate(
limit -= sizeof(zero);
}
break;
+
+ case 0xfd: /* rdpru */
+ vcpu_must_have(rdpru);
+
+ if ( !mode_ring0() )
+ {
+ fail_if(!ops->read_cr);
+ if ( (rc = ops->read_cr(4, &cr4, ctxt)) != X86EMUL_OKAY )
+ goto done;
+ generate_exception_if(cr4 & X86_CR4_TSD, EXC_UD);
+ }
+
+ switch ( _regs.ecx | -(_regs.ecx > ctxt->cpuid->extd.rdpru_max) )
+ {
+ case 0: n = MSR_IA32_MPERF; break;
+ case 1: n = MSR_IA32_APERF; break;
+ default: n = 0; break;
+ }
+
+ _regs.eflags &= ~EFLAGS_MASK;
+ if ( n )
+ {
+ fail_if(!ops->read_msr);
+ switch ( rc = ops->read_msr(n, &msr_val, ctxt) )
+ {
+ case X86EMUL_OKAY:
+ _regs.eflags |= X86_EFLAGS_CF;
+ break;
+
+ case X86EMUL_EXCEPTION:
+ x86_emul_reset_event(ctxt);
+ rc = X86EMUL_OKAY;
+ break;
+
+ default:
+ goto done;
+ }
+ }
+
+ if ( !(_regs.eflags & X86_EFLAGS_CF) )
+ msr_val = 0;
+ _regs.r(dx) = msr_val >> 32;
+ _regs.r(ax) = (uint32_t)msr_val;
+ break;
}
#define _GRP7(mod, reg) \
--- a/xen/include/asm-x86/hvm/svm/vmcb.h
+++ b/xen/include/asm-x86/hvm/svm/vmcb.h
@@ -76,7 +76,8 @@ enum GenericIntercept2bits
GENERAL2_INTERCEPT_MONITOR = 1 << 10,
GENERAL2_INTERCEPT_MWAIT = 1 << 11,
GENERAL2_INTERCEPT_MWAIT_CONDITIONAL = 1 << 12,
- GENERAL2_INTERCEPT_XSETBV = 1 << 13
+ GENERAL2_INTERCEPT_XSETBV = 1 << 13,
+ GENERAL2_INTERCEPT_RDPRU = 1 << 14,
};
@@ -300,6 +301,7 @@ enum VMEXIT_EXITCODE
VMEXIT_MWAIT = 139, /* 0x8b */
VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */
VMEXIT_XSETBV = 141, /* 0x8d */
+ VMEXIT_RDPRU = 142, /* 0x8e */
VMEXIT_NPF = 1024, /* 0x400, nested paging fault */
VMEXIT_INVALID = -1
};
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -247,6 +247,7 @@ XEN_CPUFEATURE(EFRO, 7*32+10) /
/* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
XEN_CPUFEATURE(CLZERO, 8*32+ 0) /*A CLZERO instruction */
XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
+XEN_CPUFEATURE(RDPRU, 8*32+ 4) /*A RDPRU instruction */
XEN_CPUFEATURE(WBNOINVD, 8*32+ 9) /* WBNOINVD instruction */
XEN_CPUFEATURE(IBPB, 8*32+12) /*A IBPB support only (no IBRS, used by AMD) */
XEN_CPUFEATURE(AMD_PPIN, 8*32+23) /* Protected Processor Inventory Number */
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -264,7 +264,7 @@ struct cpuid_policy
struct { DECL_BITFIELD(e8b); };
};
uint32_t nc:8, :4, apic_id_size:4, :16;
- uint32_t /* d */:32;
+ uint16_t :16, rdpru_max;
};
} extd;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06/01/2020 16:37, Jan Beulich wrote: > While the PM doesn't say so, this assumes that the MPERF value read this > way gets scaled similarly to its reading through RDMSR. > > Also introduce the SVM related constants at this occasion. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > RFC: Andrew promised to take care of the CPUID side of this; will need > re-basing over his work once available. > TBD: There are indications that the CPUID field used may be just 8 bits > wide. Getting there. The movement of CPU_POLICY into the migrate stream is a substantial step in the right direction. I've got half a mind to use RDPRU as the first real "need to opt-in" feature, seeing as there currently isn't an answer for how APERF/MPERF is virtualisation-safe in the first place, and how exposing it in userspace is any better. Perhaps better would be to make APERF/MPERF opt-in to begin with (so it starts ceasing to exist for VMs by default, but in a backwards compatible way), and then derive RDPRU as dependent on APERF/MPERF. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
If the hardware can handle accesses, we should allow it to do so. This
way we can expose EFRO to HVM guests, and "all" that's left for exposing
APERF/MPERF is to figure out how to handle writes to these MSRs. (Note
that the leaf 6 guest CPUID checks will evaluate to false for now, as
recalculate_misc() zaps the entire leaf for now.)
For TSC the intercepts are made mirror the RDTSC ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Make TSC intercepts mirror RDTSC ones. Re-base.
v3: New.
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -595,6 +595,7 @@ static void svm_cpuid_policy_changed(str
struct vmcb_struct *vmcb = svm->vmcb;
const struct cpuid_policy *cp = v->domain->arch.cpuid;
u32 bitmap = vmcb_get_exception_intercepts(vmcb);
+ unsigned int mode;
if ( opt_hvm_fep ||
(v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
@@ -607,6 +608,17 @@ static void svm_cpuid_policy_changed(str
/* Give access to MSR_PRED_CMD if the guest has been told about it. */
svm_intercept_msr(v, MSR_PRED_CMD,
cp->extd.ibpb ? MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW);
+
+ /* Allow direct reads from APERF/MPERF if permitted by the policy. */
+ mode = cp->basic.raw[6].c & CPUID6_ECX_APERFMPERF_CAPABILITY
+ ? MSR_INTERCEPT_WRITE : MSR_INTERCEPT_RW;
+ svm_intercept_msr(v, MSR_IA32_APERF, mode);
+ svm_intercept_msr(v, MSR_IA32_MPERF, mode);
+
+ /* Allow direct access to their r/o counterparts if permitted. */
+ mode = cp->extd.efro ? MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW;
+ svm_intercept_msr(v, MSR_APERF_RD_ONLY, mode);
+ svm_intercept_msr(v, MSR_MPERF_RD_ONLY, mode);
}
void svm_sync_vmcb(struct vcpu *v, enum vmcb_sync_state new_state)
@@ -860,7 +872,10 @@ static void svm_set_rdtsc_exiting(struct
{
general1_intercepts |= GENERAL1_INTERCEPT_RDTSC;
general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP;
+ svm_enable_intercept_for_msr(v, MSR_IA32_TSC);
}
+ else
+ svm_intercept_msr(v, MSR_IA32_TSC, MSR_INTERCEPT_WRITE);
vmcb_set_general1_intercepts(vmcb, general1_intercepts);
vmcb_set_general2_intercepts(vmcb, general2_intercepts);
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -108,6 +108,7 @@ static int construct_vmcb(struct vcpu *v
{
vmcb->_general1_intercepts |= GENERAL1_INTERCEPT_RDTSC;
vmcb->_general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP;
+ svm_intercept_msr(v, MSR_IA32_TSC, MSR_INTERCEPT_WRITE);
}
/* Guest segment limits. */
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1140,8 +1140,13 @@ static int construct_vmcs(struct vcpu *v
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_CS, VMX_MSR_RW);
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_ESP, VMX_MSR_RW);
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_EIP, VMX_MSR_RW);
+
+ if ( !(v->arch.hvm.vmx.exec_control & CPU_BASED_RDTSC_EXITING) )
+ vmx_clear_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
+
if ( paging_mode_hap(d) && (!is_iommu_enabled(d) || iommu_snoop) )
vmx_clear_msr_intercept(v, MSR_IA32_CR_PAT, VMX_MSR_RW);
+
if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) &&
(vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) )
vmx_clear_msr_intercept(v, MSR_IA32_BNDCFGS, VMX_MSR_RW);
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -584,6 +584,18 @@ static void vmx_cpuid_policy_changed(str
vmx_clear_msr_intercept(v, MSR_FLUSH_CMD, VMX_MSR_RW);
else
vmx_set_msr_intercept(v, MSR_FLUSH_CMD, VMX_MSR_RW);
+
+ /* Allow direct reads from APERF/MPERF if permitted by the policy. */
+ if ( cp->basic.raw[6].c & CPUID6_ECX_APERFMPERF_CAPABILITY )
+ {
+ vmx_clear_msr_intercept(v, MSR_IA32_APERF, VMX_MSR_R);
+ vmx_clear_msr_intercept(v, MSR_IA32_MPERF, VMX_MSR_R);
+ }
+ else
+ {
+ vmx_set_msr_intercept(v, MSR_IA32_APERF, VMX_MSR_R);
+ vmx_set_msr_intercept(v, MSR_IA32_MPERF, VMX_MSR_R);
+ }
}
int vmx_guest_x86_mode(struct vcpu *v)
@@ -1249,7 +1261,12 @@ static void vmx_set_rdtsc_exiting(struct
vmx_vmcs_enter(v);
v->arch.hvm.vmx.exec_control &= ~CPU_BASED_RDTSC_EXITING;
if ( enable )
+ {
v->arch.hvm.vmx.exec_control |= CPU_BASED_RDTSC_EXITING;
+ vmx_set_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
+ }
+ else
+ vmx_clear_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
vmx_update_cpu_exec_control(v);
vmx_vmcs_exit(v);
}
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -242,7 +242,7 @@ XEN_CPUFEATURE(MOVDIR64B, 6*32+28) /
/* AMD-defined CPU features, CPUID level 0x80000007.edx, word 7 */
XEN_CPUFEATURE(ITSC, 7*32+ 8) /* Invariant TSC */
-XEN_CPUFEATURE(EFRO, 7*32+10) /* APERF/MPERF Read Only interface */
+XEN_CPUFEATURE(EFRO, 7*32+10) /*S APERF/MPERF Read Only interface */
/* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
XEN_CPUFEATURE(CLZERO, 8*32+ 0) /*A CLZERO instruction */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
> From: Jan Beulich <jbeulich@suse.com> > Sent: Tuesday, January 7, 2020 12:39 AM > > If the hardware can handle accesses, we should allow it to do so. This > way we can expose EFRO to HVM guests, and "all" that's left for exposing > APERF/MPERF is to figure out how to handle writes to these MSRs. (Note > that the leaf 6 guest CPUID checks will evaluate to false for now, as > recalculate_misc() zaps the entire leaf for now.) > > For TSC the intercepts are made mirror the RDTSC ones. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 19.01.2020 03:44, Tian, Kevin wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, January 7, 2020 12:39 AM
>>
>> If the hardware can handle accesses, we should allow it to do so. This
>> way we can expose EFRO to HVM guests, and "all" that's left for exposing
>> APERF/MPERF is to figure out how to handle writes to these MSRs. (Note
>> that the leaf 6 guest CPUID checks will evaluate to false for now, as
>> recalculate_misc() zaps the entire leaf for now.)
>>
>> For TSC the intercepts are made mirror the RDTSC ones.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Thanks. I assume you've seen Andrew's comment, and hence I take it
that the R-b also applies to the adjusted version (not posted yet):
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1140,8 +1140,13 @@ static int construct_vmcs(struct vcpu *v
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_CS, VMX_MSR_RW);
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_ESP, VMX_MSR_RW);
vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_EIP, VMX_MSR_RW);
+
+ if ( !(v->arch.hvm.vmx.exec_control & CPU_BASED_RDTSC_EXITING) )
+ vmx_clear_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
+
if ( paging_mode_hap(d) && (!is_iommu_enabled(d) || iommu_snoop) )
vmx_clear_msr_intercept(v, MSR_IA32_CR_PAT, VMX_MSR_RW);
+
if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) &&
(vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) )
vmx_clear_msr_intercept(v, MSR_IA32_BNDCFGS, VMX_MSR_RW);
plus this extra vmx.c hunk:
@@ -1249,7 +1261,12 @@ static void vmx_set_rdtsc_exiting(struct
vmx_vmcs_enter(v);
v->arch.hvm.vmx.exec_control &= ~CPU_BASED_RDTSC_EXITING;
if ( enable )
+ {
v->arch.hvm.vmx.exec_control |= CPU_BASED_RDTSC_EXITING;
+ vmx_set_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
+ }
+ else
+ vmx_clear_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
vmx_update_cpu_exec_control(v);
vmx_vmcs_exit(v);
}
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, January 20, 2020 4:33 PM
>
> On 19.01.2020 03:44, Tian, Kevin wrote:
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Tuesday, January 7, 2020 12:39 AM
> >>
> >> If the hardware can handle accesses, we should allow it to do so. This
> >> way we can expose EFRO to HVM guests, and "all" that's left for exposing
> >> APERF/MPERF is to figure out how to handle writes to these MSRs. (Note
> >> that the leaf 6 guest CPUID checks will evaluate to false for now, as
> >> recalculate_misc() zaps the entire leaf for now.)
> >>
> >> For TSC the intercepts are made mirror the RDTSC ones.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>
> Thanks. I assume you've seen Andrew's comment, and hence I take it
> that the R-b also applies to the adjusted version (not posted yet):
sorry I'm not sure which comment is referred here. If you will
anyway send out a new version, please drop my R-b and I will
double check again though the below version alone looks good.
>
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1140,8 +1140,13 @@ static int construct_vmcs(struct vcpu *v
> vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_CS, VMX_MSR_RW);
> vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_ESP, VMX_MSR_RW);
> vmx_clear_msr_intercept(v, MSR_IA32_SYSENTER_EIP, VMX_MSR_RW);
> +
> + if ( !(v->arch.hvm.vmx.exec_control & CPU_BASED_RDTSC_EXITING) )
> + vmx_clear_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
> +
> if ( paging_mode_hap(d) && (!is_iommu_enabled(d) || iommu_snoop) )
> vmx_clear_msr_intercept(v, MSR_IA32_CR_PAT, VMX_MSR_RW);
> +
> if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) &&
> (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) )
> vmx_clear_msr_intercept(v, MSR_IA32_BNDCFGS, VMX_MSR_RW);
>
> plus this extra vmx.c hunk:
>
> @@ -1249,7 +1261,12 @@ static void vmx_set_rdtsc_exiting(struct
> vmx_vmcs_enter(v);
> v->arch.hvm.vmx.exec_control &= ~CPU_BASED_RDTSC_EXITING;
> if ( enable )
> + {
> v->arch.hvm.vmx.exec_control |= CPU_BASED_RDTSC_EXITING;
> + vmx_set_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
> + }
> + else
> + vmx_clear_msr_intercept(v, MSR_IA32_TSC, VMX_MSR_R);
> vmx_update_cpu_exec_control(v);
> vmx_vmcs_exit(v);
> }
>
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
The dependency on a new EFER bit implies that we need to set that bit
ourselves in order to be able to successfully invoke the insn.
Also once again introduce the SVM related constants at this occasion.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC: The exact meaning of the PM stating "any errors encountered by
those stores have been signaled to associated error logging
resources" is unclear. Depending on what this entails, blindly
enabling EFER.MCOMMIT may not be a good idea. Hence the RFC.
---
v4: New.
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -260,6 +260,7 @@ int libxl_cpuid_parse_config(libxl_cpuid
{"clzero", 0x80000008, NA, CPUID_REG_EBX, 0, 1},
{"rstr-fp-err-ptrs", 0x80000008, NA, CPUID_REG_EBX, 2, 1},
{"rdpru", 0x80000008, NA, CPUID_REG_EBX, 4, 1},
+ {"mcommit", 0x80000008, NA, CPUID_REG_EBX, 8, 1},
{"wbnoinvd", 0x80000008, NA, CPUID_REG_EBX, 9, 1},
{"ibpb", 0x80000008, NA, CPUID_REG_EBX, 12, 1},
{"ppin", 0x80000008, NA, CPUID_REG_EBX, 23, 1},
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -149,7 +149,7 @@ static const char *const str_e8b[32] =
[ 4] = "rdpru",
- /* [ 8] */ [ 9] = "wbnoinvd",
+ [ 8] = "mcommit", [ 9] = "wbnoinvd",
[12] = "ibpb",
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -800,6 +800,9 @@ static void init_amd(struct cpuinfo_x86
wrmsr(MSR_K7_HWCR, l, h);
}
+ if (cpu_has(c, X86_FEATURE_MCOMMIT))
+ write_efer(read_efer() | EFER_MCOMMIT);
+
/* Prevent TSC drift in non single-processor, single-core platforms. */
if ((smp_processor_id() == 1) && !cpu_has(c, X86_FEATURE_ITSC))
disable_c1_ramping();
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1874,6 +1874,7 @@ in_protmode(
#define vcpu_has_fma4() (ctxt->cpuid->extd.fma4)
#define vcpu_has_tbm() (ctxt->cpuid->extd.tbm)
#define vcpu_has_clzero() (ctxt->cpuid->extd.clzero)
+#define vcpu_has_mcommit() (ctxt->cpuid->extd.mcommit)
#define vcpu_has_rdpru() (ctxt->cpuid->extd.rdpru)
#define vcpu_has_wbnoinvd() (ctxt->cpuid->extd.wbnoinvd)
@@ -5663,6 +5664,28 @@ x86_emulate(
_regs.r(cx) = (uint32_t)msr_val;
goto rdtsc;
+ case 0xfa: /* monitorx / mcommit */
+ if ( vex.pfx == vex_f3 )
+ {
+ bool cf;
+
+ host_and_vcpu_must_have(mcommit);
+ if ( !ops->read_msr ||
+ ops->read_msr(MSR_EFER, &msr_val, ctxt) != X86EMUL_OKAY )
+ msr_val = 0;
+ generate_exception_if(!(msr_val & EFER_MCOMMIT), EXC_UD);
+ memcpy(get_stub(stub),
+ ((uint8_t[]){ 0xf3, 0x0f, 0x01, 0xfa, 0xc3 }), 5);
+ _regs.eflags &= ~EFLAGS_MASK;
+ invoke_stub("", ASM_FLAG_OUT(, "setc %[cf]"),
+ [cf] ASM_FLAG_OUT("=@ccc", "=qm") (cf) : "i" (0));
+ if ( cf )
+ _regs.eflags |= X86_EFLAGS_CF;
+ put_stub(stub);
+ goto done;
+ }
+ goto unrecognized_insn;
+
case 0xfc: /* clzero */
{
unsigned long zero = 0;
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -130,6 +130,9 @@
#define cpu_has_avx512_4fmaps boot_cpu_has(X86_FEATURE_AVX512_4FMAPS)
#define cpu_has_tsx_force_abort boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)
+/* CPUID level 0x80000008.ebx */
+#define cpu_has_mcommit boot_cpu_has(X86_FEATURE_MCOMMIT)
+
/* Synthesized. */
#define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
#define cpu_has_cpuid_faulting boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
--- a/xen/include/asm-x86/hvm/svm/vmcb.h
+++ b/xen/include/asm-x86/hvm/svm/vmcb.h
@@ -80,6 +80,11 @@ enum GenericIntercept2bits
GENERAL2_INTERCEPT_RDPRU = 1 << 14,
};
+/* general 3 intercepts */
+enum GenericIntercept3bits
+{
+ GENERAL3_INTERCEPT_MCOMMIT = 1 << 3,
+};
/* control register intercepts */
enum CRInterceptBits
@@ -302,6 +307,7 @@ enum VMEXIT_EXITCODE
VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */
VMEXIT_XSETBV = 141, /* 0x8d */
VMEXIT_RDPRU = 142, /* 0x8e */
+ VMEXIT_MCOMMIT = 163, /* 0xa3 */
VMEXIT_NPF = 1024, /* 0x400, nested paging fault */
VMEXIT_INVALID = -1
};
@@ -408,7 +414,8 @@ struct vmcb_struct {
u32 _exception_intercepts; /* offset 0x08 - cleanbit 0 */
u32 _general1_intercepts; /* offset 0x0C - cleanbit 0 */
u32 _general2_intercepts; /* offset 0x10 - cleanbit 0 */
- u32 res01[10];
+ u32 _general3_intercepts; /* offset 0x14 - cleanbit 0 */
+ u32 res01[9];
u16 _pause_filter_thresh; /* offset 0x3C - cleanbit 0 */
u16 _pause_filter_count; /* offset 0x3E - cleanbit 0 */
u64 _iopm_base_pa; /* offset 0x40 - cleanbit 1 */
@@ -592,6 +599,7 @@ VMCB_ACCESSORS(dr_intercepts, intercepts
VMCB_ACCESSORS(exception_intercepts, intercepts)
VMCB_ACCESSORS(general1_intercepts, intercepts)
VMCB_ACCESSORS(general2_intercepts, intercepts)
+VMCB_ACCESSORS(general3_intercepts, intercepts)
VMCB_ACCESSORS(pause_filter_count, intercepts)
VMCB_ACCESSORS(pause_filter_thresh, intercepts)
VMCB_ACCESSORS(tsc_offset, intercepts)
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -21,6 +21,7 @@
#define _EFER_NX 11 /* No execute enable */
#define _EFER_SVME 12 /* AMD: SVM enable */
#define _EFER_FFXSE 14 /* AMD: Fast FXSAVE/FXRSTOR enable */
+#define _EFER_MCOMMIT 17 /* AMD: MCOMMIT insn enable */
#define EFER_SCE (1<<_EFER_SCE)
#define EFER_LME (1<<_EFER_LME)
@@ -28,9 +29,10 @@
#define EFER_NX (1<<_EFER_NX)
#define EFER_SVME (1<<_EFER_SVME)
#define EFER_FFXSE (1<<_EFER_FFXSE)
+#define EFER_MCOMMIT (1<<_EFER_MCOMMIT)
#define EFER_KNOWN_MASK (EFER_SCE | EFER_LME | EFER_LMA | EFER_NX | \
- EFER_SVME | EFER_FFXSE)
+ EFER_SVME | EFER_FFXSE | EFER_MCOMMIT)
#define MSR_INTEL_CORE_THREAD_COUNT 0x00000035
#define MSR_CTC_THREAD_MASK 0x0000ffff
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -248,6 +248,7 @@ XEN_CPUFEATURE(EFRO, 7*32+10) /
XEN_CPUFEATURE(CLZERO, 8*32+ 0) /*A CLZERO instruction */
XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
XEN_CPUFEATURE(RDPRU, 8*32+ 4) /*A RDPRU instruction */
+XEN_CPUFEATURE(MCOMMIT, 8*32+ 8) /*A MCOMMIT instruction */
XEN_CPUFEATURE(WBNOINVD, 8*32+ 9) /* WBNOINVD instruction */
XEN_CPUFEATURE(IBPB, 8*32+12) /*A IBPB support only (no IBRS, used by AMD) */
XEN_CPUFEATURE(AMD_PPIN, 8*32+23) /* Protected Processor Inventory Number */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06/01/2020 16:39, Jan Beulich wrote: > The dependency on a new EFER bit implies that we need to set that bit > ourselves in order to be able to successfully invoke the insn. > > Also once again introduce the SVM related constants at this occasion. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > RFC: The exact meaning of the PM stating "any errors encountered by > those stores have been signaled to associated error logging > resources" is unclear. Depending on what this entails, blindly > enabling EFER.MCOMMIT may not be a good idea. Hence the RFC. From what I gather, this is only going to be useful in combination with future features. While I have no specific concerns code wise, I'd recommend deferring the patch until we have a better idea of how it is supposed to be used. (Especially given the PCOMMIT debacle on Intel.) ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06.01.2020 20:45, Andrew Cooper wrote: > On 06/01/2020 16:39, Jan Beulich wrote: >> The dependency on a new EFER bit implies that we need to set that bit >> ourselves in order to be able to successfully invoke the insn. >> >> Also once again introduce the SVM related constants at this occasion. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> RFC: The exact meaning of the PM stating "any errors encountered by >> those stores have been signaled to associated error logging >> resources" is unclear. Depending on what this entails, blindly >> enabling EFER.MCOMMIT may not be a good idea. Hence the RFC. > > From what I gather, this is only going to be useful in combination with > future features. > > While I have no specific concerns code wise, I'd recommend deferring the > patch until we have a better idea of how it is supposed to be used. Right - hence the RFC. I have a supposed-to-be-used inquiry pending with AMD. > (Especially given the PCOMMIT debacle on Intel.) ;-) Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.