>>> On 29.09.17 at 17:38, <paul.durr...@citrix.com> wrote: >> -----Original Message----- >> From: Andrew Cooper >> Sent: 29 September 2017 16:35 >> To: Paul Durrant <paul.durr...@citrix.com>; xen-de...@lists.xenproject.org >> Cc: Jan Beulich <jbeul...@suse.com> >> Subject: Re: [PATCH v8 01/11] x86/hvm/ioreq: maintain an array of ioreq >> servers rather than a list >> >> On 29/09/17 15:51, Paul Durrant wrote: >> > A subsequent patch will remove the current implicit limitation on creation >> > of ioreq servers which is due to the allocation of gfns for the ioreq >> > structures and buffered ioreq ring. >> > >> > It will therefore be necessary to introduce an explicit limit and, since >> > this limit should be small, it simplifies the code to maintain an array of >> > that size rather than using a list. >> > >> > Also, by reserving an array slot for the default server and populating >> > array slots early in create, the need to pass an 'is_default' boolean >> > to sub-functions can be avoided. >> > >> > Some function return values are changed by this patch: Specifically, in >> > the case where the id of the default ioreq server is passed in, - >> EOPNOTSUPP >> > is now returned rather than -ENOENT. >> > >> > Signed-off-by: Paul Durrant <paul.durr...@citrix.com> >> > Reviewed-by: Roger Pau Monné <roger....@citrix.com> >> > --- >> > Cc: Jan Beulich <jbeul...@suse.com> >> > Cc: Andrew Cooper <andrew.coop...@citrix.com> >> > >> > v8: >> > - Addressed various comments from Jan. >> > >> > v7: >> > - Fixed assertion failure found in testing. >> > >> > v6: >> > - Updated according to comments made by Roger on v4 that I'd missed. >> > >> > v5: >> > - Switched GET/SET_IOREQ_SERVER() macros to get/set_ioreq_server() >> > functions to avoid possible double-evaluation issues. >> > >> > v4: >> > - Introduced more helper macros and relocated them to the top of the >> > code. >> > >> > v3: >> > - New patch (replacing "move is_default into struct hvm_ioreq_server") in >> > response to review comments. >> > --- >> > xen/arch/x86/hvm/ioreq.c | 525 ++++++++++++++++++++-------------- >> ----- >> > xen/include/asm-x86/hvm/domain.h | 10 +- >> > 2 files changed, 270 insertions(+), 265 deletions(-) >> > >> > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c >> > index f2e0b3f74a..e655d2eab3 100644 >> > --- a/xen/arch/x86/hvm/ioreq.c >> > +++ b/xen/arch/x86/hvm/ioreq.c >> > @@ -33,6 +33,41 @@ >> > >> > #include <public/hvm/ioreq.h> >> > >> > +static void set_ioreq_server(struct domain *d, unsigned int id, >> > + struct hvm_ioreq_server *s) >> > +{ >> > + ASSERT(id < MAX_NR_IOREQ_SERVERS); >> > + ASSERT(!s || !d->arch.hvm_domain.ioreq_server.server[id]); >> > + >> > + d->arch.hvm_domain.ioreq_server.server[id] = s; >> > +} >> > + >> > +#define GET_IOREQ_SERVER(d, id) \ >> > + (d)->arch.hvm_domain.ioreq_server.server[id] >> > + >> > +static struct hvm_ioreq_server *get_ioreq_server(const struct domain >> *d, >> > + unsigned int id) >> > +{ >> > + if ( id >= MAX_NR_IOREQ_SERVERS ) >> > + return NULL; >> > + >> > + return GET_IOREQ_SERVER(d, id); >> > +} >> > + >> > +#define IS_DEFAULT(s) \ >> > + ((s) == get_ioreq_server((s)->domain, DEFAULT_IOSERVID)) >> > + >> > +/* >> > + * Iterate over all possible ioreq servers. The use of inline function >> > + * get_ioreq_server() in the increment is deliberate as use of the >> > + * GET_IOREQ_SERVER() macro will cause gcc to complain about an array >> > + * overflow. >> > + */ >> > +#define FOR_EACH_IOREQ_SERVER(d, id, s) \ >> > + for ( (id) = 0, (s) = GET_IOREQ_SERVER(d, 0); \ >> > + (id) < MAX_NR_IOREQ_SERVERS; \ >> > + (s) = get_ioreq_server(d, ++(id)) ) >> >> I'm guessing from the various constructs, the list of ioreq servers >> might have embedded NULLs in the middle? >> >> If so, how about this? >> >> #define FOR_EACH_IOREQ_SERVER(d, id, s) \ >> for ( (id) = 0, (s) = GET_IOREQ_SERVER(d, 0); \ >> (id) < MAX_NR_IOREQ_SERVERS; \ >> (s) = get_ioreq_server(d, ++(id)) ) \ >> if ( !s ) \ >> continue; \ >> else > > I'm ok with it but I'll wait for others opinion on whether this is taking > the macro magic too far.
I'm fine with Andrew's suggestion; I'm surprised though trickery like this is being suggested at all, as commonly games I happen to be trying to play once in a while don't seem to be really liked. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel