Re: [PATCH 1/4] xen: Introduce non-broken hypercalls for the p2m pool size

[Jan Beulich <jbeulich@suse.com>]

On 26.10.2022 23:24, Julien Grall wrote:
> On 26/10/2022 20:22, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/mm/hap/hap.c
>>>> +++ b/xen/arch/x86/mm/hap/hap.c
>>>> @@ -345,6 +345,16 @@ unsigned int hap_get_allocation(struct domain *d)
>>>>               + ((pg & ((1 << (20 - PAGE_SHIFT)) - 1)) ? 1 : 0));
>>>>   }
>>>>   
>>>> +int hap_get_allocation_bytes(struct domain *d, uint64_t *size)
>>>> +{
>>>> +    unsigned long pages = (d->arch.paging.hap.total_pages +
>>>> +                           d->arch.paging.hap.p2m_pages);
>>> Unlike for Arm no ACCESS_ONCE() here? Also the addition can in
>>> principle overflow, because being done only in 32 bits.
>>
>> I'm not actually convinced ARM needs ACCESS_ONCE() to begin with.  I
>> can't see any legal transformation of that logic which could result in a
>> torn load.
> 
> AFAIU, ACCESS_ONCE() is not only about torn load but also making sure 
> that the compiler will only read the value once.
> 
> When LTO is enabled (not yet supported) in Xen, can we guarantee the 
> compiler will not try to access total_pages twice (obviously it would be 
> caller dependent)?

Aren't all accesses (supposed to be) under paging lock? At which point
there's no issue with multiple (or torn) accesses?

Jan