On 6/1/22 02:56, Cédric Le Goater wrote:
On 5/31/22 23: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>
---
hw/pci-host/pnv_phb.c | 107 ++++++++++++++++++++++++++++++++++++++----
hw/pci-host/pnv_phb.h | 17 +++++++
2 files changed, 116 insertions(+), 8 deletions(-)
diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index 321c4e768a..5047e90d3a 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -114,15 +114,106 @@ static void pnv_phb_class_init(ObjectClass *klass, void
*data)
dc->user_creatable = true;
}
-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 *rootport = PNV_PHB_ROOT_PORT(dev);
+ PCIDevice *d = PCI_DEVICE(dev);
+ uint8_t *conf = d->config;
+ rpc->parent_reset(dev);
+
+ if (rootport->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);
+ PCIDevice *pci = PCI_DEVICE(dev);
+ PCIBus *bus = pci_get_bus(pci);
+ PnvPHB *phb = NULL;
+ Error *local_err = NULL;
+
+ phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
+ TYPE_PNV_PHB);
+
+ if (!phb) {
+ error_setg(errp,
+"pnv_phb_root_port devices must be connected to pnv-phb buses");
+ return;
+ }
+
+ /* Set unique chassis/slot values for the root port */
+ qdev_prop_set_uint8(&pci->qdev, "chassis", phb->chip_id);
+ qdev_prop_set_uint16(&pci->qdev, "slot", phb->phb_id);
+
+ rpc->parent_realize(dev, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ pci_config_set_interrupt_pin(pci->config, 0);
+}
+
+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_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 = true;
+
+ k->vendor_id = PCI_VENDOR_ID_IBM;
+ /* device_id represents the latest PHB root port version supported */
+ k->device_id = PNV_PHB5_DEVICE_ID;
does that mean powernv8 machines will see phb devices as phb5 devices ?
I had something like this in this patch that would set device_id properly:
diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index 5d66264a96..d468e8d44a 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -144,6 +144,22 @@ static void pnv_phb_root_port_realize(DeviceState *dev, Error
**errp)
PCIBus *bus = pci_get_bus(pci);
PnvPHB *phb = NULL;
Error *local_err = NULL;
+ PnvPHBRootPort *phb_rp = PNV_PHB_ROOT_PORT(dev);
+ PCIDeviceClass *k = PCI_DEVICE_GET_CLASS(pci);
+
+ switch (phb_rp->version) {
+ case 3:
+ k->device_id = PNV_PHB3_DEVICE_ID;
+ break;
+ case 4:
+ k->device_id = PNV_PHB4_DEVICE_ID;
+ break;
+ case 5:
+ k->device_id = PNV_PHB5_DEVICE_ID;
+ break;
+ default:
+ g_assert_not_reached();
+ }
phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
TYPE_PNV_PHB);
@@ -166,6 +182,11 @@ static void pnv_phb_root_port_realize(DeviceState *dev, Error
**errp)
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);
@@ -181,6 +202,7 @@ static void pnv_phb_root_port_class_init(ObjectClass *klass, void
*data)
&rpc->parent_reset);
dc->reset = &pnv_phb_root_port_reset;
+ device_class_set_props(dc, pnv_phb_root_port_properties);
dc->user_creatable = true;
k->vendor_id = PCI_VENDOR_ID_IBM;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 4d2ea405db..13c8753eb2 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2148,6 +2148,7 @@ static void pnv_machine_power8_class_init(ObjectClass *oc, void
*data)
static GlobalProperty phb_compat[] = {
{ TYPE_PNV_PHB, "version", "3" },
+ { TYPE_PNV_PHB_ROOT_PORT, "version", "3" },
};
mc->desc = "IBM PowerNV (Non-Virtualized) POWER8";
@@ -2173,6 +2174,7 @@ static void pnv_machine_power9_class_init(ObjectClass *oc, void
*data)
static GlobalProperty phb_compat[] = {
{ TYPE_PNV_PHB, "version", "4" },
+ { TYPE_PNV_PHB_ROOT_PORT, "version", "4" },
};
mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
@@ -2199,6 +2201,7 @@ static void pnv_machine_power10_class_init(ObjectClass *oc,
void *data)
static GlobalProperty phb_compat[] = {
{ TYPE_PNV_PHB, "version", "5" },
+ { TYPE_PNV_PHB_ROOT_PORT, "version", "5" },
};
mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
The reason I didn't follow it through is because I wasn't sure if setting
k->device_id
during realize() time was acceptable. Everyone else seems to set k->device_id
during
class_init() or via an extra attribute 'device_id' that is written directly into
the PCI header.
If this is something that we can do then I'm fine with fixing this up in this
patch.