Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 04/14/2016 07:08 PM, Jan Beulich wrote:
>>>> Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel