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
