| Message ID | 20190821145903.45934-4-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ioreq: add support for internal servers | expand |
> -----Original Message----- > From: Roger Pau Monne <roger.pau@citrix.com> > Sent: 21 August 2019 15:59 > To: xen-devel@lists.xenproject.org > Cc: Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich > <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org> > Subject: [PATCH 3/7] ioreq: allow dispatching ioreqs to internal servers > > Internal ioreq servers are always processed first, and ioreqs are > dispatched by calling the handler function. If no internal servers have > registered for an ioreq it's then forwarded to external callers. Distinct id ranges would help here... Internal ids could be walked first, then external. If there's no possibility of interleaving then you don't need the retry. Paul > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/hvm/ioreq.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c > index 23ef9b0c02..3fb6fe9585 100644 > --- a/xen/arch/x86/hvm/ioreq.c > +++ b/xen/arch/x86/hvm/ioreq.c > @@ -1305,6 +1305,7 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, > uint8_t type; > uint64_t addr; > unsigned int id; > + bool internal = true; > > if ( p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO ) > return NULL; > @@ -1345,11 +1346,12 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, > addr = p->addr; > } > > + retry: > FOR_EACH_IOREQ_SERVER(d, id, s) > { > struct rangeset *r; > > - if ( !s->enabled ) > + if ( !s->enabled || s->internal != internal ) > continue; > > r = s->range[type]; > @@ -1387,6 +1389,12 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, > } > } > > + if ( internal ) > + { > + internal = false; > + goto retry; > + } > + > return NULL; > } > > @@ -1492,9 +1500,18 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s, ioreq_t *proto_p, > > ASSERT(s); > > + if ( s->internal && buffered ) > + { > + ASSERT_UNREACHABLE(); > + return X86EMUL_UNHANDLEABLE; > + } > + > if ( buffered ) > return hvm_send_buffered_ioreq(s, proto_p); > > + if ( s->internal ) > + return s->handler(curr, proto_p); > + > if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) > return X86EMUL_RETRY; > > -- > 2.22.0
On Wed, Aug 21, 2019 at 06:29:04PM +0200, Paul Durrant wrote: > > -----Original Message----- > > From: Roger Pau Monne <roger.pau@citrix.com> > > Sent: 21 August 2019 15:59 > > To: xen-devel@lists.xenproject.org > > Cc: Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich > > <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org> > > Subject: [PATCH 3/7] ioreq: allow dispatching ioreqs to internal servers > > > > Internal ioreq servers are always processed first, and ioreqs are > > dispatched by calling the handler function. If no internal servers have > > registered for an ioreq it's then forwarded to external callers. > > Distinct id ranges would help here... Internal ids could be walked first, then external. If there's no possibility of interleaving then you don't need the retry. So if internal vs external is keyed on the ID then we would end up with two different arrays in hvm_domain, one for internal and one for external ioreq servers. Maybe instead of my previous suggestion it would be better to just define consecutive ranges for external and internal servers, like: #define MAX_NR_EXTERNAL_IOREQ_SERVERS 8 #define MAX_NR_INTERNAL_IOREQ_SERVERS 1 #define MAX_NR_IOREQ_SERVERS \ (MAX_NR_EXTERNAL_IOREQ_SERVERS + MAX_NR_INTERNAL_IOREQ_SERVERS) #define FOR_EACH_IOREQ_SERVER(d, id, s) \ for ( (id) = MAX_NR_IOREQ_SERVERS * 2; (id) != 0; ) \ if ( !(s = GET_IOREQ_SERVER(d, --(id))) ) \ continue; \ else #define FOR_EACH_INTERNAL_IOREQ_SERVER(d, id, s) \ for ( (id) = MAX_NR_IOREQ_SERVERS; (id) > MAX_NR_INTERNAL_IOREQ_SERVERS && (id) != 0; ) \ if ( !(s = GET_IOREQ_SERVER(d, --(id))) ) \ continue; \ else #define FOR_EACH_EXTERNAL_IOREQ_SERVER(d, id, s) \ for ( (id) = MAX_NR_INTERNAL_IOREQ_SERVERS; (id) != 0; ) \ if ( !(s = GET_IOREQ_SERVER(d, --(id))) ) \ continue; \ else That would also force FOR_EACH_IOREQ_SERVER to always process internal ioreq servers first. We could even have something like: union { struct { struct hvm_ioreq_server *external_server[MAX_NR_EXTERNAL_IOREQ_SERVERS]; struct hvm_ioreq_server *internal_server[MAX_NR_INTERNAL_IOREQ_SERVERS]; } struct hvm_ioreq_server *server[MAX_NR_IOREQ_SERVERS]; } In order to split the arrays if required. Thanks, Roger.
> -----Original Message----- > From: Roger Pau Monne <roger.pau@citrix.com> > Sent: 22 August 2019 08:41 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org> > Subject: Re: [PATCH 3/7] ioreq: allow dispatching ioreqs to internal servers > > On Wed, Aug 21, 2019 at 06:29:04PM +0200, Paul Durrant wrote: > > > -----Original Message----- > > > From: Roger Pau Monne <roger.pau@citrix.com> > > > Sent: 21 August 2019 15:59 > > > To: xen-devel@lists.xenproject.org > > > Cc: Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich > > > <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org> > > > Subject: [PATCH 3/7] ioreq: allow dispatching ioreqs to internal servers > > > > > > Internal ioreq servers are always processed first, and ioreqs are > > > dispatched by calling the handler function. If no internal servers have > > > registered for an ioreq it's then forwarded to external callers. > > > > Distinct id ranges would help here... Internal ids could be walked first, then external. If there's > no possibility of interleaving then you don't need the retry. > > So if internal vs external is keyed on the ID then we would end up > with two different arrays in hvm_domain, one for internal and one for > external ioreq servers. > > Maybe instead of my previous suggestion it would be better to just > define consecutive ranges for external and internal servers, like: > > #define MAX_NR_EXTERNAL_IOREQ_SERVERS 8 > #define MAX_NR_INTERNAL_IOREQ_SERVERS 1 > #define MAX_NR_IOREQ_SERVERS \ > (MAX_NR_EXTERNAL_IOREQ_SERVERS + MAX_NR_INTERNAL_IOREQ_SERVERS) > > #define FOR_EACH_IOREQ_SERVER(d, id, s) \ > for ( (id) = MAX_NR_IOREQ_SERVERS * 2; (id) != 0; ) \ > if ( !(s = GET_IOREQ_SERVER(d, --(id))) ) \ > continue; \ > else > > #define FOR_EACH_INTERNAL_IOREQ_SERVER(d, id, s) \ > for ( (id) = MAX_NR_IOREQ_SERVERS; (id) > MAX_NR_INTERNAL_IOREQ_SERVERS && (id) != 0; ) \ > if ( !(s = GET_IOREQ_SERVER(d, --(id))) ) \ > continue; \ > else > > #define FOR_EACH_EXTERNAL_IOREQ_SERVER(d, id, s) \ > for ( (id) = MAX_NR_INTERNAL_IOREQ_SERVERS; (id) != 0; ) \ > if ( !(s = GET_IOREQ_SERVER(d, --(id))) ) \ > continue; \ > else > > That would also force FOR_EACH_IOREQ_SERVER to always process internal > ioreq servers first. Exactly what I was thinking. > > We could even have something like: > > union { > struct { > struct hvm_ioreq_server *external_server[MAX_NR_EXTERNAL_IOREQ_SERVERS]; > struct hvm_ioreq_server *internal_server[MAX_NR_INTERNAL_IOREQ_SERVERS]; > } > struct hvm_ioreq_server *server[MAX_NR_IOREQ_SERVERS]; > } > > In order to split the arrays if required. > I'd not considered a union, but it makes sense :-) Paul > Thanks, Roger.
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c index 23ef9b0c02..3fb6fe9585 100644 --- a/xen/arch/x86/hvm/ioreq.c +++ b/xen/arch/x86/hvm/ioreq.c @@ -1305,6 +1305,7 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, uint8_t type; uint64_t addr; unsigned int id; + bool internal = true; if ( p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO ) return NULL; @@ -1345,11 +1346,12 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, addr = p->addr; } + retry: FOR_EACH_IOREQ_SERVER(d, id, s) { struct rangeset *r; - if ( !s->enabled ) + if ( !s->enabled || s->internal != internal ) continue; r = s->range[type]; @@ -1387,6 +1389,12 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, } } + if ( internal ) + { + internal = false; + goto retry; + } + return NULL; } @@ -1492,9 +1500,18 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s, ioreq_t *proto_p, ASSERT(s); + if ( s->internal && buffered ) + { + ASSERT_UNREACHABLE(); + return X86EMUL_UNHANDLEABLE; + } + if ( buffered ) return hvm_send_buffered_ioreq(s, proto_p); + if ( s->internal ) + return s->handler(curr, proto_p); + if ( unlikely(!vcpu_start_shutdown_deferral(curr)) ) return X86EMUL_RETRY;
Internal ioreq servers are always processed first, and ioreqs are dispatched by calling the handler function. If no internal servers have registered for an ioreq it's then forwarded to external callers. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/hvm/ioreq.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)