| Message ID | 6e7500d2-262c-29c7-b9be-3fc9be26d198@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86emul: further work | expand |
On 05/05/2020 09:14, Jan Beulich wrote: > --- a/xen/tools/gen-cpuid.py > +++ b/xen/tools/gen-cpuid.py > @@ -284,6 +284,9 @@ def crunch_numbers(state): > # as dependent features simplifies Xen's logic, and prevents the guest > # from seeing implausible configurations. > IBRSB: [STIBP, SSBD], > + > + # In principle the TSXLDTRK insns could also be considered independent. > + RTM: [TSXLDTRK], Why the link? There is no relevant interaction AFAICT. ~Andrew
On 07.05.2020 22:13, Andrew Cooper wrote: > On 05/05/2020 09:14, Jan Beulich wrote: >> --- a/xen/tools/gen-cpuid.py >> +++ b/xen/tools/gen-cpuid.py >> @@ -284,6 +284,9 @@ def crunch_numbers(state): >> # as dependent features simplifies Xen's logic, and prevents the guest >> # from seeing implausible configurations. >> IBRSB: [STIBP, SSBD], >> + >> + # In principle the TSXLDTRK insns could also be considered independent. >> + RTM: [TSXLDTRK], > > Why the link? There is no relevant interaction AFAICT. Do the insns make any sense without TSX? Anyway - hence the comment, and if you're convinced the connection does not need making, I'd be okay dropping it. I would assume though that we'd better hide TSXLDTRK whenever we hide RTM, which is most easily achieved by having a connection here. Jan
On 08/05/2020 08:38, Jan Beulich wrote: > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > On 07.05.2020 22:13, Andrew Cooper wrote: >> On 05/05/2020 09:14, Jan Beulich wrote: >>> --- a/xen/tools/gen-cpuid.py >>> +++ b/xen/tools/gen-cpuid.py >>> @@ -284,6 +284,9 @@ def crunch_numbers(state): >>> # as dependent features simplifies Xen's logic, and prevents the guest >>> # from seeing implausible configurations. >>> IBRSB: [STIBP, SSBD], >>> + >>> + # In principle the TSXLDTRK insns could also be considered independent. >>> + RTM: [TSXLDTRK], >> Why the link? There is no relevant interaction AFAICT. > Do the insns make any sense without TSX? Anyway - hence the > comment, and if you're convinced the connection does not > need making, I'd be okay dropping it. I would assume though > that we'd better hide TSXLDTRK whenever we hide RTM, which > is most easily achieved by having a connection here. Actually - that is a very good point. I expect there will (or should) be an interaction with MSR_TSX_CTRL, as it has CPUID-hiding functionality. For now, could I ask you to not expose this to guests in this patch? For the emulator side of things alone I think this is ok (although looking over it a second time, we could really do with a comment in the code explaining that we're never in an RTM region, hence the nop behaviour). I'll follow up with Intel, and we can figure out the CPUID derivation details at a later point. If you're happy with this plan, then A-by to save a round trip. ~Andrew
On 08.05.2020 15:15, Andrew Cooper wrote: > On 08/05/2020 08:38, Jan Beulich wrote: >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. >> >> On 07.05.2020 22:13, Andrew Cooper wrote: >>> On 05/05/2020 09:14, Jan Beulich wrote: >>>> --- a/xen/tools/gen-cpuid.py >>>> +++ b/xen/tools/gen-cpuid.py >>>> @@ -284,6 +284,9 @@ def crunch_numbers(state): >>>> # as dependent features simplifies Xen's logic, and prevents the guest >>>> # from seeing implausible configurations. >>>> IBRSB: [STIBP, SSBD], >>>> + >>>> + # In principle the TSXLDTRK insns could also be considered independent. >>>> + RTM: [TSXLDTRK], >>> Why the link? There is no relevant interaction AFAICT. >> Do the insns make any sense without TSX? Anyway - hence the >> comment, and if you're convinced the connection does not >> need making, I'd be okay dropping it. I would assume though >> that we'd better hide TSXLDTRK whenever we hide RTM, which >> is most easily achieved by having a connection here. > > Actually - that is a very good point. I expect there will (or should) > be an interaction with MSR_TSX_CTRL, as it has CPUID-hiding functionality. > > For now, could I ask you to not expose this to guests in this patch? As per our irc discussion, I'd make it 'a' then instead of 'A'. Will need to wait for gen-cpuid.py to accept 'a' then. > For the emulator side of things alone I think this is ok (although > looking over it a second time, we could really do with a comment in the > code explaining that we're never in an RTM region, hence the nop behaviour). I've added /* * We're never in a transactional region when coming here * - nothing else to do. */ to both paths. > I'll follow up with Intel, and we can figure out the CPUID derivation > details at a later point. > > If you're happy with this plan, then A-by to save a round trip. Thanks. Jan
--- a/tools/libxl/libxl_cpuid.c +++ b/tools/libxl/libxl_cpuid.c @@ -208,6 +208,7 @@ int libxl_cpuid_parse_config(libxl_cpuid {"avx512-vnni", 0x00000007, 0, CPUID_REG_ECX, 11, 1}, {"avx512-bitalg",0x00000007, 0, CPUID_REG_ECX, 12, 1}, {"avx512-vpopcntdq",0x00000007,0,CPUID_REG_ECX, 14, 1}, + {"tsxldtrk", 0x00000007, 0, CPUID_REG_ECX, 16, 1}, {"rdpid", 0x00000007, 0, CPUID_REG_ECX, 22, 1}, {"cldemote", 0x00000007, 0, CPUID_REG_ECX, 25, 1}, --- a/tools/misc/xen-cpuid.c +++ b/tools/misc/xen-cpuid.c @@ -128,6 +128,7 @@ static const char *const str_7c0[32] = [10] = "vpclmulqdq", [11] = "avx512_vnni", [12] = "avx512_bitalg", [14] = "avx512_vpopcntdq", + [16] = "tsxldtrk", [22] = "rdpid", /* 24 */ [25] = "cldemote", --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -1921,6 +1921,7 @@ amd_like(const struct x86_emulate_ctxt * #define vcpu_has_avx512_vnni() (ctxt->cpuid->feat.avx512_vnni) #define vcpu_has_avx512_bitalg() (ctxt->cpuid->feat.avx512_bitalg) #define vcpu_has_avx512_vpopcntdq() (ctxt->cpuid->feat.avx512_vpopcntdq) +#define vcpu_has_tsxldtrk() (ctxt->cpuid->feat.tsxldtrk) #define vcpu_has_rdpid() (ctxt->cpuid->feat.rdpid) #define vcpu_has_movdiri() (ctxt->cpuid->feat.movdiri) #define vcpu_has_movdir64b() (ctxt->cpuid->feat.movdir64b) @@ -5668,6 +5669,20 @@ x86_emulate( host_and_vcpu_must_have(serialize); asm volatile ( ".byte 0x0f, 0x01, 0xe8" ); break; + case vex_f2: /* xsusldtrk */ + vcpu_must_have(tsxldtrk); + break; + default: + goto unimplemented_insn; + } + break; + + case 0xe9: + switch ( vex.pfx ) + { + case vex_f2: /* xresldtrk */ + vcpu_must_have(tsxldtrk); + break; default: goto unimplemented_insn; } --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -236,6 +236,7 @@ XEN_CPUFEATURE(VPCLMULQDQ, 6*32+10) / XEN_CPUFEATURE(AVX512_VNNI, 6*32+11) /*A Vector Neural Network Instrs */ XEN_CPUFEATURE(AVX512_BITALG, 6*32+12) /*A Support for VPOPCNT[B,W] and VPSHUFBITQMB */ XEN_CPUFEATURE(AVX512_VPOPCNTDQ, 6*32+14) /*A POPCNT for vectors of DW/QW */ +XEN_CPUFEATURE(TSXLDTRK, 6*32+16) /*A TSX load tracking suspend/resume insns */ 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 */ --- a/xen/tools/gen-cpuid.py +++ b/xen/tools/gen-cpuid.py @@ -284,6 +284,9 @@ def crunch_numbers(state): # as dependent features simplifies Xen's logic, and prevents the guest # from seeing implausible configurations. IBRSB: [STIBP, SSBD], + + # In principle the TSXLDTRK insns could also be considered independent. + RTM: [TSXLDTRK], } deep_features = tuple(sorted(deps.keys()))
There's nothing to be done by the emulator, as we unconditionally abort any XBEGIN. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v6: New.