On Tue, 2015-02-10 at 15:03 +0800, Chen Fan wrote:
> because function pcie_aer_init() is for adding a new aer capability,
> but for vfio device, we only need to capture the aer capability from
> vfio device configuration space, so here we introduce pcie_aer_setup()
> to init all raw devices.
I don't see why pcie_add_capability is split out, see for instance
msix_init() where the call still includes a call to
pci_add_capability().
> Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com>
> ---
> hw/pci/pcie_aer.c | 63
> +++++++++++++++++++++++++++++------------------
> include/hw/pci/pcie_aer.h | 1 +
> 2 files changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index ece1487..18caf43 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -94,53 +94,31 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
> aer_log->log_num = 0;
> }
>
> -int pcie_aer_init(PCIDevice *dev, uint16_t offset)
> +void pcie_aer_setup(PCIDevice *dev, uint16_t offset, uint16_t log_max)
> {
> PCIExpressDevice *exp;
>
> - pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
> - offset, PCI_ERR_SIZEOF);
> exp = &dev->exp;
> exp->aer_cap = offset;
>
> - /* log_max is property */
> - if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) {
> - dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
> - }
> - /* clip down the value to avoid unreasobale memory usage */
> - if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_LIMIT) {
> - return -EINVAL;
> - }
> - dev->exp.aer_log.log = g_malloc0(sizeof dev->exp.aer_log.log[0] *
> - dev->exp.aer_log.log_max);
>
> pci_set_long(dev->w1cmask + offset + PCI_ERR_UNCOR_STATUS,
> PCI_ERR_UNC_SUPPORTED);
>
> - pci_set_long(dev->config + offset + PCI_ERR_UNCOR_SEVER,
> - PCI_ERR_UNC_SEVERITY_DEFAULT);
> pci_set_long(dev->wmask + offset + PCI_ERR_UNCOR_SEVER,
> PCI_ERR_UNC_SUPPORTED);
>
> pci_long_test_and_set_mask(dev->w1cmask + offset + PCI_ERR_COR_STATUS,
> PCI_ERR_COR_SUPPORTED);
>
> - pci_set_long(dev->config + offset + PCI_ERR_COR_MASK,
> - PCI_ERR_COR_MASK_DEFAULT);
> pci_set_long(dev->wmask + offset + PCI_ERR_COR_MASK,
> PCI_ERR_COR_SUPPORTED);
>
> - /* capabilities and control. multiple header logging is supported */
> - if (dev->exp.aer_log.log_max > 0) {
> - pci_set_long(dev->config + offset + PCI_ERR_CAP,
> - PCI_ERR_CAP_ECRC_GENC | PCI_ERR_CAP_ECRC_CHKC |
> - PCI_ERR_CAP_MHRC);
> + if (log_max > 0) {
> pci_set_long(dev->wmask + offset + PCI_ERR_CAP,
> PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE |
> PCI_ERR_CAP_MHRE);
> } else {
> - pci_set_long(dev->config + offset + PCI_ERR_CAP,
> - PCI_ERR_CAP_ECRC_GENC | PCI_ERR_CAP_ECRC_CHKC);
> pci_set_long(dev->wmask + offset + PCI_ERR_CAP,
> PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
> }
> @@ -160,6 +138,43 @@ int pcie_aer_init(PCIDevice *dev, uint16_t offset)
> /* nothing */
> break;
> }
> +}
> +
> +int pcie_aer_init(PCIDevice *dev, uint16_t offset)
> +{
> +
> + pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
> + offset, PCI_ERR_SIZEOF);
> +
> + /* log_max is property */
> + if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) {
> + dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
> + }
> + /* clip down the value to avoid unreasobale memory usage */
> + if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_LIMIT) {
> + return -EINVAL;
> + }
> +
> + dev->exp.aer_log.log = g_malloc0(sizeof dev->exp.aer_log.log[0] *
> + dev->exp.aer_log.log_max);
> +
> + /* capabilities and control. multiple header logging is supported */
> + if (dev->exp.aer_log.log_max > 0) {
> + pci_set_long(dev->config + offset + PCI_ERR_CAP,
> + PCI_ERR_CAP_ECRC_GENC | PCI_ERR_CAP_ECRC_CHKC |
> + PCI_ERR_CAP_MHRC);
> + } else {
> + pci_set_long(dev->config + offset + PCI_ERR_CAP,
> + PCI_ERR_CAP_ECRC_GENC | PCI_ERR_CAP_ECRC_CHKC);
> + }
> +
> + pci_set_long(dev->config + offset + PCI_ERR_UNCOR_SEVER,
> + PCI_ERR_UNC_SEVERITY_DEFAULT);
> + pci_set_long(dev->config + offset + PCI_ERR_COR_MASK,
> + PCI_ERR_COR_MASK_DEFAULT);
> +
> + pcie_aer_setup(dev, offset, dev->exp.aer_log.log_max);
> +
> return 0;
> }
>
> diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
> index bcac80a..e675c7d 100644
> --- a/include/hw/pci/pcie_aer.h
> +++ b/include/hw/pci/pcie_aer.h
> @@ -87,6 +87,7 @@ struct PCIEAERErr {
>
> extern const VMStateDescription vmstate_pcie_aer_log;
>
> +void pcie_aer_setup(PCIDevice *dev, uint16_t offset, uint16_t log_max);
> int pcie_aer_init(PCIDevice *dev, uint16_t offset);
> void pcie_aer_exit(PCIDevice *dev);
> void pcie_aer_write_config(PCIDevice *dev,
