We have two very similar root-port devices, pnv-phb3-root-port and
pnv-phb4-root-port. Both consist of a wrapper around the PCIESlot device
that, until now, has no additional attributes.

The main difference between the PHB3 and PHB4 root ports is that
pnv-phb4-root-port has the pnv_phb4_root_port_reset() callback. All
other differences can be merged in a single device without too much
trouble.

This patch introduces the unified pnv-phb-root-port that, in time, will
be used as the default root port for the pnv-phb device.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb.c | 115 +++++++++++++++++++++++++++++++++++++++---
 hw/pci-host/pnv_phb.h |  16 ++++++
 2 files changed, 123 insertions(+), 8 deletions(-)

diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index abcbcca445..5e61f85614 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -112,15 +112,114 @@ static void pnv_phb_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 }
 
-static void pnv_phb_register_type(void)
+static void pnv_phb_root_port_reset(DeviceState *dev)
 {
-    static const TypeInfo pnv_phb_type_info = {
-        .name          = TYPE_PNV_PHB,
-        .parent        = TYPE_PCIE_HOST_BRIDGE,
-        .instance_size = sizeof(PnvPHB),
-        .class_init    = pnv_phb_class_init,
-    };
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
+    PnvPHBRootPort *phb_rp = PNV_PHB_ROOT_PORT(dev);
+    PCIDevice *d = PCI_DEVICE(dev);
+    uint8_t *conf = d->config;
 
+    rpc->parent_reset(dev);
+
+    if (phb_rp->version == 3) {
+        return;
+    }
+
+    /* PHB4 and later requires these extra reset steps */
+    pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
+                               PCI_IO_RANGE_MASK & 0xff);
+    pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
+                                 PCI_IO_RANGE_MASK & 0xff);
+    pci_set_word(conf + PCI_MEMORY_BASE, 0);
+    pci_set_word(conf + PCI_MEMORY_LIMIT, 0xfff0);
+    pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0x1);
+    pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1);
+    pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */
+    pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0xffffffff);
+    pci_config_set_interrupt_pin(conf, 0);
+}
+
+static void pnv_phb_root_port_realize(DeviceState *dev, Error **errp)
+{
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
+    PnvPHBRootPort *phb_rp = PNV_PHB_ROOT_PORT(dev);
+    PCIDevice *pci = PCI_DEVICE(dev);
+    uint16_t device_id = 0;
+    Error *local_err = NULL;
+
+    rpc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    switch (phb_rp->version) {
+    case 3:
+        device_id = PNV_PHB3_DEVICE_ID;
+        break;
+    case 4:
+        device_id = PNV_PHB4_DEVICE_ID;
+        break;
+    case 5:
+        device_id = PNV_PHB5_DEVICE_ID;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    pci_config_set_device_id(pci->config, device_id);
+    pci_config_set_interrupt_pin(pci->config, 0);
+}
+
+static Property pnv_phb_root_port_properties[] = {
+    DEFINE_PROP_UINT32("version", PnvPHBRootPort, version, 0),
+
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pnv_phb_root_port_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
+
+    dc->desc     = "IBM PHB PCIE Root Port";
+
+    device_class_set_props(dc, pnv_phb_root_port_properties);
+    device_class_set_parent_realize(dc, pnv_phb_root_port_realize,
+                                    &rpc->parent_realize);
+    device_class_set_parent_reset(dc, pnv_phb_root_port_reset,
+                                  &rpc->parent_reset);
+    dc->reset = &pnv_phb_root_port_reset;
+    dc->user_creatable = false;
+
+    k->vendor_id = PCI_VENDOR_ID_IBM;
+    /* device_id will be written during realize() */
+    k->device_id = 0;
+    k->revision  = 0;
+
+    rpc->exp_offset = 0x48;
+    rpc->aer_offset = 0x100;
+}
+
+static const TypeInfo pnv_phb_type_info = {
+    .name          = TYPE_PNV_PHB,
+    .parent        = TYPE_PCIE_HOST_BRIDGE,
+    .instance_size = sizeof(PnvPHB),
+    .class_init    = pnv_phb_class_init,
+};
+
+static const TypeInfo pnv_phb_root_port_info = {
+    .name          = TYPE_PNV_PHB_ROOT_PORT,
+    .parent        = TYPE_PCIE_ROOT_PORT,
+    .instance_size = sizeof(PnvPHBRootPort),
+    .class_init    = pnv_phb_root_port_class_init,
+};
+
+static void pnv_phb_register_types(void)
+{
     type_register_static(&pnv_phb_type_info);
+    type_register_static(&pnv_phb_root_port_info);
 }
-type_init(pnv_phb_register_type)
+
+type_init(pnv_phb_register_types)
diff --git a/hw/pci-host/pnv_phb.h b/hw/pci-host/pnv_phb.h
index a7cc8610e2..58ebd6dd0f 100644
--- a/hw/pci-host/pnv_phb.h
+++ b/hw/pci-host/pnv_phb.h
@@ -36,4 +36,20 @@ struct PnvPHB {
 #define TYPE_PNV_PHB "pnv-phb"
 OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB, PNV_PHB)
 
+/*
+ * PHB PCIe Root port
+ */
+#define PNV_PHB3_DEVICE_ID         0x03dc
+#define PNV_PHB4_DEVICE_ID         0x04c1
+#define PNV_PHB5_DEVICE_ID         0x0652
+
+typedef struct PnvPHBRootPort {
+    PCIESlot parent_obj;
+
+    uint32_t version;
+} PnvPHBRootPort;
+
+#define TYPE_PNV_PHB_ROOT_PORT "pnv-phb-root-port"
+OBJECT_DECLARE_SIMPLE_TYPE(PnvPHBRootPort, PNV_PHB_ROOT_PORT)
+
 #endif /* PCI_HOST_PNV_PHB_H */
-- 
2.36.1

On 24/06/2022 10:49, Daniel Henrique Barboza wrote:
> We have two very similar root-port devices, pnv-phb3-root-port and
> pnv-phb4-root-port. Both consist of a wrapper around the PCIESlot device
> that, until now, has no additional attributes.
> 
> The main difference between the PHB3 and PHB4 root ports is that
> pnv-phb4-root-port has the pnv_phb4_root_port_reset() callback. All
> other differences can be merged in a single device without too much
> trouble.
> 
> This patch introduces the unified pnv-phb-root-port that, in time, will
> be used as the default root port for the pnv-phb device.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred


>   hw/pci-host/pnv_phb.c | 115 +++++++++++++++++++++++++++++++++++++++---
>   hw/pci-host/pnv_phb.h |  16 ++++++
>   2 files changed, 123 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
> index abcbcca445..5e61f85614 100644
> --- a/hw/pci-host/pnv_phb.c
> +++ b/hw/pci-host/pnv_phb.c
> @@ -112,15 +112,114 @@ static void pnv_phb_class_init(ObjectClass *klass, void *data)
>       dc->user_creatable = false;
>   }
>   
> -static void pnv_phb_register_type(void)
> +static void pnv_phb_root_port_reset(DeviceState *dev)
>   {
> -    static const TypeInfo pnv_phb_type_info = {
> -        .name          = TYPE_PNV_PHB,
> -        .parent        = TYPE_PCIE_HOST_BRIDGE,
> -        .instance_size = sizeof(PnvPHB),
> -        .class_init    = pnv_phb_class_init,
> -    };
> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
> +    PnvPHBRootPort *phb_rp = PNV_PHB_ROOT_PORT(dev);
> +    PCIDevice *d = PCI_DEVICE(dev);
> +    uint8_t *conf = d->config;
>   
> +    rpc->parent_reset(dev);
> +
> +    if (phb_rp->version == 3) {
> +        return;
> +    }
> +
> +    /* PHB4 and later requires these extra reset steps */
> +    pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
> +                               PCI_IO_RANGE_MASK & 0xff);
> +    pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
> +                                 PCI_IO_RANGE_MASK & 0xff);
> +    pci_set_word(conf + PCI_MEMORY_BASE, 0);
> +    pci_set_word(conf + PCI_MEMORY_LIMIT, 0xfff0);
> +    pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0x1);
> +    pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1);
> +    pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */
> +    pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0xffffffff);
> +    pci_config_set_interrupt_pin(conf, 0);
> +}
> +
> +static void pnv_phb_root_port_realize(DeviceState *dev, Error **errp)
> +{
> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
> +    PnvPHBRootPort *phb_rp = PNV_PHB_ROOT_PORT(dev);
> +    PCIDevice *pci = PCI_DEVICE(dev);
> +    uint16_t device_id = 0;
> +    Error *local_err = NULL;
> +
> +    rpc->parent_realize(dev, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    switch (phb_rp->version) {
> +    case 3:
> +        device_id = PNV_PHB3_DEVICE_ID;
> +        break;
> +    case 4:
> +        device_id = PNV_PHB4_DEVICE_ID;
> +        break;
> +    case 5:
> +        device_id = PNV_PHB5_DEVICE_ID;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    pci_config_set_device_id(pci->config, device_id);
> +    pci_config_set_interrupt_pin(pci->config, 0);
> +}
> +
> +static Property pnv_phb_root_port_properties[] = {
> +    DEFINE_PROP_UINT32("version", PnvPHBRootPort, version, 0),
> +
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pnv_phb_root_port_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
> +
> +    dc->desc     = "IBM PHB PCIE Root Port";
> +
> +    device_class_set_props(dc, pnv_phb_root_port_properties);
> +    device_class_set_parent_realize(dc, pnv_phb_root_port_realize,
> +                                    &rpc->parent_realize);
> +    device_class_set_parent_reset(dc, pnv_phb_root_port_reset,
> +                                  &rpc->parent_reset);
> +    dc->reset = &pnv_phb_root_port_reset;
> +    dc->user_creatable = false;
> +
> +    k->vendor_id = PCI_VENDOR_ID_IBM;
> +    /* device_id will be written during realize() */
> +    k->device_id = 0;
> +    k->revision  = 0;
> +
> +    rpc->exp_offset = 0x48;
> +    rpc->aer_offset = 0x100;
> +}
> +
> +static const TypeInfo pnv_phb_type_info = {
> +    .name          = TYPE_PNV_PHB,
> +    .parent        = TYPE_PCIE_HOST_BRIDGE,
> +    .instance_size = sizeof(PnvPHB),
> +    .class_init    = pnv_phb_class_init,
> +};
> +
> +static const TypeInfo pnv_phb_root_port_info = {
> +    .name          = TYPE_PNV_PHB_ROOT_PORT,
> +    .parent        = TYPE_PCIE_ROOT_PORT,
> +    .instance_size = sizeof(PnvPHBRootPort),
> +    .class_init    = pnv_phb_root_port_class_init,
> +};
> +
> +static void pnv_phb_register_types(void)
> +{
>       type_register_static(&pnv_phb_type_info);
> +    type_register_static(&pnv_phb_root_port_info);
>   }
> -type_init(pnv_phb_register_type)
> +
> +type_init(pnv_phb_register_types)
> diff --git a/hw/pci-host/pnv_phb.h b/hw/pci-host/pnv_phb.h
> index a7cc8610e2..58ebd6dd0f 100644
> --- a/hw/pci-host/pnv_phb.h
> +++ b/hw/pci-host/pnv_phb.h
> @@ -36,4 +36,20 @@ struct PnvPHB {
>   #define TYPE_PNV_PHB "pnv-phb"
>   OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB, PNV_PHB)
>   
> +/*
> + * PHB PCIe Root port
> + */
> +#define PNV_PHB3_DEVICE_ID         0x03dc
> +#define PNV_PHB4_DEVICE_ID         0x04c1
> +#define PNV_PHB5_DEVICE_ID         0x0652
> +
> +typedef struct PnvPHBRootPort {
> +    PCIESlot parent_obj;
> +
> +    uint32_t version;
> +} PnvPHBRootPort;
> +
> +#define TYPE_PNV_PHB_ROOT_PORT "pnv-phb-root-port"
> +OBJECT_DECLARE_SIMPLE_TYPE(PnvPHBRootPort, PNV_PHB_ROOT_PORT)
> +
>   #endif /* PCI_HOST_PNV_PHB_H */