On 06/01/2020 16:18, Jan Beulich wrote:
> On 06.01.2020 17:09, Andrew Cooper wrote:
>> On 06/01/2020 15:34, Jan Beulich wrote:
>>> --- a/xen/include/asm-x86/page.h
>>> +++ b/xen/include/asm-x86/page.h
>>> @@ -55,6 +55,16 @@
>>> #define l4e_write(l4ep, l4e) \
>>> pte_write(&l4e_get_intpte(*(l4ep)), l4e_get_intpte(l4e))
>>>
>>> +/* Type-correct ACCESS_ONCE() wrappers for PTE accesses. */
>>> +#define l1e_access_once(l1e)
>>> (*container_of(&ACCESS_ONCE(l1e_get_intpte(l1e)), \
>>> + volatile l1_pgentry_t, l1))
>>> +#define l2e_access_once(l2e)
>>> (*container_of(&ACCESS_ONCE(l2e_get_intpte(l2e)), \
>>> + volatile l2_pgentry_t, l2))
>>> +#define l3e_access_once(l3e)
>>> (*container_of(&ACCESS_ONCE(l3e_get_intpte(l3e)), \
>>> + volatile l3_pgentry_t, l3))
>>> +#define l4e_access_once(l4e)
>>> (*container_of(&ACCESS_ONCE(l4e_get_intpte(l4e)), \
>>> + volatile l4_pgentry_t, l4))
>> What's wrong with l?e_read_atomic() which already exist, and are already
>> used elsewhere?
> I did consider going that route, but predicted you would object to its
> use here: Iirc you've previously voiced opinion in this direction
> (perhaps not on the page table accessors themselves but the underlying
> {read,write}_u<N>_atomic()).
>
>> If nothing, then Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
>> to save another round of posting.
> Let's get the above clarified - I'll be happy to switch to
> l<N>e_read_atomic() if that's fine by you.
I'd definitely prefer reusing l?e_read_atomic() than introducing a new
set of helpers.
I have two key issues with the general _atomic() infrastructure.
First, the term is overloaded for two very different things. We use it
both for "don't subdivide this read/write" and "use a locked update",
where the latter is what is expected by the name "atomic".
Second, and specifically with {read,write}_u<N>_atomic(), it is their
construction using macros which makes them impossible to locate in the
source code. This can trivially be fixed by not using macros. (If it
were up to me, the use of ## would be disallowed in general, because it
does very little but to obfuscate code.)
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
