Re: [Xen-devel] [PATCH v3 4/9] xen/pci: split code to size BARs from pci_add_device

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Fri, May 19, 2017 at 07:56:55AM -0600, Jan Beulich wrote:
> >>> On 27.04.17 at 16:35, <roger.pau@xxxxxxxxxx> wrote:
> > @@ -663,38 +708,13 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
> >                             seg, bus, slot, func, i);
> >                      continue;
> >                  }
> > -                pci_conf_write32(seg, bus, slot, func, idx, ~0);
> > -                if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
> > -                     PCI_BASE_ADDRESS_MEM_TYPE_64 )
> > -                {
> > -                    if ( i >= PCI_SRIOV_NUM_BARS )
> > -                    {
> > -                        printk(XENLOG_WARNING
> > -                               "SR-IOV device %04x:%02x:%02x.%u with 
> > 64-bit"
> > -                               " vf BAR in last slot\n",
> > -                               seg, bus, slot, func);
> > -                        break;
> > -                    }
> > -                    hi = pci_conf_read32(seg, bus, slot, func, idx + 4);
> > -                    pci_conf_write32(seg, bus, slot, func, idx + 4, ~0);
> > -                }
> > -                pdev->vf_rlen[i] = pci_conf_read32(seg, bus, slot, func, 
> > idx) &
> > -                                   PCI_BASE_ADDRESS_MEM_MASK;
> > -                if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
> > -                     PCI_BASE_ADDRESS_MEM_TYPE_64 )
> > -                {
> > -                    pdev->vf_rlen[i] |= (u64)pci_conf_read32(seg, bus,
> > -                                                             slot, func,
> > -                                                             idx + 4) << 
> > 32;
> > -                    pci_conf_write32(seg, bus, slot, func, idx + 4, hi);
> > -                }
> > -                else if ( pdev->vf_rlen[i] )
> > -                    pdev->vf_rlen[i] |= (u64)~0 << 32;
> > -                pci_conf_write32(seg, bus, slot, func, idx, bar);
> > -                pdev->vf_rlen[i] = -pdev->vf_rlen[i];
> > -                if ( (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
> > -                     PCI_BASE_ADDRESS_MEM_TYPE_64 )
> > -                    ++i;
> > +                ret = pci_size_bar(seg, bus, slot, func, pos + 
> > PCI_SRIOV_BAR,
> > +                                   PCI_SRIOV_NUM_BARS, &i, &addr,
> > +                                   &pdev->vf_rlen[i]);
> > +                if ( ret )
> > +                    dprintk(XENLOG_WARNING,
> > +                            "%04x:%02x:%02x.%u: failed to size SR-IOV 
> > BAR%u\n",
> > +                            seg, bus, slot, func, i);
> 
> You shouldn't log two messages for the same problem (the called
> function already logs one).
> 
> A final more general remark: With you intending to call this function
> from other than pci_add_device() context, some further care may /
> will be needed. For example, are all to be added callers such that
> you playing with config space won't interfere with what Dom0 does?
> Are you sure you can get away without disabling memory decode
> while fiddling with the BARs?

So far I've been able to get away, but you are right that callers
should disable memory decode before trying to size the BARs. I will do
this in the callers however.

Roger.

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