Re: [Xen-devel] [PATCH v10 4/6] VT-d: introduce update_irte to update irte safely

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.03.17 at 02:52, <chao.gao@xxxxxxxxx> wrote:
> On Thu, Mar 16, 2017 at 04:29:29AM -0600, Jan Beulich wrote:
>>>>> On 15.03.17 at 23:39, <chao.gao@xxxxxxxxx> wrote:
>>> On Wed, Mar 15, 2017 at 10:48:25AM -0600, Jan Beulich wrote:
>>>>>>> On 15.03.17 at 06:11, <chao.gao@xxxxxxxxx> wrote:
>>>>> +        /*
>>>>> +         * The following method to update IRTE is safe on condition that
>>>>> +         * only the high qword or the low qword is to be updated.
>>>>> +         * If entire IRTE is to be updated, callers should make sure the
>>>>> +         * IRTE is not in use.
>>>>> +         */
>>>>> +        entry->lo = new_ire->lo;
>>>>> +        entry->hi = new_ire->hi;
>>>>
>>>>How is this any better than structure assignment? Furthermore
>>> 
>>> Indeed, not better. when using structure assignment, the assembly code is
>>> 48 8b 06                mov    (%rsi),%rax                    
>>> 48 8b 56 08             mov    0x8(%rsi),%rdx                 
>>> 48 89 07                mov    %rax,(%rdi)                    
>>> 48 89 57 08             mov    %rdx,0x8(%rdi)
>>> Using the code above, the assembly code is
>>> 48 8b 06                mov    (%rsi),%rax                  
>>> 48 89 07                mov    %rax,(%rdi)                    
>>> 48 8b 46 08             mov    0x8(%rsi),%rax                 
>>> 48 89 47 08             mov    %rax,0x8(%rdi)
>>> 
>>> I thought structure assignment maybe ultilize memcpy considering structure
>>> of a big size, so I made this change. I will change this back. Although
>>> that, this patch is trying to make the change safer when cmpxchg16() is
>>> supported. 
>>
>>Perhaps you've really meant to use write_atomic()?
> 
> I don't understand what you mean. But I think write_atomic may be not related
> to the problem how to update a 16 byte memory atomically if cmpxchg16() is not
> supported.

Of course not, but you were concerned about memcpy() being
called by the compiler. I.e. I did assume that when doing the
2x8-byte update you would want to have them carried out as
two 8-byte writes, instead of possibly being broken up further
by the compiler.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel