| Message ID | 1457320984-6540-5-git-send-email-caoj.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 7 Mar 2016 11:22:57 +0800 Cao jin <caoj.fnst@cn.fujitsu.com> wrote: > From: Chen Fan <chen.fan.fnst@cn.fujitsu.com> > > Calling pcie_aer_init to initilize aer related registers for > vfio device, then reload physical related registers to expose > device capability. > > Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> > --- > hw/vfio/pci.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > hw/vfio/pci.h | 3 +++ > 2 files changed, 81 insertions(+), 3 deletions(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index e64cce3..8ec9b25 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1868,6 +1868,62 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos) > return 0; > } > > +static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, > + int pos, uint16_t size) > +{ > + PCIDevice *pdev = &vdev->pdev; > + PCIDevice *dev_iter; > + uint8_t type; > + uint32_t errcap; > + > + if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) { > + pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR, > + cap_ver, pos, size); > + return 0; > + } > + > + dev_iter = pci_bridge_get_device(pdev->bus); > + if (!dev_iter) { > + goto error; > + } > + > + while (dev_iter) { > + type = pcie_cap_get_type(dev_iter); This asserts if dev_iter doesn't have an express capability so do we really ever get to the error goto in practice? I think an average user is going to get more information from your error_report than from an assert. Thanks, Alex > + if ((type != PCI_EXP_TYPE_ROOT_PORT && > + type != PCI_EXP_TYPE_UPSTREAM && > + type != PCI_EXP_TYPE_DOWNSTREAM)) { > + goto error; > + } > + > + if (!dev_iter->exp.aer_cap) { > + goto error; > + } > + > + dev_iter = pci_bridge_get_device(dev_iter->bus); > + } > + > + errcap = vfio_pci_read_config(pdev, pos + PCI_ERR_CAP, 4); > + /* > + * The ability to record multiple headers is depending on > + * the state of the Multiple Header Recording Capable bit and > + * enabled by the Multiple Header Recording Enable bit. > + */ > + if ((errcap & PCI_ERR_CAP_MHRC) && > + (errcap & PCI_ERR_CAP_MHRE)) { > + pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT; > + } else { > + pdev->exp.aer_log.log_max = 0; > + } > + > + pcie_cap_deverr_init(pdev); > + return pcie_aer_init(pdev, pos, size); > + > +error: > + error_report("vfio: Unable to enable AER for device %s, parent bus " > + "does not support AER signaling", vdev->vbasedev.name); > + return -1; > +} > + > static int vfio_add_ext_cap(VFIOPCIDevice *vdev) > { > PCIDevice *pdev = &vdev->pdev; > @@ -1875,6 +1931,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) > uint16_t cap_id, next, size; > uint8_t cap_ver; > uint8_t *config; > + int ret = 0; > > /* > * pcie_add_capability always inserts the new capability at the tail > @@ -1898,16 +1955,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) > */ > size = vfio_ext_cap_max_size(config, next); > > - pcie_add_capability(pdev, cap_id, cap_ver, next, size); > - pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); > + switch (cap_id) { > + case PCI_EXT_CAP_ID_ERR: > + ret = vfio_setup_aer(vdev, cap_ver, next, size); > + break; > + default: > + pcie_add_capability(pdev, cap_id, cap_ver, next, size); > + break; > + } > + > + if (ret) { > + goto out; > + } > + > + pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); > > /* Use emulated next pointer to allow dropping extended caps */ > pci_long_test_and_set_mask(vdev->emulated_config_bits + next, > PCI_EXT_CAP_NEXT_MASK); > } > > +out: > g_free(config); > - return 0; > + return ret; > } > > static int vfio_add_capabilities(VFIOPCIDevice *vdev) > @@ -2662,6 +2732,11 @@ static int vfio_initfn(PCIDevice *pdev) > goto out_teardown; > } > > + if ((vdev->features & VFIO_FEATURE_ENABLE_AER) && > + !pdev->exp.aer_cap) { > + goto out_teardown; > + } > + > /* QEMU emulates all of MSI & MSIX */ > if (pdev->cap_present & QEMU_PCI_CAP_MSIX) { > memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff, > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h > index 6256587..e0c53f2 100644 > --- a/hw/vfio/pci.h > +++ b/hw/vfio/pci.h > @@ -15,6 +15,7 @@ > #include "qemu-common.h" > #include "exec/memory.h" > #include "hw/pci/pci.h" > +#include "hw/pci/pci_bridge.h" > #include "hw/vfio/vfio-common.h" > #include "qemu/event_notifier.h" > #include "qemu/queue.h" > @@ -128,6 +129,8 @@ typedef struct VFIOPCIDevice { > #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT) > #define VFIO_FEATURE_ENABLE_REQ_BIT 1 > #define VFIO_FEATURE_ENABLE_REQ (1 << VFIO_FEATURE_ENABLE_REQ_BIT) > +#define VFIO_FEATURE_ENABLE_AER_BIT 2 > +#define VFIO_FEATURE_ENABLE_AER (1 << VFIO_FEATURE_ENABLE_AER_BIT) > int32_t bootindex; > uint8_t pm_cap; > bool has_vga;
On 03/09/2016 06:55 AM, Alex Williamson wrote: > On Mon, 7 Mar 2016 11:22:57 +0800 > Cao jin <caoj.fnst@cn.fujitsu.com> wrote: > >> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com> >> >> Calling pcie_aer_init to initilize aer related registers for >> vfio device, then reload physical related registers to expose >> device capability. >> >> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> >> --- >> hw/vfio/pci.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- >> hw/vfio/pci.h | 3 +++ >> 2 files changed, 81 insertions(+), 3 deletions(-) >> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index e64cce3..8ec9b25 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -1868,6 +1868,62 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos) >> return 0; >> } >> >> +static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, >> + int pos, uint16_t size) >> +{ >> + PCIDevice *pdev = &vdev->pdev; >> + PCIDevice *dev_iter; >> + uint8_t type; >> + uint32_t errcap; >> + >> + if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) { >> + pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR, >> + cap_ver, pos, size); >> + return 0; >> + } >> + >> + dev_iter = pci_bridge_get_device(pdev->bus); >> + if (!dev_iter) { >> + goto error; >> + } >> + >> + while (dev_iter) { >> + type = pcie_cap_get_type(dev_iter); > This asserts if dev_iter doesn't have an express capability so do we > really ever get to the error goto in practice? I think an average user > is going to get more information from your error_report than from an > assert. Thanks, you are right, I will fix it in the next version. Thanks, Chen > > Alex > >> + if ((type != PCI_EXP_TYPE_ROOT_PORT && >> + type != PCI_EXP_TYPE_UPSTREAM && >> + type != PCI_EXP_TYPE_DOWNSTREAM)) { >> + goto error; >> + } >> + >> + if (!dev_iter->exp.aer_cap) { >> + goto error; >> + } >> + >> + dev_iter = pci_bridge_get_device(dev_iter->bus); >> + } >> + >> + errcap = vfio_pci_read_config(pdev, pos + PCI_ERR_CAP, 4); >> + /* >> + * The ability to record multiple headers is depending on >> + * the state of the Multiple Header Recording Capable bit and >> + * enabled by the Multiple Header Recording Enable bit. >> + */ >> + if ((errcap & PCI_ERR_CAP_MHRC) && >> + (errcap & PCI_ERR_CAP_MHRE)) { >> + pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT; >> + } else { >> + pdev->exp.aer_log.log_max = 0; >> + } >> + >> + pcie_cap_deverr_init(pdev); >> + return pcie_aer_init(pdev, pos, size); >> + >> +error: >> + error_report("vfio: Unable to enable AER for device %s, parent bus " >> + "does not support AER signaling", vdev->vbasedev.name); >> + return -1; >> +} >> + >> static int vfio_add_ext_cap(VFIOPCIDevice *vdev) >> { >> PCIDevice *pdev = &vdev->pdev; >> @@ -1875,6 +1931,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) >> uint16_t cap_id, next, size; >> uint8_t cap_ver; >> uint8_t *config; >> + int ret = 0; >> >> /* >> * pcie_add_capability always inserts the new capability at the tail >> @@ -1898,16 +1955,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) >> */ >> size = vfio_ext_cap_max_size(config, next); >> >> - pcie_add_capability(pdev, cap_id, cap_ver, next, size); >> - pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); >> + switch (cap_id) { >> + case PCI_EXT_CAP_ID_ERR: >> + ret = vfio_setup_aer(vdev, cap_ver, next, size); >> + break; >> + default: >> + pcie_add_capability(pdev, cap_id, cap_ver, next, size); >> + break; >> + } >> + >> + if (ret) { >> + goto out; >> + } >> + >> + pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); >> >> /* Use emulated next pointer to allow dropping extended caps */ >> pci_long_test_and_set_mask(vdev->emulated_config_bits + next, >> PCI_EXT_CAP_NEXT_MASK); >> } >> >> +out: >> g_free(config); >> - return 0; >> + return ret; >> } >> >> static int vfio_add_capabilities(VFIOPCIDevice *vdev) >> @@ -2662,6 +2732,11 @@ static int vfio_initfn(PCIDevice *pdev) >> goto out_teardown; >> } >> >> + if ((vdev->features & VFIO_FEATURE_ENABLE_AER) && >> + !pdev->exp.aer_cap) { >> + goto out_teardown; >> + } >> + >> /* QEMU emulates all of MSI & MSIX */ >> if (pdev->cap_present & QEMU_PCI_CAP_MSIX) { >> memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff, >> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h >> index 6256587..e0c53f2 100644 >> --- a/hw/vfio/pci.h >> +++ b/hw/vfio/pci.h >> @@ -15,6 +15,7 @@ >> #include "qemu-common.h" >> #include "exec/memory.h" >> #include "hw/pci/pci.h" >> +#include "hw/pci/pci_bridge.h" >> #include "hw/vfio/vfio-common.h" >> #include "qemu/event_notifier.h" >> #include "qemu/queue.h" >> @@ -128,6 +129,8 @@ typedef struct VFIOPCIDevice { >> #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT) >> #define VFIO_FEATURE_ENABLE_REQ_BIT 1 >> #define VFIO_FEATURE_ENABLE_REQ (1 << VFIO_FEATURE_ENABLE_REQ_BIT) >> +#define VFIO_FEATURE_ENABLE_AER_BIT 2 >> +#define VFIO_FEATURE_ENABLE_AER (1 << VFIO_FEATURE_ENABLE_AER_BIT) >> int32_t bootindex; >> uint8_t pm_cap; >> bool has_vga; > > > . >
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index e64cce3..8ec9b25 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1868,6 +1868,62 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos) return 0; } +static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, + int pos, uint16_t size) +{ + PCIDevice *pdev = &vdev->pdev; + PCIDevice *dev_iter; + uint8_t type; + uint32_t errcap; + + if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) { + pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR, + cap_ver, pos, size); + return 0; + } + + dev_iter = pci_bridge_get_device(pdev->bus); + if (!dev_iter) { + goto error; + } + + while (dev_iter) { + type = pcie_cap_get_type(dev_iter); + if ((type != PCI_EXP_TYPE_ROOT_PORT && + type != PCI_EXP_TYPE_UPSTREAM && + type != PCI_EXP_TYPE_DOWNSTREAM)) { + goto error; + } + + if (!dev_iter->exp.aer_cap) { + goto error; + } + + dev_iter = pci_bridge_get_device(dev_iter->bus); + } + + errcap = vfio_pci_read_config(pdev, pos + PCI_ERR_CAP, 4); + /* + * The ability to record multiple headers is depending on + * the state of the Multiple Header Recording Capable bit and + * enabled by the Multiple Header Recording Enable bit. + */ + if ((errcap & PCI_ERR_CAP_MHRC) && + (errcap & PCI_ERR_CAP_MHRE)) { + pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT; + } else { + pdev->exp.aer_log.log_max = 0; + } + + pcie_cap_deverr_init(pdev); + return pcie_aer_init(pdev, pos, size); + +error: + error_report("vfio: Unable to enable AER for device %s, parent bus " + "does not support AER signaling", vdev->vbasedev.name); + return -1; +} + static int vfio_add_ext_cap(VFIOPCIDevice *vdev) { PCIDevice *pdev = &vdev->pdev; @@ -1875,6 +1931,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) uint16_t cap_id, next, size; uint8_t cap_ver; uint8_t *config; + int ret = 0; /* * pcie_add_capability always inserts the new capability at the tail @@ -1898,16 +1955,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) */ size = vfio_ext_cap_max_size(config, next); - pcie_add_capability(pdev, cap_id, cap_ver, next, size); - pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); + switch (cap_id) { + case PCI_EXT_CAP_ID_ERR: + ret = vfio_setup_aer(vdev, cap_ver, next, size); + break; + default: + pcie_add_capability(pdev, cap_id, cap_ver, next, size); + break; + } + + if (ret) { + goto out; + } + + pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); /* Use emulated next pointer to allow dropping extended caps */ pci_long_test_and_set_mask(vdev->emulated_config_bits + next, PCI_EXT_CAP_NEXT_MASK); } +out: g_free(config); - return 0; + return ret; } static int vfio_add_capabilities(VFIOPCIDevice *vdev) @@ -2662,6 +2732,11 @@ static int vfio_initfn(PCIDevice *pdev) goto out_teardown; } + if ((vdev->features & VFIO_FEATURE_ENABLE_AER) && + !pdev->exp.aer_cap) { + goto out_teardown; + } + /* QEMU emulates all of MSI & MSIX */ if (pdev->cap_present & QEMU_PCI_CAP_MSIX) { memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff, diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h index 6256587..e0c53f2 100644 --- a/hw/vfio/pci.h +++ b/hw/vfio/pci.h @@ -15,6 +15,7 @@ #include "qemu-common.h" #include "exec/memory.h" #include "hw/pci/pci.h" +#include "hw/pci/pci_bridge.h" #include "hw/vfio/vfio-common.h" #include "qemu/event_notifier.h" #include "qemu/queue.h" @@ -128,6 +129,8 @@ typedef struct VFIOPCIDevice { #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT) #define VFIO_FEATURE_ENABLE_REQ_BIT 1 #define VFIO_FEATURE_ENABLE_REQ (1 << VFIO_FEATURE_ENABLE_REQ_BIT) +#define VFIO_FEATURE_ENABLE_AER_BIT 2 +#define VFIO_FEATURE_ENABLE_AER (1 << VFIO_FEATURE_ENABLE_AER_BIT) int32_t bootindex; uint8_t pm_cap; bool has_vga;