We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra
root complexes to be associated with SMMU.
Although this change does not affect functionality at present, it is
required when we add support for user-creatable SMMUv3 devices in
future patches.
Note: Added a specific check to identify pxb-pcie to avoid matching
pxb-cxl host bridges, which are also of type PCI_HOST_BRIDGE. This
restriction can be relaxed once support for CXL devices on arm/virt
is added and validated with SMMUv3.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Nathan Chen <nathanc@nvidia.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
hw/arm/smmu-common.c | 29 ++++++++++++++++++++++++++---
hw/pci-bridge/pci_expander_bridge.c | 1 -
include/hw/pci/pci_bridge.h | 1 +
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index f39b99e526..b15e7fd0e4 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -20,6 +20,7 @@
#include "trace.h"
#include "exec/target_page.h"
#include "hw/core/cpu.h"
+#include "hw/pci/pci_bridge.h"
#include "hw/qdev-properties.h"
#include "qapi/error.h"
#include "qemu/jhash.h"
@@ -925,6 +926,7 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
{
SMMUState *s = ARM_SMMU(dev);
SMMUBaseClass *sbc = ARM_SMMU_GET_CLASS(dev);
+ PCIBus *pci_bus = s->primary_bus;
Error *local_err = NULL;
sbc->parent_realize(dev, &local_err);
@@ -937,11 +939,32 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
g_free, g_free);
s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL);
- if (s->primary_bus) {
- pci_setup_iommu(s->primary_bus, &smmu_ops, s);
- } else {
+ if (!pci_bus) {
error_setg(errp, "SMMU is not attached to any PCI bus!");
+ return;
+ }
+
+ /*
+ * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra
+ * root complexes to be associated with SMMU.
+ */
+ if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
+ object_dynamic_cast(OBJECT(pci_bus)->parent, TYPE_PCI_HOST_BRIDGE)) {
+ /*
+ * For pxb-pcie, parent_dev will be set. Make sure it is
+ * pxb-pcie indeed.
+ */
+ if (pci_bus->parent_dev) {
+ if (!object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)) {
+ goto out_err;
+ }
+ }
+ pci_setup_iommu(pci_bus, &smmu_ops, s);
+ return;
}
+out_err:
+ error_setg(errp, "SMMU should be attached to a default PCIe root complex"
+ "(pcie.0) or a pxb-pcie based root complex");
}
/*
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 3a29dfefc2..1bcceddbc4 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -34,7 +34,6 @@ typedef struct PXBBus PXBBus;
DECLARE_INSTANCE_CHECKER(PXBBus, PXB_BUS,
TYPE_PXB_BUS)
-#define TYPE_PXB_PCIE_BUS "pxb-pcie-bus"
DECLARE_INSTANCE_CHECKER(PXBBus, PXB_PCIE_BUS,
TYPE_PXB_PCIE_BUS)
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 8cdacbc4e1..a055fd8d32 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -104,6 +104,7 @@ typedef struct PXBPCIEDev {
PXBDev parent_obj;
} PXBPCIEDev;
+#define TYPE_PXB_PCIE_BUS "pxb-pcie-bus"
#define TYPE_PXB_CXL_BUS "pxb-cxl-bus"
#define TYPE_PXB_DEV "pxb"
OBJECT_DECLARE_SIMPLE_TYPE(PXBDev, PXB_DEV)
--
2.47.0On Tue, Jul 08, 2025 at 04:40:45PM +0100, Shameer Kolothum wrote:
> We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra
> root complexes to be associated with SMMU.
>
> Although this change does not affect functionality at present, it is
> required when we add support for user-creatable SMMUv3 devices in
> future patches.
>
> Note: Added a specific check to identify pxb-pcie to avoid matching
> pxb-cxl host bridges, which are also of type PCI_HOST_BRIDGE. This
> restriction can be relaxed once support for CXL devices on arm/virt
> is added and validated with SMMUv3.
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Nathan Chen <nathanc@nvidia.com>
> Tested-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
With a small suggestion for clarification.
> + /*
> + * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra
> + * root complexes to be associated with SMMU.
> + */
> + if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
> + object_dynamic_cast(OBJECT(pci_bus)->parent, TYPE_PCI_HOST_BRIDGE)) {
> + /*
> + * For pxb-pcie, parent_dev will be set. Make sure it is
> + * pxb-pcie indeed.
> + */
/*
* While pcie.0 doesn't set the parent_dev, either pxb-pcie or pxb-cxl
* does. Re-test the type to make sure it is pxb-pcie indeed.
*/
> + if (pci_bus->parent_dev) {
> + if (!object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)) {
> + goto out_err;
On Tue, Jul 08, 2025 at 04:40:45PM +0100, Shameer Kolothum wrote:
> @@ -937,11 +939,32 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
> g_free, g_free);
> s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL);
Although this is not introduced by this patch, is there a
g_hash_table_remove() somewhere in the code?
> + /*
> + * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra
> + * root complexes to be associated with SMMU.
> + */
> + if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
> + object_dynamic_cast(OBJECT(pci_bus)->parent, TYPE_PCI_HOST_BRIDGE)) {
> + /*
> + * For pxb-pcie, parent_dev will be set. Make sure it is
> + * pxb-pcie indeed.
> + */
> + if (pci_bus->parent_dev) {
> + if (!object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)) {
The pci_bus_is_express(pci_bus) at the top is equivalent to:
object_dynamic_cast(OBJECT(pci_bus), TYPE_PCIE_BUS)
Then here it is doing:
object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)
So, this checks the same pci_bus but expects two different types?
I don't see the code check "PCIe Root Complex" explicitly, which
should be TYPE_GPEX_HOST?
Thanks
Nicolin
> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Tuesday, July 8, 2025 9:57 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> gustavo.romero@linaro.org; mst@redhat.com;
> marcel.apfelbaum@gmail.com; Linuxarm <linuxarm@huawei.com>;
> Wangzhou (B) <wangzhou1@hisilicon.com>; jiangkunkun
> <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH v7 02/12] hw/arm/smmu-common: Check SMMU has
> PCIe Root Complex association
>
> On Tue, Jul 08, 2025 at 04:40:45PM +0100, Shameer Kolothum wrote:
> > @@ -937,11 +939,32 @@ static void smmu_base_realize(DeviceState
> *dev, Error **errp)
> > g_free, g_free);
> > s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL);
>
> Although this is not introduced by this patch, is there a
> g_hash_table_remove() somewhere in the code?
g_hash_table_remove() is to remove a key/value pair, isn't it? Or you meant
a corresponding free in case of failure here? It's a realize() fn and errp is set
if something goes wrong and QEMU will exit. Not sure we need an explicit
free here.
> > + /*
> > + * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based
> extra
> > + * root complexes to be associated with SMMU.
> > + */
> > + if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
> > + object_dynamic_cast(OBJECT(pci_bus)->parent,
> TYPE_PCI_HOST_BRIDGE)) {
> > + /*
> > + * For pxb-pcie, parent_dev will be set. Make sure it is
> > + * pxb-pcie indeed.
> > + */
> > + if (pci_bus->parent_dev) {
> > + if (!object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)) {
>
> The pci_bus_is_express(pci_bus) at the top is equivalent to:
> object_dynamic_cast(OBJECT(pci_bus), TYPE_PCIE_BUS)
> Then here it is doing:
> object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)
Yes.
> So, this checks the same pci_bus but expects two different types?
In QEMU, we can have three types of PCIe root complexes to be specified for
virt machine.
1. default pcie.0 (TYPE_GPEX_HOST --> TYPE_PCIE_HOST_BRIDGE --> TYPE_PCI_HOST_BRIDGE)
2. pxb-pcie (TYPE_PXB_HOST -->TYPE_PCI_HOST_BRIDGE)
2. pxb-cxl (TYPE_PXB_CXL_HOST --> TYPE_PCI_HOST_BRIDGE)
The above first check is to see whether the bus is PCIE && root bus && parent
of type TYPE_PCI_HOST_BRIDGE. This will identify all the above three cases.
Both pxb-pcie and pxb-cxl are special extra root complexes based on PCI
expansion bridges and has a parent_dev set(both has pcie.0 has parent bus).
Hence we check to see parent_dev is set and make sure it is indeed
TYPE_PXB_PCIE_BUS to avoid attaching to pxb-cxl.
As mentioned in the commit log above, cxl support for virt is currently in
progress and once it has verified the functionality with SMMUv3
we can relax that check.
> I don't see the code check "PCIe Root Complex" explicitly, which
> should be TYPE_GPEX_HOST?
Hope it is clear now.
Thanks,
Shameer
On Wed, Jul 09, 2025 at 08:08:49AM +0000, Shameerali Kolothum Thodi wrote:
> > On Tue, Jul 08, 2025 at 04:40:45PM +0100, Shameer Kolothum wrote:
> > > @@ -937,11 +939,32 @@ static void smmu_base_realize(DeviceState
> > *dev, Error **errp)
> > > g_free, g_free);
> > > s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL);
> >
> > Although this is not introduced by this patch, is there a
> > g_hash_table_remove() somewhere in the code?
>
> g_hash_table_remove() is to remove a key/value pair, isn't it?
Yes.
> Or you meant
> a corresponding free in case of failure here?
Yes. But I saw the other two g_hash_table_new_full calls were not
reverted in the exit path either. Then I saw smmu_base_reset_exit
does the clean up of those two but not this smmu_pcibus_by_busptr.
> It's a realize() fn and errp is set
> if something goes wrong and QEMU will exit. Not sure we need an explicit
> free here.
>
> > > + /*
> > > + * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based
> > extra
> > > + * root complexes to be associated with SMMU.
> > > + */
> > > + if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
> > > + object_dynamic_cast(OBJECT(pci_bus)->parent,
> > TYPE_PCI_HOST_BRIDGE)) {
> > > + /*
> > > + * For pxb-pcie, parent_dev will be set. Make sure it is
> > > + * pxb-pcie indeed.
> > > + */
> > > + if (pci_bus->parent_dev) {
> > > + if (!object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)) {
> >
> > The pci_bus_is_express(pci_bus) at the top is equivalent to:
> > object_dynamic_cast(OBJECT(pci_bus), TYPE_PCIE_BUS)
> > Then here it is doing:
> > object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)
>
> Yes.
Hmm?
We have these two types defined as two different strings, right?
#define TYPE_PCIE_BUS "PCIE"
#define TYPE_PXB_PCIE_BUS "pxb-pcie-bus"
So the first test is to make sure pci_bus string is "PCIE",
then the second one testing the same pci_bus string will
never be true?
> > So, this checks the same pci_bus but expects two different types?
>
> In QEMU, we can have three types of PCIe root complexes to be specified for
> virt machine.
>
> 1. default pcie.0 (TYPE_GPEX_HOST --> TYPE_PCIE_HOST_BRIDGE --> TYPE_PCI_HOST_BRIDGE)
> 2. pxb-pcie (TYPE_PXB_HOST -->TYPE_PCI_HOST_BRIDGE)
> 2. pxb-cxl (TYPE_PXB_CXL_HOST --> TYPE_PCI_HOST_BRIDGE)
>
> The above first check is to see whether the bus is PCIE && root bus && parent
> of type TYPE_PCI_HOST_BRIDGE. This will identify all the above three cases.
>
> Both pxb-pcie and pxb-cxl are special extra root complexes based on PCI
> expansion bridges and has a parent_dev set(both has pcie.0 has parent bus).
>
> Hence we check to see parent_dev is set and make sure it is indeed
> TYPE_PXB_PCIE_BUS to avoid attaching to pxb-cxl.
I see. That's clear now. I think it'd help by writing:
/*
* While pcie.0 doesn't set the parent_dev, either pxb-pcie or
* pxb-cxl does. Re-test the type to make sure it is pxb-pcie.
*/
Thanks
Nicolin
> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Thursday, July 10, 2025 12:54 AM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> gustavo.romero@linaro.org; mst@redhat.com;
> marcel.apfelbaum@gmail.com; Linuxarm <linuxarm@huawei.com>;
> Wangzhou (B) <wangzhou1@hisilicon.com>; jiangkunkun
> <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH v7 02/12] hw/arm/smmu-common: Check SMMU has
> PCIe Root Complex association
>
> On Wed, Jul 09, 2025 at 08:08:49AM +0000, Shameerali Kolothum Thodi
> wrote:
> > > On Tue, Jul 08, 2025 at 04:40:45PM +0100, Shameer Kolothum wrote:
> > > > @@ -937,11 +939,32 @@ static void smmu_base_realize(DeviceState
> > > *dev, Error **errp)
> > > > g_free, g_free);
> > > > s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL);
> > >
> > > Although this is not introduced by this patch, is there a
> > > g_hash_table_remove() somewhere in the code?
> >
> > g_hash_table_remove() is to remove a key/value pair, isn't it?
>
> Yes.
>
> > Or you meant
> > a corresponding free in case of failure here?
>
> Yes. But I saw the other two g_hash_table_new_full calls were not
> reverted in the exit path either. Then I saw smmu_base_reset_exit
> does the clean up of those two but not this smmu_pcibus_by_busptr.
Ok. I think that is by design. The insert for busptr cache happens during
early stages of Qemu through get_address_space() callback and
smmu_base_reset_exit() is called after that, just before the Guest boot.
So if you clean that cache at that time , you need to handle it differently
at a later stage. Also I don't think it makes much sense to clear busptr
before the Guest boot as it is not going to become stale unlike configs
and iotlb.
> > It's a realize() fn and errp is set
> > if something goes wrong and QEMU will exit. Not sure we need an explicit
> > free here.
> >
> > > > + /*
> > > > + * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie
> based
> > > extra
> > > > + * root complexes to be associated with SMMU.
> > > > + */
> > > > + if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
> > > > + object_dynamic_cast(OBJECT(pci_bus)->parent,
> > > TYPE_PCI_HOST_BRIDGE)) {
> > > > + /*
> > > > + * For pxb-pcie, parent_dev will be set. Make sure it is
> > > > + * pxb-pcie indeed.
> > > > + */
> > > > + if (pci_bus->parent_dev) {
> > > > + if (!object_dynamic_cast(OBJECT(pci_bus),
> TYPE_PXB_PCIE_BUS)) {
> > >
> > > The pci_bus_is_express(pci_bus) at the top is equivalent to:
> > > object_dynamic_cast(OBJECT(pci_bus), TYPE_PCIE_BUS)
> > > Then here it is doing:
> > > object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)
> >
> > Yes.
>
> Hmm?
>
> We have these two types defined as two different strings, right?
>
> #define TYPE_PCIE_BUS "PCIE"
> #define TYPE_PXB_PCIE_BUS "pxb-pcie-bus"
>
> So the first test is to make sure pci_bus string is "PCIE",
> then the second one testing the same pci_bus string will
> never be true?
>
It will be true.
static const TypeInfo pxb_pcie_bus_info = {
.name = TYPE_PXB_PCIE_BUS,
.parent = TYPE_PCIE_BUS,
.instance_size = sizeof(PXBBus),
.class_init = pxb_bus_class_init,
};
TYPE_PXB_PCIE_BUS has a parent TYPE_PCIE_BUS. And the function
object_dynamic_cast() does the magic. It will return non-null for an
exact object type and also for its parents in the QOM hierarchy.
> > > So, this checks the same pci_bus but expects two different types?
>
> > In QEMU, we can have three types of PCIe root complexes to be specified
> for
> > virt machine.
> >
> > 1. default pcie.0 (TYPE_GPEX_HOST --> TYPE_PCIE_HOST_BRIDGE -->
> TYPE_PCI_HOST_BRIDGE)
> > 2. pxb-pcie (TYPE_PXB_HOST -->TYPE_PCI_HOST_BRIDGE)
> > 2. pxb-cxl (TYPE_PXB_CXL_HOST --> TYPE_PCI_HOST_BRIDGE)
> >
> > The above first check is to see whether the bus is PCIE && root bus &&
> parent
> > of type TYPE_PCI_HOST_BRIDGE. This will identify all the above three
> cases.
> >
> > Both pxb-pcie and pxb-cxl are special extra root complexes based on PCI
> > expansion bridges and has a parent_dev set(both has pcie.0 has parent
> bus).
> >
> > Hence we check to see parent_dev is set and make sure it is indeed
> > TYPE_PXB_PCIE_BUS to avoid attaching to pxb-cxl.
>
> I see. That's clear now. I think it'd help by writing:
> /*
> * While pcie.0 doesn't set the parent_dev, either pxb-pcie
> or
> * pxb-cxl does. Re-test the type to make sure it is pxb-pcie.
> */
I think it is already captured in the comments in this patch.
Thanks,
Shameer
On Thu, Jul 10, 2025 at 07:27:10AM +0000, Shameerali Kolothum Thodi wrote:
> > On Wed, Jul 09, 2025 at 08:08:49AM +0000, Shameerali Kolothum Thodi
> > wrote:
> > > > On Tue, Jul 08, 2025 at 04:40:45PM +0100, Shameer Kolothum wrote:
> > > > > @@ -937,11 +939,32 @@ static void smmu_base_realize(DeviceState
> > > > *dev, Error **errp)
> > > > > g_free, g_free);
> > > > > s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL);
> > > >
> > > > Although this is not introduced by this patch, is there a
> > > > g_hash_table_remove() somewhere in the code?
> > >
> > > g_hash_table_remove() is to remove a key/value pair, isn't it?
> >
> > Yes.
> >
> > > Or you meant
> > > a corresponding free in case of failure here?
> >
> > Yes. But I saw the other two g_hash_table_new_full calls were not
> > reverted in the exit path either. Then I saw smmu_base_reset_exit
> > does the clean up of those two but not this smmu_pcibus_by_busptr.
>
> Ok. I think that is by design. The insert for busptr cache happens during
> early stages of Qemu through get_address_space() callback and
> smmu_base_reset_exit() is called after that, just before the Guest boot.
> So if you clean that cache at that time , you need to handle it differently
> at a later stage. Also I don't think it makes much sense to clear busptr
> before the Guest boot as it is not going to become stale unlike configs
> and iotlb.
Hmm, my main point was there is seemingly no "g_hash_table_remove"
for s->smmu_pcibus_by_busptr throughout the vIOMMU code.
> > > It's a realize() fn and errp is set
> > > if something goes wrong and QEMU will exit. Not sure we need an explicit
> > > free here.
> > >
> > > > > + /*
> > > > > + * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie
> > based
> > > > extra
> > > > > + * root complexes to be associated with SMMU.
> > > > > + */
> > > > > + if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
> > > > > + object_dynamic_cast(OBJECT(pci_bus)->parent,
> > > > TYPE_PCI_HOST_BRIDGE)) {
> > > > > + /*
> > > > > + * For pxb-pcie, parent_dev will be set. Make sure it is
> > > > > + * pxb-pcie indeed.
> > > > > + */
> > > > > + if (pci_bus->parent_dev) {
> > > > > + if (!object_dynamic_cast(OBJECT(pci_bus),
> > TYPE_PXB_PCIE_BUS)) {
> > > >
> > > > The pci_bus_is_express(pci_bus) at the top is equivalent to:
> > > > object_dynamic_cast(OBJECT(pci_bus), TYPE_PCIE_BUS)
> > > > Then here it is doing:
> > > > object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)
> > >
> > > Yes.
> >
> > Hmm?
> >
> > We have these two types defined as two different strings, right?
> >
> > #define TYPE_PCIE_BUS "PCIE"
> > #define TYPE_PXB_PCIE_BUS "pxb-pcie-bus"
> >
> > So the first test is to make sure pci_bus string is "PCIE",
> > then the second one testing the same pci_bus string will
> > never be true?
> >
>
> It will be true.
>
> static const TypeInfo pxb_pcie_bus_info = {
> .name = TYPE_PXB_PCIE_BUS,
> .parent = TYPE_PCIE_BUS,
> .instance_size = sizeof(PXBBus),
> .class_init = pxb_bus_class_init,
> };
>
> TYPE_PXB_PCIE_BUS has a parent TYPE_PCIE_BUS. And the function
> object_dynamic_cast() does the magic. It will return non-null for an
> exact object type and also for its parents in the QOM hierarchy.
I see. Thanks for the explain.
> > > > So, this checks the same pci_bus but expects two different types?
> >
> > > In QEMU, we can have three types of PCIe root complexes to be specified
> > for
> > > virt machine.
> > >
> > > 1. default pcie.0 (TYPE_GPEX_HOST --> TYPE_PCIE_HOST_BRIDGE -->
> > TYPE_PCI_HOST_BRIDGE)
> > > 2. pxb-pcie (TYPE_PXB_HOST -->TYPE_PCI_HOST_BRIDGE)
> > > 2. pxb-cxl (TYPE_PXB_CXL_HOST --> TYPE_PCI_HOST_BRIDGE)
> > >
> > > The above first check is to see whether the bus is PCIE && root bus &&
> > parent
> > > of type TYPE_PCI_HOST_BRIDGE. This will identify all the above three
> > cases.
> > >
> > > Both pxb-pcie and pxb-cxl are special extra root complexes based on PCI
> > > expansion bridges and has a parent_dev set(both has pcie.0 has parent
> > bus).
> > >
> > > Hence we check to see parent_dev is set and make sure it is indeed
> > > TYPE_PXB_PCIE_BUS to avoid attaching to pxb-cxl.
> >
> > I see. That's clear now. I think it'd help by writing:
> > /*
> > * While pcie.0 doesn't set the parent_dev, either pxb-pcie
> > or
> > * pxb-cxl does. Re-test the type to make sure it is pxb-pcie.
> > */
>
> I think it is already captured in the comments in this patch.
But I couldn't understand until your further clarification in the
mail :(
Thanks
Nicolin
© 2016 - 2025 Red Hat, Inc.