RE: [PATCH 05/12] evtchn/sched: reject poll requests for unusable ports

From: Paul Durrant <xadimgnik@gmail.com>
To: "'Jan Beulich'" <jbeulich@suse.com>, <xen-devel@lists.xenproject.org>
Cc: "'Andrew Cooper'" <andrew.cooper3@citrix.com>,
	"'George Dunlap'" <George.Dunlap@eu.citrix.com>,
	"'Ian Jackson'" <iwj@xenproject.org>,
	"'Julien Grall'" <julien@xen.org>, "'Wei Liu'" <wl@xen.org>,
	"'Stefano Stabellini'" <sstabellini@kernel.org>,
	"'Dario Faggioli'" <dfaggioli@suse.com>
Subject: RE: [PATCH 05/12] evtchn/sched: reject poll requests for unusable ports
Date: Tue, 29 Sep 2020 17:51:04 +0100	[thread overview]
Message-ID: <002501d69680$b935a810$2ba0f830$@xen.org> (raw)
In-Reply-To: <802a0866-6bcf-cb52-1c3f-edb628fbc9c7@suse.com>

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan Beulich
> Sent: 28 September 2020 11:59
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian
> Jackson <iwj@xenproject.org>; Julien Grall <julien@xen.org>; Wei Liu <wl@xen.org>; Stefano Stabellini
> <sstabellini@kernel.org>; Dario Faggioli <dfaggioli@suse.com>
> Subject: [PATCH 05/12] evtchn/sched: reject poll requests for unusable ports
> 
> Before and after XSA-342 there has been an asymmetry in how not really
> usable ports get treated in do_poll(): Ones beyond a certain boundary
> (max_evtchns originally, valid_evtchns subsequently) did get refused
> with -EINVAL, while lower ones were accepted despite there potentially
> being no way to wake the vCPU again from its polling state. Arrange to
> also honor evtchn_usable() output in the decision.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Paul Durrant <paul@xen.org>

> 
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -1427,13 +1427,13 @@ static long do_poll(struct sched_poll *s
>          if ( __copy_from_guest_offset(&port, sched_poll->ports, i, 1) )
>              goto out;
> 
> -        rc = -EINVAL;
> -        if ( !port_is_valid(d, port) )
> -            goto out;
> -
> -        rc = 0;
> -        if ( evtchn_port_is_pending(d, port) )
> +        rc = evtchn_port_poll(d, port);
> +        if ( rc )
> +        {
> +            if ( rc > 0 )
> +                rc = 0;
>              goto out;
> +        }
>      }
> 
>      if ( sched_poll->nr_ports == 1 )
> --- a/xen/include/xen/event.h
> +++ b/xen/include/xen/event.h
> @@ -240,19 +240,6 @@ static inline bool evtchn_is_pending(con
>      return evtchn_usable(evtchn) && d->evtchn_port_ops->is_pending(d, evtchn);
>  }
> 
> -static inline bool evtchn_port_is_pending(struct domain *d, evtchn_port_t port)
> -{
> -    struct evtchn *evtchn = evtchn_from_port(d, port);
> -    bool rc;
> -    unsigned long flags;
> -
> -    spin_lock_irqsave(&evtchn->lock, flags);
> -    rc = evtchn_is_pending(d, evtchn);
> -    spin_unlock_irqrestore(&evtchn->lock, flags);
> -
> -    return rc;
> -}
> -
>  static inline bool evtchn_is_masked(const struct domain *d,
>                                      const struct evtchn *evtchn)
>  {
> @@ -279,6 +266,24 @@ static inline bool evtchn_is_busy(const
>             d->evtchn_port_ops->is_busy(d, evtchn);
>  }
> 
> +static inline int evtchn_port_poll(struct domain *d, evtchn_port_t port)
> +{
> +    int rc = -EINVAL;
> +
> +    if ( port_is_valid(d, port) )
> +    {
> +        struct evtchn *evtchn = evtchn_from_port(d, port);
> +        unsigned long flags;
> +
> +        spin_lock_irqsave(&evtchn->lock, flags);
> +        if ( evtchn_usable(evtchn) )
> +            rc = evtchn_is_pending(d, evtchn);
> +        spin_unlock_irqrestore(&evtchn->lock, flags);
> +    }
> +
> +    return rc;
> +}
> +
>  static inline int evtchn_port_set_priority(struct domain *d,
>                                             struct evtchn *evtchn,
>                                             unsigned int priority)
>