On Fri, Nov 04, 2016 at 06:07:41PM +0100, Andrew Jones wrote:
> > > +bool pci_probe(void)
> > > +{
> > > + pcidevaddr_t dev;
> > > + u8 header;
> > > + u32 cmd;
> > > + int i;
> > > +
> > > + assert(!pci_host_bridge);
> > > + pci_host_bridge = pci_dt_probe();
> > > + if (!pci_host_bridge)
> > > + return false;
> > > +
> > > + for (dev = 0; dev < 256; dev++) {
> > > + if (!pci_dev_exists(dev))
> > > + continue;
> > > +
> > > + /* We are only interested in normal PCI devices */
> > > + header = pci_config_readb(dev, PCI_HEADER_TYPE);
> > > + if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
> > > + continue;
> > > +
> > > + cmd = PCI_COMMAND_SERR;
> > > +
> > > + for (i = 0; i < 6; i++) {
> > > + u64 addr;
> > > +
> > > + if (pci_alloc_resource(dev, i, &addr)) {
> > > + pci_bar_set_addr(dev, i, addr);
> > > +
> > > + if (pci_bar_is_memory(dev, i))
> > > + cmd |= PCI_COMMAND_MEMORY;
> > > + else
> > > + cmd |= PCI_COMMAND_IO;
> > > + }
> > > +
> > > + if (pci_bar_is64(dev, i))
> > > + i++;
> > > + }
> > > +
> > > + pci_config_writel(dev, PCI_COMMAND, cmd);
> >
> > As I asked in the last review, is this PCI_COMMAND_SERR safe/desired
> > to write, even when no resources are allocated? If not, then above
> > we need to capture the return value of pci_alloc_resource, add
> > an 'else break', and then check the return value here before doing
> > the write.
>
> Another note on this PCI_COMMAND write. Why is it safe [correct?] to
> use writel? This is a 16-bit register according to the comment in
> lib/linux/pci_regs.h. Don't we need to introduce a pci_config_writew
> and use that? If so, then you should grab Peter Xu's patch that
> introduces pci_config_writeb/w.
You are right, this is cleanly a bug.
However, I can not put Peter's patch neither before nor after mine,
since pci_config_write*() are both introduced and used in this patch.
As Peter's patch needs a refreshment anyway (see my comment on it)
I will add pci_config_writew() to this patch. Hope, that works for
everyone.
> >
> > > + }
> > > +
> > > + return true;
> > > +}
> > Thanks,
> > drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
