Re: [PATCH v2 5/8] tools/hvmloader: Retrieve (x2)APIC IDs from the APs themselves

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On 27/05/2024 09:52, Roger Pau Monné wrote:
>> My intent here was to prevent the compiler omitting the write (though in
>> practice it didn't). I think the barrier is still required to prevent
>> reordering according to the spec.
> 
> Yes, my understating is that you added the ACCESS_ONCE() to possibly
> prevent the compiler from re-ordering the CPU_TO_X2APICID[ap_cpuid]) =
> read_apic_id() after the 'hlt' loop?

Yes, but not only that. Also preventing the compiler from eliding the
write altogether on the basis that it's not read after within the
function. I have suffered that particular behaviour on older versions of
GCC and it stung very much.

> 
> AFAICT the expressions in the `for` statement are considered sequence
> points, and the calling `read_apic_id()` could have side-effects, so
> the compiler won't be able to elide or move the setting of
> CPU_TO_X2APICID[] due to all side-effects of previous evaluations must
> be complete at sequence points.
> > I'm fine with leaving the wmb() + ACCESS_ONCE().
> 
> Thanks, Roger.

Cheers,
Alejandro