From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
Paul Durrant <Paul.Durrant@citrix.com>, Wei Liu <wl@xen.org>,
RogerPau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 6/6] x86emul: support MOVDIR{I,64B} insns
Date: Wed, 28 Aug 2019 08:26:55 +0200 [thread overview]
Message-ID: <545c2be3-9641-d9ff-291d-a0bf85fb3dd3@suse.com> (raw)
In-Reply-To: <a7f91e8d-a550-73f8-9abc-33173a5be5e2@citrix.com>
On 27.08.2019 18:04, Andrew Cooper wrote:
> On 01/07/2019 12:58, Jan Beulich wrote:
>> @@ -9896,6 +9902,32 @@ x86_emulate(
>> : "0" ((uint32_t)src.val), "rm" (_regs.edx) );
>> break;
>>
>> + case X86EMUL_OPC_66(0x0f38, 0xf8): /* movdir64b r,m512 */
>> + 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);
>> + /* Ignore the non-temporal behavior for now. */
>> + fail_if(!ops->write);
>> + BUILD_BUG_ON(sizeof(*mmvalp) < 64);
>> + if ( (rc = ops->read(ea.mem.seg, ea.mem.off, mmvalp, 64,
>> + ctxt)) != X86EMUL_OKAY ||
>> + (rc = ops->write(x86_seg_es, src.val, mmvalp, 64,
>> + ctxt)) != X86EMUL_OKAY )
>> + goto done;
>> + state->simd_size = simd_none;
>> + sfence = true;
>> + 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;
>
> I'm not certain this gives the required atomicity. AFAICT, it degrades
> into ops->write(), which can end up with bytewise writes.
>
> I think we need to map the destination and issue an explicit mov
> instruction.
I don't think so, no - plain MOV has the same property (in particular
when not going through the cache), and also uses the ->write() hook.
It's the hook function that needs to behave properly for all of this
to be correct.
>> --- a/tools/tests/x86_emulator/x86-emulate.c
>> +++ b/tools/tests/x86_emulator/x86-emulate.c
>> @@ -76,6 +76,8 @@ bool emul_test_init(void)
>> cp.feat.adx = true;
>> cp.feat.avx512pf = cp.feat.avx512f;
>> cp.feat.rdpid = true;
>> + cp.feat.movdiri = true;
>> + cp.feat.movdir64b = true;
>> cp.extd.clzero = true;
>>
>> if ( cpu_has_xsave )
>> @@ -137,15 +139,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/MOVDIR* 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) | (1U << 28);
>
> I've just noticed, but we shouldn't be having both the named variables
> and these unnamed ones. Is their presence a rebasing issue around
> patches into the test suite?
emul_test_init() gained its current shape from fd35f32b4b, while
emul_test_cpuid() had been left untouched at that point. So I guess
it's more like a forgotten piece of conversion work. I'm unsure
though whether such a conversion is actually a good idea, since aiui
it would mean cloning at least guest_cpuid()'s first switch() into
this function, which is quite a bit more code than there is right
now. Perhaps (the common part of) that switch() could be morphed
into a library function ...
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-08-28 6:27 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-01 11:47 [Xen-devel] [PATCH 0/6] x86emul: further work Jan Beulich
2019-07-01 11:56 ` [Xen-devel] [PATCH 1/6] x86emul: generalize wbinvd() hook Jan Beulich
2019-07-02 10:22 ` Paul Durrant
2019-07-02 10:50 ` Jan Beulich
2019-07-02 10:53 ` Paul Durrant
2019-08-27 10:44 ` Andrew Cooper
2019-08-27 12:51 ` Andrew Cooper
2019-08-27 18:47 ` Andrew Cooper
2019-09-02 11:10 ` [Xen-devel] Ping: " Jan Beulich
2019-09-02 12:04 ` Paul Durrant
2019-07-01 11:56 ` [Xen-devel] [PATCH 2/6] x86emul: support WBNOINVD Jan Beulich
2019-07-02 10:38 ` Paul Durrant
2019-08-27 14:47 ` Andrew Cooper
2019-08-27 15:08 ` Jan Beulich
2019-08-27 16:45 ` Andrew Cooper
2019-08-28 11:42 ` Andrew Cooper
2019-07-01 11:56 ` [Xen-devel] [PATCH 3/6] x86emul: generalize invlpg() hook Jan Beulich
2019-07-02 10:47 ` Paul Durrant
2019-08-27 14:55 ` Andrew Cooper
2019-07-01 11:57 ` [Xen-devel] [PATCH 4/6] x86: move INVPCID_TYPE_* to x86-defns.h Jan Beulich
2019-07-02 10:49 ` Paul Durrant
2019-08-27 14:57 ` Andrew Cooper
2019-07-01 11:57 ` [Xen-devel] [PATCH 5/6] x86emul: support INVPCID Jan Beulich
2019-07-02 12:54 ` Paul Durrant
2019-08-27 15:31 ` Andrew Cooper
2019-08-27 15:53 ` Jan Beulich
2019-08-27 17:27 ` Andrew Cooper
2019-08-28 7:14 ` Jan Beulich
2019-08-28 11:33 ` Andrew Cooper
2019-07-01 11:58 ` [Xen-devel] [PATCH 6/6] x86emul: support MOVDIR{I,64B} insns Jan Beulich
2019-08-27 16:04 ` Andrew Cooper
2019-08-28 6:26 ` Jan Beulich [this message]
2019-08-28 11:51 ` Andrew Cooper
2019-08-28 12:19 ` Jan Beulich
2019-08-15 14:24 ` [Xen-devel] [PATCH 0/6] x86emul: further work Andrew Cooper
2019-08-27 8:37 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=545c2be3-9641-d9ff-291d-a0bf85fb3dd3@suse.com \
--to=jbeulich@suse.com \
--cc=Paul.Durrant@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=roger.pau@citrix.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).