On 07/12/2018 12:12, Jan Beulich wrote:
>> @@ -244,19 +240,20 @@ long do_update_descriptor(uint64_t pa, uint64_t desc)
>>          break;
>>      }
>>  
>> -    paging_mark_dirty(currd, _mfn(mfn));
>> +    paging_mark_dirty(currd, mfn);
>>  
>>      /* All is good so make the update. */
>> -    gdt_pent = map_domain_page(_mfn(mfn));
>> -    write_atomic((uint64_t *)&gdt_pent[offset], *(uint64_t *)&d);
>> -    unmap_domain_page(gdt_pent);
>> +    entry = map_domain_page(mfn) + (gaddr & ~PAGE_MASK);
>> +    ACCESS_ONCE(entry->raw) = d.raw;
> Why not "ACCESS_ONCE(*entry) = d;"? I'm having trouble to
> understand why the macro insists on using scalar types (there's
> no comment there explaining the need).

GCC 4.x (6 and 7 iirc) have a bug where the volatile cast doesn't get
applied to structure members.  This case I think it would be safe as the
write can't be hoisted before the map_domain_page(), but this bug did
result in incorrect compiled code in Linux.

~Andrew

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to