On Tue, 28 Mar 2017 21:47:30 +0800 Cao jin <caoj.f...@cn.fujitsu.com> wrote:
> On 03/25/2017 06:12 AM, Alex Williamson wrote: > > On Thu, 23 Mar 2017 17:09:21 +0800 > > Cao jin <caoj.f...@cn.fujitsu.com> wrote: > > > >> For devices which support AER function, verify it can work or not in the > >> system: > >> 1. AER capable device is a PCIe device, it can't be plugged into PCI bus > >> 2. If root port doesn't support AER, then there is no need to expose the > >> AER capability > >> > >> Signed-off-by: Dou Liyang <douly.f...@cn.fujitsu.com> > >> Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> > >> --- > >> hw/pci/pcie_aer.c | 28 ++++++++++++++++++++++++++++ > >> 1 file changed, 28 insertions(+) > >> > >> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > >> index daf1f65..a2e9818 100644 > >> --- a/hw/pci/pcie_aer.c > >> +++ b/hw/pci/pcie_aer.c > >> @@ -100,6 +100,34 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log) > >> int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset, > >> uint16_t size, Error **errp) > >> { > >> + PCIDevice *parent_dev; > >> + uint8_t type; > >> + uint8_t parent_type; > >> + > >> + /* Topology test: see if there is need to expose AER cap */ > >> + type = pcie_cap_get_type(dev); > >> + parent_dev = pci_bridge_get_device(dev->bus); > >> + while (parent_dev) { > >> + parent_type = pcie_cap_get_type(parent_dev); > >> + > >> + if (type == PCI_EXP_TYPE_ENDPOINT && > >> + (parent_type != PCI_EXP_TYPE_ROOT_PORT && > >> + parent_type != PCI_EXP_TYPE_DOWNSTREAM)) { > >> + error_setg(errp, "Parent device is not a PCIe component"); > >> + return -ENOTSUP; > >> + } > >> + > >> + if (parent_type == PCI_EXP_TYPE_ROOT_PORT) { > >> + if (!parent_dev->exp.aer_cap) > >> + { > > > > Curly brace at the end of the previous line. > > > >> + error_setg(errp, "Root port does not support AER"); > >> + return -ENOTSUP; > >> + } > >> + } > >> + > >> + parent_dev = pci_bridge_get_device(parent_dev->bus); > >> + } > >> + > >> pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, cap_ver, > >> offset, size); > >> dev->exp.aer_cap = offset; > > > > This patch makes existing configurations including PCIe root ports, > > upstream ports, downstream ports, and e1000e fail if they do not meet > > this new configuration requirement. > > > > Yes, I noticed that e1000e could be realized on i440fx, which I think is > not possible in real world, like the commit log(1.) said. > > But for those ports, what are the conditions they will fail with this patch? It seems that the algorithm expects a PCIe device on a PCIe machine type to have PCIe components all the way to the root bus. This is easy to break with a Q35 machine, DMI-to-PCI bridge (ie. PCIe-to-PCI bridge), and a PCIe root port. Such a configuration isn't exactly legitimate from a PCI topology perspective, but QEMU will let you do it. If we had generic PCIe-to-PCI and PCI-to-PCIe bridges, it could even be done with a legitimate PCI topology. Thanks, Alex