Re: [Xen-devel] [PATCH v5 08/11] vpci/bars: add handlers to map the BARs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.09.17 at 13:48, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, Sep 12, 2017 at 04:06:13AM -0600, Jan Beulich wrote:
>> >>> On 12.09.17 at 11:54, <roger.pau@xxxxxxxxxx> wrote:
>> > On Thu, Sep 07, 2017 at 03:53:07AM -0600, Jan Beulich wrote:
>> >> >>> On 14.08.17 at 16:28, <roger.pau@xxxxxxxxxx> wrote:
>> >> > +
>> >> > +    /*
>> >> > +     * The PCI Local Bus Specification suggests writing ~0 to both the 
> high
>> >> > +     * and the low part of the BAR registers before attempting to read 
> back
>> >> > +     * the size.
>> >> > +     *
>> >> > +     * However real device BARs registers (at least the ones I've 
>> >> > tried)
>> >> > +     * will return the size of the BAR just by having written ~0 to 
>> >> > one 
> half
>> >> > +     * of it, independently of the value of the other half of the 
> register.
>> >> > +     * Hence here Xen will switch to returning the size as soon as one 
> half
>> >> > +     * of the BAR register has been written with ~0.
>> >> > +     */
>> >> 
>> >> I don't believe this is correct behavior (but I'd have to play with
>> >> some hardware to see whether I can confirm the behavior you
>> >> describe): How would you place a BAR at, say, 0x1ffffff0?
>> > 
>> > I don't think it's 'correct' behavior either, but FreeBSD has been
>> > sizing BARs like that, and nobody noticed any issues. I just fixed it
>> > recently:
>> > 
>> > https://svnweb.freebsd.org/base/head/sys/dev/pci/pci.c?r1=312250&r2=321863 
>> 
>> Oh, no, that old code was fine afaict. You have to view the two
>> halves of a 64-bit BAR as completely distinct registers, and
>> sizing of the full BAR can be done either by writing both, then
>> reading both, or reading/writing each half.
> 
> OK, the example in the specification seems to suggest that you should
> first write to both registers, and then read back the values.
> 
>> The problem with the
>> code in the patch here is that you don't treat the two halves as
>> fully separate registers, implying that one half being written with
>> all ones _also_ makes the other half return the sizing value
>> instead of the last written address.
> 
> Right, then I think adding a sizing_hi/sizing_lo field is the right
> answer.

That was my first thought too, but meanwhile I think these flags
are pointless and misleading, and hence should be dropped.
There's really nothing special about those sizing writes: They
simply write a special address, but to the handler this shouldn't
matter. All you need to make sure is that hardwired to zero bits
come back as zero on the following read(s), entirely independent
of what precise value was written (the r/o bits at the bottom of
the first half left aside here, of course).

Jan


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