Re: [kvm-unit-tests PATCH v9 10/12] pci: Add generic ECAM host support

[Alexander Gordeev <agordeev@redhat.com>]

On Fri, Nov 04, 2016 at 03:33:52PM +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

Yes, it allows a device to report grave PCI bus errors. Moreover,
I think I should have enabled parity errors check as well.

I assume we want to notice such errors, including the case no
resources were assigned. That would be more troubling even.

In Linux it is up to device drivers to enable these, but I
would just let all devices scream loudly by default.

> 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.
> 
> > +	}
> > +
> > +	return true;
> > +}
> Thanks,
> drew