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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Thanks, Roger.

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