Re: [PATCH v2 6/8] evtchn: convert vIRQ lock to an r/w one

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 20/10/2020 15:10, Jan Beulich wrote:
> There's no need to serialize all sending of vIRQ-s; all that's needed
> is serialization against the closing of the respective event channels
> (so far by means of a barrier). To facilitate the conversion, switch to
> an ordinary write locked region in evtchn_close().
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Don't introduce/use rw_barrier() here. Add comment to
>      evtchn_bind_virq(). Re-base.
>
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -160,7 +160,7 @@ struct vcpu *vcpu_create(struct domain *
>       v->vcpu_id = vcpu_id;
>       v->dirty_cpu = VCPU_CPU_CLEAN;
>   
> -    spin_lock_init(&v->virq_lock);
> +    rwlock_init(&v->virq_lock);
>   
>       tasklet_init(&v->continue_hypercall_tasklet, NULL, NULL);
>   
> --- 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.
Cheers,

--
Julien Grall