Re: [Xen-devel] [PATCH v2 09/30] x86/vtd: fix and simplify mapping RMRR regions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.09.16 at 13:27, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Sep 29, 2016 at 08:18:36AM -0600, Jan Beulich wrote:
>> >>> On 27.09.16 at 17:57, <roger.pau@xxxxxxxxxx> wrote:
>> > +{
>> > +    int rc;
>> > +
>> > +    while ( nr_pages > 0 )
>> > +    {
>> > +        rc = (map ? map_mmio_regions : unmap_mmio_regions)
>> > +             (d, _gfn(pfn), nr_pages, _mfn(pfn));
>> > +        if ( rc == 0 )
>> > +            break;
>> > +        if ( rc < 0 )
>> > +        {
>> > +            printk(XENLOG_ERR
>> > +                "Failed to %smap %#lx - %#lx into domain %d memory map: 
>> > %d\n",
>> > +                   map ? "" : "un", pfn, pfn + nr_pages, d->domain_id, 
>> > rc);
>> > +            return rc;
>> > +        }
>> > +        nr_pages -= rc;
>> > +        pfn += rc;
>> > +        process_pending_softirqs();
>> > +    }
>> > +
>> > +    return rc;
>> 
>> The way this is coded it appears to possibly return non-zero even in
>> success case. I think this would therefore better be a for ( ; ; ) loop.
> 
> I don't think this is possible, {un}map_mmio_regions will return < 0 on 
> error, > 0 if there are pending pages to map, and 0 when all the requested 
> pages have been mapped successfully.

Right - hence the "appears to" in my reply; it took me a while to
figure it's not actually possible, and hence my desire to make this
more obvious to the reader.

Jan


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