Re: [Xen-devel] [PATCH v8 1/3] ioreq-server: add support for multiple servers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.05.14 at 17:06, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 20 May 2014 15:55
>> To: Paul Durrant
>> Cc: Ian Jackson; Stefano Stabellini; xen-devel@xxxxxxxxxxxxx 
>> Subject: Re: [PATCH v8 1/3] ioreq-server: add support for multiple servers
>> 
>> >>> On 19.05.14 at 15:47, <paul.durrant@xxxxxxxxxx> wrote:
>> > +static int hvm_access_cf8(
>> > +    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
>> > +{
>> > +    struct domain *d = current->domain;
>> > +
>> > +    if ( dir == IOREQ_WRITE &&
>> > +         bytes == 4 )
>> 
>> Don't you also need to check port == 0xcf8 here?
>> 
> 
> I don't think I do because (for AFAICT port IO at least) Xen should ensure 
> the only possible 4 byte write that can come in here has to be to cf8. Any 
> access to cf9, cfa, or cfb would have to be less than 4 bytes long.

Ah, indeed (and the same for MMIO). In that case if you could make
the if() a single line...

>> > +static ioservid_t next_ioservid(struct domain *d)
>> > +{
>> > +    struct hvm_ioreq_server *s;
>> > +    ioservid_t id;
>> > +
>> > +    ASSERT(spin_is_locked(&d->arch.hvm_domain.ioreq_server.lock));
>> > +
>> > +    id = d->arch.hvm_domain.ioreq_server.id;
>> > +
>> > + again:
>> > +    id++;
>> > +
>> > +    /* Check for uniqueness */
>> > +    list_for_each_entry ( s,
>> > +                          &d->arch.hvm_domain.ioreq_server.list,
>> > +                          list_entry )
>> > +    {
>> > +        if (id == s->id)
>> 
>> The beloved coding style remark again.
>> 
> 
> Unnecessary braces? Ok.

No - the braces are a matter of taste. There are missing blanks inside
the parentheses.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel