From: Razvan Cojocaru <rcojocaru@bitdefender.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: tim@xen.org, wei.liu2@citrix.com, george.dunlap@eu.citrix.com,
andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com,
xen-devel@lists.xen.org, paul.durrant@citrix.com,
david.vrabel@citrix.com, keir@xen.org
Subject: Re: [for-4.7] x86/emulate: synchronize LOCKed instruction emulation
Date: Mon, 18 Apr 2016 15:14:04 +0300 [thread overview]
Message-ID: <5714CF8C.3040408@bitdefender.com> (raw)
In-Reply-To: <570FCE8402000078000E67F2@prv-mh.provo.novell.com>
On 04/14/2016 07:08 PM, Jan Beulich wrote:
>>>> Razvan Cojocaru <rcojocaru@bitdefender.com> 04/14/16 5:45 PM >>>
>> On 04/14/2016 06:40 PM, Jan Beulich wrote:
>>> To be honest, just having remembered that we do the write back for locked
>>> instructions using CMPXCHG, I'd first of all like to see a proper description
>>> of "the _whole_ issue".
>>
>> I believe at least part of the issue has to do with the comment on line
>> 1013 from xen/arch/x86/hvm/emulate.c:
>>
> >994 static int hvmemul_cmpxchg(
> >995 enum x86_segment seg,
> >996 unsigned long offset,
> >997 void *p_old,
> >998 void *p_new,
> >999 unsigned int bytes,
>> 1000 struct x86_emulate_ctxt *ctxt)
>> 1001 {
>> 1002 struct hvm_emulate_ctxt *hvmemul_ctxt =
>> 1003 container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>> 1004
>> 1005 if ( unlikely(hvmemul_ctxt->set_context) )
>> 1006 {
>> 1007 int rc = set_context_data(p_new, bytes);
>> 1008
>> 1009 if ( rc != X86EMUL_OKAY )
>> 1010 return rc;
>> 1011 }
>> 1012
>> 1013 /* Fix this in case the guest is really relying on r-m-w atomicity. */
>> 1014 return hvmemul_write(seg, offset, p_new, bytes, ctxt);
>> 1015 }
>
> Ah, so _that's_ where the problem wants to be fixed then (leaving - afaict -
> PV emulation paths completely unaffected).
That's what I had hoped too, an early version of this patch simply used
a spinlock that locked / unlock on entry / exit of hvmemul_cmpxchg().
Even with this patch, I've just tried it again with all ops->smp_lock()
/ ops->smp_unlock() calls commented out in x86_emulate(), and
hvmemul_cmpxchg() modified as follows:
994 static int hvmemul_cmpxchg(
995 enum x86_segment seg,
996 unsigned long offset,
997 void *p_old,
998 void *p_new,
999 unsigned int bytes,
1000 struct x86_emulate_ctxt *ctxt)
1001 {
1002 struct hvm_emulate_ctxt *hvmemul_ctxt =
1003 container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
1004 int rc;
1005
1006 emulate_smp_lock(1);
1007
1008 if ( unlikely(hvmemul_ctxt->set_context) )
1009 {
1010 int rc = set_context_data(p_new, bytes);
1011
1012 if ( rc != X86EMUL_OKAY )
1013 return rc;
1014 }
1015
1016 /* Fix this in case the guest is really relying on r-m-w
atomicity. */
1017 rc = hvmemul_write(seg, offset, p_new, bytes, ctxt);
1018
1019 emulate_smp_unlock(1);
1020
1021 return rc;
1022 }
Unfortunately, with just this the guest still hangs, while with read and
write locking in x86_emulate() it does not.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-04-18 12:14 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-13 12:26 [for-4.7] x86/emulate: synchronize LOCKed instruction emulation Razvan Cojocaru
2016-04-14 4:35 ` Jan Beulich
2016-04-14 5:56 ` Razvan Cojocaru
2016-04-14 6:09 ` Juergen Gross
2016-04-14 6:31 ` Razvan Cojocaru
2016-04-14 7:46 ` Juergen Gross
2016-04-14 8:01 ` Andrew Cooper
2016-04-14 8:18 ` Juergen Gross
2016-04-14 8:25 ` Razvan Cojocaru
2016-04-14 8:07 ` Andrew Cooper
2016-04-14 8:09 ` Razvan Cojocaru
2016-04-14 9:08 ` Razvan Cojocaru
2016-04-14 15:33 ` Jan Beulich
2016-04-14 15:44 ` Jan Beulich
2016-04-14 16:00 ` Razvan Cojocaru
2016-04-14 16:11 ` Jan Beulich
2016-04-14 8:51 ` Razvan Cojocaru
2016-04-14 15:31 ` Jan Beulich
2016-04-14 15:40 ` Razvan Cojocaru
2016-04-14 10:35 ` David Vrabel
2016-04-14 11:43 ` Razvan Cojocaru
2016-04-14 15:40 ` Jan Beulich
2016-04-14 15:45 ` Razvan Cojocaru
2016-04-14 16:08 ` Jan Beulich
2016-04-18 12:14 ` Razvan Cojocaru [this message]
2016-04-18 16:45 ` Jan Beulich
2016-04-19 11:01 ` Razvan Cojocaru
2016-04-19 16:35 ` Jan Beulich
2016-04-26 16:03 ` George Dunlap
2016-04-26 17:23 ` Razvan Cojocaru
2016-04-26 17:39 ` Andrew Cooper
2016-04-27 6:25 ` Jan Beulich
2016-04-27 7:36 ` Andrew Cooper
2016-04-27 6:22 ` Jan Beulich
2016-04-27 7:14 ` Razvan Cojocaru
2016-05-03 14:20 ` Razvan Cojocaru
2016-05-03 14:30 ` Jan Beulich
2016-05-03 14:41 ` Razvan Cojocaru
2016-05-03 15:13 ` Jan Beulich
2016-05-04 11:32 ` Razvan Cojocaru
2016-05-04 13:42 ` Jan Beulich
2016-05-05 9:25 ` Razvan Cojocaru
2016-05-05 16:38 ` Jan Beulich
2016-04-14 15:45 ` Andrew Cooper
2016-04-14 16:09 ` Jan Beulich
2016-05-13 15:27 ` Wei Liu
2016-05-13 15:51 ` 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=5714CF8C.3040408@bitdefender.com \
--to=rcojocaru@bitdefender.com \
--cc=andrew.cooper3@citrix.com \
--cc=david.vrabel@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=keir@xen.org \
--cc=paul.durrant@citrix.com \
--cc=tim@xen.org \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xen.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).