On 30.10.2020 11:57, Julien Grall wrote:
> On 20/10/2020 15:10, Jan Beulich wrote:
>> --- a/xen/common/event_channel.c
>> +++ b/xen/common/event_channel.c
>> @@ -449,6 +449,13 @@ int evtchn_bind_virq(evtchn_bind_virq_t
>>
>> spin_unlock_irqrestore(&chn->lock, flags);
>>
>> + /*
>> + * If by any, the update of virq_to_evtchn[] would need guarding by
>> + * virq_lock, but since this is the last action here, there's no strict
>> + * need to acquire the lock. Hnece holding event_lock isn't helpful
>
> s/Hnece/Hence/
>
>> + * anymore at this point, but utilize that its unlocking acts as the
>> + * otherwise necessary smp_wmb() here.
>> + */
>> v->virq_to_evtchn[virq] = bind->port = port;
>
> I think all access to v->virq_to_evtchn[virq] should use ACCESS_ONCE()
> or {read, write}_atomic() to avoid any store/load tearing.
IOW you're suggesting this to be the subject of a separate patch?
I don't think such a conversion belongs here (nor even in this
series, seeing the much wider applicability of such a change
throughout the code base). Or are you seeing anything here which
would require such a conversion to be done as a prereq?
Jan
