Re: [Xen-devel] [PATCH v4 6/9] xen/vpci: add handlers to map the BARs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.08.17 at 14:35, <roger.pau@xxxxxxxxxx> wrote:
> On Sat, Jul 29, 2017 at 10:44:02AM -0600, Jan Beulich wrote:
>> >>> Roger Pau Monne <roger.pau@xxxxxxxxxx> 07/24/17 4:58 PM >>>
>> >On Fri, Jul 14, 2017 at 09:11:29AM -0600, Jan Beulich wrote:
>> >> >>> On 30.06.17 at 17:01, <roger.pau@xxxxxxxxxx> wrote:
>> >> > +        /* Size the BAR and map it. */
>> >> > +        rc = pci_size_mem_bar(seg, bus, slot, func, reg, i == num_bars 
>> >> > - 1,
>> >> > +                              &addr, &size);
>> >> > +        if ( rc < 0 )
>> >> > +            return rc;
>> >> > +
>> >> > +        if ( size == 0 )
>> >> > +        {
>> >> > +            bars[i].type = VPCI_BAR_EMPTY;
>> >> > +            continue;
>> >> > +        }
>> >> > +
>> >> > +        bars[i].addr = (cmd & PCI_COMMAND_MEMORY) ? addr : 
>> >> > INVALID_PADDR;
>> >> 
>> >> This doesn't match up with logic further up: When the memory decode
>> >> bit gets cleared, you don't zap the addresses, so I think you'd better
>> >> store it here too. Use INVALID_PADDR only when the value read has
>> >> all address bits set (same caveat as pointed out earlier).
>> >
>> >OK, note that .addr can only possibly be INVALID_PADDR at
>> >initialization time, once the user has written something to the BAR
>> >.addr will be different than INVALID_PADDR.
>> 
>> Which is part of what worries me - it would be better if the field wouldn't
>> ever hold a special init-time-only value.
> 
> Right, but that matches the behavior of the hardware itself. On boot
> the address of the BAR is not valid, but there's no way AFAIK to
> restore the BAR to this state once an address has been written (except
> by doing a reset of the device itself).

True, but the BARs still hold _some_ value. And hence they can
equally well be made hold a value consistent with normal runtime.

Jan


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