Although this change does not affect functionality at present, it is
required when we add support for user-creatable SMMUv3 devices in
future patches.
Tested-by: Nathan Chen <nathanc@nvidia.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
hw/arm/smmu-common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index f39b99e526..7890aa12c1 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"
@@ -937,7 +938,8 @@ 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) {
+ if (s->primary_bus && object_dynamic_cast(OBJECT(s->primary_bus)->parent,
+ TYPE_PCI_HOST_BRIDGE)) {
pci_setup_iommu(s->primary_bus, &smmu_ops, s);
} else {
error_setg(errp, "SMMU is not attached to any PCI bus!");
--
2.47.0
On Fri, 13 Jun 2025 15:44:43 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> Although this change does not affect functionality at present, it is
Patch title says PCIe. This check is vs PCI host bridge.
No idea which one you wanted, but if it is PCIe needs to be
TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
I think.
> required when we add support for user-creatable SMMUv3 devices in
> future patches.
>
> Tested-by: Nathan Chen <nathanc@nvidia.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> hw/arm/smmu-common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index f39b99e526..7890aa12c1 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"
> @@ -937,7 +938,8 @@ 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) {
> + if (s->primary_bus && object_dynamic_cast(OBJECT(s->primary_bus)->parent,
> + TYPE_PCI_HOST_BRIDGE)) {
> pci_setup_iommu(s->primary_bus, &smmu_ops, s);
> } else {
> error_setg(errp, "SMMU is not attached to any PCI bus!");
On 6/16/25 12:20 PM, Jonathan Cameron wrote:
> On Fri, 13 Jun 2025 15:44:43 +0100
> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
>
>> Although this change does not affect functionality at present, it is
> Patch title says PCIe. This check is vs PCI host bridge.
>
> No idea which one you wanted, but if it is PCIe needs to be
> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
> I think.
I think we need TYPE_PCI_HOST_BRIDGE as we want to check against pxb
pci-bridge/pci_expander_bridge.c: .parent = TYPE_PCI_HOST_BRIDGE,
Besides the commit title/desc suggested by Jonathan you may also refine
the error msg which can stem now from 2 different causes
Thanks
Eric
>
>> required when we add support for user-creatable SMMUv3 devices in
>> future patches.
>>
>> Tested-by: Nathan Chen <nathanc@nvidia.com>
>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
>> ---
>> hw/arm/smmu-common.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>> index f39b99e526..7890aa12c1 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"
>> @@ -937,7 +938,8 @@ 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) {
>> + if (s->primary_bus && object_dynamic_cast(OBJECT(s->primary_bus)->parent,
>> + TYPE_PCI_HOST_BRIDGE)) {
>> pci_setup_iommu(s->primary_bus, &smmu_ops, s);
>> } else {
>> error_setg(errp, "SMMU is not attached to any PCI bus!");
On Tue, 17 Jun 2025 09:49:54 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> On 6/16/25 12:20 PM, Jonathan Cameron wrote:
> > On Fri, 13 Jun 2025 15:44:43 +0100
> > Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> >
> >> Although this change does not affect functionality at present, it is
> > Patch title says PCIe. This check is vs PCI host bridge.
> >
> > No idea which one you wanted, but if it is PCIe needs to be
> > TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
> > I think.
> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against pxb
>
> pci-bridge/pci_expander_bridge.c: .parent = TYPE_PCI_HOST_BRIDGE,
Hmm. That's awkward and I'd forgotten that wrinkle.
Need a stronger test but which one? The PXB root bus has a parent of
TYPE_PCIE_BUS. Maybe we can check that?
>
> Besides the commit title/desc suggested by Jonathan you may also refine
> the error msg which can stem now from 2 different causes
>
> Thanks
>
> Eric
> >
> >> required when we add support for user-creatable SMMUv3 devices in
> >> future patches.
> >>
> >> Tested-by: Nathan Chen <nathanc@nvidia.com>
> >> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> >> ---
> >> hw/arm/smmu-common.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> >> index f39b99e526..7890aa12c1 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"
> >> @@ -937,7 +938,8 @@ 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) {
> >> + if (s->primary_bus && object_dynamic_cast(OBJECT(s->primary_bus)->parent,
> >> + TYPE_PCI_HOST_BRIDGE)) {
> >> pci_setup_iommu(s->primary_bus, &smmu_ops, s);
> >> } else {
> >> error_setg(errp, "SMMU is not attached to any PCI bus!");
>
>
> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: Tuesday, June 17, 2025 5:53 PM
> To: Eric Auger <eric.auger@redhat.com>
> Cc: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Linuxarm
> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com;
> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com;
> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe
> Root Complex association
>
> On Tue, 17 Jun 2025 09:49:54 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
> > On 6/16/25 12:20 PM, Jonathan Cameron wrote:
> > > On Fri, 13 Jun 2025 15:44:43 +0100
> > > Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> > >
> > >> Although this change does not affect functionality at present, it is
> > > Patch title says PCIe. This check is vs PCI host bridge.
> > >
> > > No idea which one you wanted, but if it is PCIe needs to be
> > > TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
> > > I think.
> > I think we need TYPE_PCI_HOST_BRIDGE as we want to check against pxb
> >
> > pci-bridge/pci_expander_bridge.c: .parent =
> TYPE_PCI_HOST_BRIDGE,
>
> Hmm. That's awkward and I'd forgotten that wrinkle.
> Need a stronger test but which one? The PXB root bus has a parent of
> TYPE_PCIE_BUS. Maybe we can check that?
Ok. How about we do something like below?
@@ -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,10 +939,31 @@ 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), "pxb-pcie-bus")) {
+ error_setg(errp, "SMMU is not attached to pxb-pcie bus!");
+ return;
+ }
+ }
+ pci_setup_iommu(pci_bus, &smmu_ops, s);
+ } else {
+ error_setg(errp, "SMMU should be attached to a default PCIe
root complex"
+ "(pcie.0) or a pxb-pcie based root complex");
}
}
Please let me know if this is good enough or not.
Thanks,
Shameer
Hi Shameer, Jonathan,
On 6/18/25 10:35 AM, Shameerali Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Jonathan Cameron <jonathan.cameron@huawei.com>
>> Sent: Tuesday, June 17, 2025 5:53 PM
>> To: Eric Auger <eric.auger@redhat.com>
>> Cc: Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; Linuxarm
>> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu-
>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com;
>> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com;
>> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
>> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org
>> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe
>> Root Complex association
>>
>> On Tue, 17 Jun 2025 09:49:54 +0200
>> Eric Auger <eric.auger@redhat.com> wrote:
>>
>>> On 6/16/25 12:20 PM, Jonathan Cameron wrote:
>>>> On Fri, 13 Jun 2025 15:44:43 +0100
>>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
>>>>
>>>>> Although this change does not affect functionality at present, it is
>>>> Patch title says PCIe. This check is vs PCI host bridge.
>>>>
>>>> No idea which one you wanted, but if it is PCIe needs to be
>>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
>>>> I think.
>>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against pxb
>>>
>>> pci-bridge/pci_expander_bridge.c: .parent =
>> TYPE_PCI_HOST_BRIDGE,
sorry but I still fail to understand why we can't just check against
TYPE_PCI_HOST_BRIDGE for making sure the SMMU is attached to PXB or GPEX. What does it fail to check? Why shall we care about PCI vs PCIe?
Thanks
Eric
>>
>> Hmm. That's awkward and I'd forgotten that wrinkle.
>> Need a stronger test but which one? The PXB root bus has a parent of
>> TYPE_PCIE_BUS. Maybe we can check that?
> Ok. How about we do something like below?
>
>
> @@ -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,10 +939,31 @@ 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), "pxb-pcie-bus")) {
> + error_setg(errp, "SMMU is not attached to pxb-pcie bus!");
> + return;
> + }
> + }
> + pci_setup_iommu(pci_bus, &smmu_ops, s);
> + } else {
> + error_setg(errp, "SMMU should be attached to a default PCIe
> root complex"
> + "(pcie.0) or a pxb-pcie based root complex");
> }
> }
>
> Please let me know if this is good enough or not.
>
> Thanks,
> Shameer
>
Hi Eric, > -----Original Message----- > From: Eric Auger <eric.auger@redhat.com> > Sent: Wednesday, June 18, 2025 6:00 PM > To: Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > <jonathan.cameron@huawei.com> > Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe > Root Complex association > > Hi Shameer, Jonathan, > > On 6/18/25 10:35 AM, Shameerali Kolothum Thodi wrote: > > > >> -----Original Message----- > >> From: Jonathan Cameron <jonathan.cameron@huawei.com> > >> Sent: Tuesday, June 17, 2025 5:53 PM > >> To: Eric Auger <eric.auger@redhat.com> > >> Cc: Shameerali Kolothum Thodi > >> <shameerali.kolothum.thodi@huawei.com>; Linuxarm > >> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > >> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > >> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > >> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > >> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > >> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > >> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > PCIe > >> Root Complex association > >> > >> On Tue, 17 Jun 2025 09:49:54 +0200 > >> Eric Auger <eric.auger@redhat.com> wrote: > >> > >>> On 6/16/25 12:20 PM, Jonathan Cameron wrote: > >>>> On Fri, 13 Jun 2025 15:44:43 +0100 > >>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: > >>>> > >>>>> Although this change does not affect functionality at present, it is > >>>> Patch title says PCIe. This check is vs PCI host bridge. > >>>> > >>>> No idea which one you wanted, but if it is PCIe needs to be > >>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one > >>>> I think. > >>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against > pxb > >>> > >>> pci-bridge/pci_expander_bridge.c: .parent = > >> TYPE_PCI_HOST_BRIDGE, > sorry but I still fail to understand why we can't just check against > > TYPE_PCI_HOST_BRIDGE for making sure the SMMU is attached to PXB or > GPEX. What does it fail to check? Why shall we care about PCI vs PCIe? I think the concern is getting any other TYPE_PCI_HOST_BRIDGE types attached to SMMUv3 other than pxb-pcie or GPEX. For example you could do, -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ -device arm-smmuv3,primary-bus=cxl.1,id=smmuv3.1 \ as pxb-cxl is of type TYPE_PCI_HOST_BRIDGE. I don't know if there are any other ones similar to this out there. So the aim is to make the checking more specific to PXB. Thanks, Shameer
Hi Shameer, On 6/19/25 9:24 AM, Shameerali Kolothum Thodi wrote: > Hi Eric, > >> -----Original Message----- >> From: Eric Auger <eric.auger@redhat.com> >> Sent: Wednesday, June 18, 2025 6:00 PM >> To: Shameerali Kolothum Thodi >> <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron >> <jonathan.cameron@huawei.com> >> Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- >> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; >> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; >> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; >> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; >> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org >> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe >> Root Complex association >> >> Hi Shameer, Jonathan, >> >> On 6/18/25 10:35 AM, Shameerali Kolothum Thodi wrote: >>>> -----Original Message----- >>>> From: Jonathan Cameron <jonathan.cameron@huawei.com> >>>> Sent: Tuesday, June 17, 2025 5:53 PM >>>> To: Eric Auger <eric.auger@redhat.com> >>>> Cc: Shameerali Kolothum Thodi >>>> <shameerali.kolothum.thodi@huawei.com>; Linuxarm >>>> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- >>>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; >>>> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; >>>> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; >>>> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; >>>> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org >>>> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has >> PCIe >>>> Root Complex association >>>> >>>> On Tue, 17 Jun 2025 09:49:54 +0200 >>>> Eric Auger <eric.auger@redhat.com> wrote: >>>> >>>>> On 6/16/25 12:20 PM, Jonathan Cameron wrote: >>>>>> On Fri, 13 Jun 2025 15:44:43 +0100 >>>>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: >>>>>> >>>>>>> Although this change does not affect functionality at present, it is >>>>>> Patch title says PCIe. This check is vs PCI host bridge. >>>>>> >>>>>> No idea which one you wanted, but if it is PCIe needs to be >>>>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one >>>>>> I think. >>>>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against >> pxb >>>>> pci-bridge/pci_expander_bridge.c: .parent = >>>> TYPE_PCI_HOST_BRIDGE, >> sorry but I still fail to understand why we can't just check against >> >> TYPE_PCI_HOST_BRIDGE for making sure the SMMU is attached to PXB or >> GPEX. What does it fail to check? Why shall we care about PCI vs PCIe? > I think the concern is getting any other TYPE_PCI_HOST_BRIDGE types attached > to SMMUv3 other than pxb-pcie or GPEX. For example you could do, > > -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ > -device arm-smmuv3,primary-bus=cxl.1,id=smmuv3.1 \ > > as pxb-cxl is of type TYPE_PCI_HOST_BRIDGE. I don't know if there are any other > ones similar to this out there. > > So the aim is to make the checking more specific to PXB. thank you for the clarification. Is it invalid to have the SMMU protecting RIDs comming from the pxb-cxl hierarchy? Eric > > Thanks, > Shameer
> -----Original Message----- > From: Eric Auger <eric.auger@redhat.com> > Sent: Thursday, June 19, 2025 8:41 AM > To: Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > <jonathan.cameron@huawei.com> > Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe > Root Complex association > > Hi Shameer, > > On 6/19/25 9:24 AM, Shameerali Kolothum Thodi wrote: > > Hi Eric, > > > >> -----Original Message----- > >> From: Eric Auger <eric.auger@redhat.com> > >> Sent: Wednesday, June 18, 2025 6:00 PM > >> To: Shameerali Kolothum Thodi > >> <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > >> <jonathan.cameron@huawei.com> > >> Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > >> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > >> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > >> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > >> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > >> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > >> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > PCIe > >> Root Complex association > >> > >> Hi Shameer, Jonathan, > >> > >> On 6/18/25 10:35 AM, Shameerali Kolothum Thodi wrote: > >>>> -----Original Message----- > >>>> From: Jonathan Cameron <jonathan.cameron@huawei.com> > >>>> Sent: Tuesday, June 17, 2025 5:53 PM > >>>> To: Eric Auger <eric.auger@redhat.com> > >>>> Cc: Shameerali Kolothum Thodi > >>>> <shameerali.kolothum.thodi@huawei.com>; Linuxarm > >>>> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > >>>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > >>>> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > >>>> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > >>>> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > >>>> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > >>>> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > >> PCIe > >>>> Root Complex association > >>>> > >>>> On Tue, 17 Jun 2025 09:49:54 +0200 > >>>> Eric Auger <eric.auger@redhat.com> wrote: > >>>> > >>>>> On 6/16/25 12:20 PM, Jonathan Cameron wrote: > >>>>>> On Fri, 13 Jun 2025 15:44:43 +0100 > >>>>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > wrote: > >>>>>> > >>>>>>> Although this change does not affect functionality at present, it is > >>>>>> Patch title says PCIe. This check is vs PCI host bridge. > >>>>>> > >>>>>> No idea which one you wanted, but if it is PCIe needs to be > >>>>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one > >>>>>> I think. > >>>>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against > >> pxb > >>>>> pci-bridge/pci_expander_bridge.c: .parent = > >>>> TYPE_PCI_HOST_BRIDGE, > >> sorry but I still fail to understand why we can't just check against > >> > >> TYPE_PCI_HOST_BRIDGE for making sure the SMMU is attached to PXB or > >> GPEX. What does it fail to check? Why shall we care about PCI vs PCIe? > > I think the concern is getting any other TYPE_PCI_HOST_BRIDGE types > attached > > to SMMUv3 other than pxb-pcie or GPEX. For example you could do, > > > > -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ > > -device arm-smmuv3,primary-bus=cxl.1,id=smmuv3.1 \ > > > > as pxb-cxl is of type TYPE_PCI_HOST_BRIDGE. I don't know if there are any > other > > ones similar to this out there. > > > > So the aim is to make the checking more specific to PXB. > > thank you for the clarification. Is it invalid to have the SMMU > protecting RIDs comming from the pxb-cxl hierarchy? That’s a good question. I don't know that for sure. Anyway currently the full support for CXL on virt is in progress here, https://lore.kernel.org/qemu-devel/20250612134338.1871023-1-Jonathan.Cameron@huawei.com/ Jonathan? Thanks, Shameer >
On Thu, 19 Jun 2025 09:05:07 +0100 Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > -----Original Message----- > > From: Eric Auger <eric.auger@redhat.com> > > Sent: Thursday, June 19, 2025 8:41 AM > > To: Shameerali Kolothum Thodi > > <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > > <jonathan.cameron@huawei.com> > > Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe > > Root Complex association > > > > Hi Shameer, > > > > On 6/19/25 9:24 AM, Shameerali Kolothum Thodi wrote: > > > Hi Eric, > > > > > >> -----Original Message----- > > >> From: Eric Auger <eric.auger@redhat.com> > > >> Sent: Wednesday, June 18, 2025 6:00 PM > > >> To: Shameerali Kolothum Thodi > > >> <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > > >> <jonathan.cameron@huawei.com> > > >> Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > >> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > >> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > >> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > >> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > >> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > >> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > > PCIe > > >> Root Complex association > > >> > > >> Hi Shameer, Jonathan, > > >> > > >> On 6/18/25 10:35 AM, Shameerali Kolothum Thodi wrote: > > >>>> -----Original Message----- > > >>>> From: Jonathan Cameron <jonathan.cameron@huawei.com> > > >>>> Sent: Tuesday, June 17, 2025 5:53 PM > > >>>> To: Eric Auger <eric.auger@redhat.com> > > >>>> Cc: Shameerali Kolothum Thodi > > >>>> <shameerali.kolothum.thodi@huawei.com>; Linuxarm > > >>>> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > >>>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > >>>> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > >>>> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > >>>> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > >>>> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > >>>> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > > >> PCIe > > >>>> Root Complex association > > >>>> > > >>>> On Tue, 17 Jun 2025 09:49:54 +0200 > > >>>> Eric Auger <eric.auger@redhat.com> wrote: > > >>>> > > >>>>> On 6/16/25 12:20 PM, Jonathan Cameron wrote: > > >>>>>> On Fri, 13 Jun 2025 15:44:43 +0100 > > >>>>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > wrote: > > >>>>>> > > >>>>>>> Although this change does not affect functionality at present, it is > > >>>>>> Patch title says PCIe. This check is vs PCI host bridge. > > >>>>>> > > >>>>>> No idea which one you wanted, but if it is PCIe needs to be > > >>>>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one > > >>>>>> I think. > > >>>>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against > > >> pxb > > >>>>> pci-bridge/pci_expander_bridge.c: .parent = > > >>>> TYPE_PCI_HOST_BRIDGE, > > >> sorry but I still fail to understand why we can't just check against > > >> > > >> TYPE_PCI_HOST_BRIDGE for making sure the SMMU is attached to PXB or > > >> GPEX. What does it fail to check? Why shall we care about PCI vs PCIe? > > > I think the concern is getting any other TYPE_PCI_HOST_BRIDGE types > > attached > > > to SMMUv3 other than pxb-pcie or GPEX. For example you could do, > > > > > > -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ > > > -device arm-smmuv3,primary-bus=cxl.1,id=smmuv3.1 \ > > > > > > as pxb-cxl is of type TYPE_PCI_HOST_BRIDGE. I don't know if there are any > > other > > > ones similar to this out there. > > > > > > So the aim is to make the checking more specific to PXB. > > > > thank you for the clarification. Is it invalid to have the SMMU > > protecting RIDs comming from the pxb-cxl hierarchy? > > That’s a good question. I don't know that for sure. It should be fine to support CXL for this but we can work that out later. For now limited use cases as there is no CXL VFIO support and the only thing emulated devices do that the SMMU might influence is MSIX. The one that concerned me is pxb-pci if we only care about pcie. I'm not sure if we need to make that distinction or not. Jonathan > Anyway currently the full support for CXL on virt is in progress here, > https://lore.kernel.org/qemu-devel/20250612134338.1871023-1-Jonathan.Cameron@huawei.com/ > > Jonathan? > > Thanks, > Shameer > > > >
On Thu, 19 Jun 2025 10:30:28 +0100 Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > On Thu, 19 Jun 2025 09:05:07 +0100 > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > > > -----Original Message----- > > > From: Eric Auger <eric.auger@redhat.com> > > > Sent: Thursday, June 19, 2025 8:41 AM > > > To: Shameerali Kolothum Thodi > > > <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > > > <jonathan.cameron@huawei.com> > > > Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > > devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > > nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > > imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > > smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > > jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > > Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe > > > Root Complex association > > > > > > Hi Shameer, > > > > > > On 6/19/25 9:24 AM, Shameerali Kolothum Thodi wrote: > > > > Hi Eric, > > > > > > > >> -----Original Message----- > > > >> From: Eric Auger <eric.auger@redhat.com> > > > >> Sent: Wednesday, June 18, 2025 6:00 PM > > > >> To: Shameerali Kolothum Thodi > > > >> <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > > > >> <jonathan.cameron@huawei.com> > > > >> Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > > >> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > > >> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > > >> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > > >> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > > >> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > > >> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > > > PCIe > > > >> Root Complex association > > > >> > > > >> Hi Shameer, Jonathan, > > > >> > > > >> On 6/18/25 10:35 AM, Shameerali Kolothum Thodi wrote: > > > >>>> -----Original Message----- > > > >>>> From: Jonathan Cameron <jonathan.cameron@huawei.com> > > > >>>> Sent: Tuesday, June 17, 2025 5:53 PM > > > >>>> To: Eric Auger <eric.auger@redhat.com> > > > >>>> Cc: Shameerali Kolothum Thodi > > > >>>> <shameerali.kolothum.thodi@huawei.com>; Linuxarm > > > >>>> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > > >>>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > > >>>> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > > >>>> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > > >>>> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > > >>>> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > > >>>> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > > > >> PCIe > > > >>>> Root Complex association > > > >>>> > > > >>>> On Tue, 17 Jun 2025 09:49:54 +0200 > > > >>>> Eric Auger <eric.auger@redhat.com> wrote: > > > >>>> > > > >>>>> On 6/16/25 12:20 PM, Jonathan Cameron wrote: > > > >>>>>> On Fri, 13 Jun 2025 15:44:43 +0100 > > > >>>>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > > wrote: > > > >>>>>> > > > >>>>>>> Although this change does not affect functionality at present, it is > > > >>>>>> Patch title says PCIe. This check is vs PCI host bridge. > > > >>>>>> > > > >>>>>> No idea which one you wanted, but if it is PCIe needs to be > > > >>>>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one > > > >>>>>> I think. > > > >>>>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against > > > >> pxb > > > >>>>> pci-bridge/pci_expander_bridge.c: .parent = > > > >>>> TYPE_PCI_HOST_BRIDGE, > > > >> sorry but I still fail to understand why we can't just check against > > > >> > > > >> TYPE_PCI_HOST_BRIDGE for making sure the SMMU is attached to PXB or > > > >> GPEX. What does it fail to check? Why shall we care about PCI vs PCIe? > > > > I think the concern is getting any other TYPE_PCI_HOST_BRIDGE types > > > attached > > > > to SMMUv3 other than pxb-pcie or GPEX. For example you could do, > > > > > > > > -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ > > > > -device arm-smmuv3,primary-bus=cxl.1,id=smmuv3.1 \ > > > > > > > > as pxb-cxl is of type TYPE_PCI_HOST_BRIDGE. I don't know if there are any > > > other > > > > ones similar to this out there. > > > > > > > > So the aim is to make the checking more specific to PXB. > > > > > > thank you for the clarification. Is it invalid to have the SMMU > > > protecting RIDs comming from the pxb-cxl hierarchy? > > > > That’s a good question. I don't know that for sure. > > It should be fine to support CXL for this but we can work that out later. > > For now limited use cases as there is no CXL VFIO support and the only thing > emulated devices do that the SMMU might influence is MSIX. > > The one that concerned me is pxb-pci if we only care about pcie. > I'm not sure if we need to make that distinction or not. > Hmm. Shameer pointed out I hallucinated the existence of a PCI only expander bridge. So ignore that. > Jonathan > > > Anyway currently the full support for CXL on virt is in progress here, > > https://lore.kernel.org/qemu-devel/20250612134338.1871023-1-Jonathan.Cameron@huawei.com/ > > > > Jonathan? > > > > Thanks, > > Shameer > > > > > > > >
On Thu, 19 Jun 2025 10:38:02 +0100 Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > On Thu, 19 Jun 2025 10:30:28 +0100 > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > > On Thu, 19 Jun 2025 09:05:07 +0100 > > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > -----Original Message----- > > > > From: Eric Auger <eric.auger@redhat.com> > > > > Sent: Thursday, June 19, 2025 8:41 AM > > > > To: Shameerali Kolothum Thodi > > > > <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > > > > <jonathan.cameron@huawei.com> > > > > Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > > > devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > > > nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > > > imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > > > smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > > > jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > > > Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe > > > > Root Complex association > > > > > > > > Hi Shameer, > > > > > > > > On 6/19/25 9:24 AM, Shameerali Kolothum Thodi wrote: > > > > > Hi Eric, > > > > > > > > > >> -----Original Message----- > > > > >> From: Eric Auger <eric.auger@redhat.com> > > > > >> Sent: Wednesday, June 18, 2025 6:00 PM > > > > >> To: Shameerali Kolothum Thodi > > > > >> <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron > > > > >> <jonathan.cameron@huawei.com> > > > > >> Cc: Linuxarm <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > > > >> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > > > >> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > > > >> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > > > >> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > > > >> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > > > >> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > > > > PCIe > > > > >> Root Complex association > > > > >> > > > > >> Hi Shameer, Jonathan, > > > > >> > > > > >> On 6/18/25 10:35 AM, Shameerali Kolothum Thodi wrote: > > > > >>>> -----Original Message----- > > > > >>>> From: Jonathan Cameron <jonathan.cameron@huawei.com> > > > > >>>> Sent: Tuesday, June 17, 2025 5:53 PM > > > > >>>> To: Eric Auger <eric.auger@redhat.com> > > > > >>>> Cc: Shameerali Kolothum Thodi > > > > >>>> <shameerali.kolothum.thodi@huawei.com>; Linuxarm > > > > >>>> <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu- > > > > >>>> devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com; > > > > >>>> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com; > > > > >>>> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com; > > > > >>>> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>; > > > > >>>> jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org > > > > >>>> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has > > > > >> PCIe > > > > >>>> Root Complex association > > > > >>>> > > > > >>>> On Tue, 17 Jun 2025 09:49:54 +0200 > > > > >>>> Eric Auger <eric.auger@redhat.com> wrote: > > > > >>>> > > > > >>>>> On 6/16/25 12:20 PM, Jonathan Cameron wrote: > > > > >>>>>> On Fri, 13 Jun 2025 15:44:43 +0100 > > > > >>>>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > > > wrote: > > > > >>>>>> > > > > >>>>>>> Although this change does not affect functionality at present, it is > > > > >>>>>> Patch title says PCIe. This check is vs PCI host bridge. > > > > >>>>>> > > > > >>>>>> No idea which one you wanted, but if it is PCIe needs to be > > > > >>>>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one > > > > >>>>>> I think. > > > > >>>>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against > > > > >> pxb > > > > >>>>> pci-bridge/pci_expander_bridge.c: .parent = > > > > >>>> TYPE_PCI_HOST_BRIDGE, > > > > >> sorry but I still fail to understand why we can't just check against > > > > >> > > > > >> TYPE_PCI_HOST_BRIDGE for making sure the SMMU is attached to PXB or > > > > >> GPEX. What does it fail to check? Why shall we care about PCI vs PCIe? > > > > > I think the concern is getting any other TYPE_PCI_HOST_BRIDGE types > > > > attached > > > > > to SMMUv3 other than pxb-pcie or GPEX. For example you could do, > > > > > > > > > > -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ > > > > > -device arm-smmuv3,primary-bus=cxl.1,id=smmuv3.1 \ > > > > > > > > > > as pxb-cxl is of type TYPE_PCI_HOST_BRIDGE. I don't know if there are any > > > > other > > > > > ones similar to this out there. > > > > > > > > > > So the aim is to make the checking more specific to PXB. > > > > > > > > thank you for the clarification. Is it invalid to have the SMMU > > > > protecting RIDs comming from the pxb-cxl hierarchy? > > > > > > That’s a good question. I don't know that for sure. > > > > It should be fine to support CXL for this but we can work that out later. > > > > For now limited use cases as there is no CXL VFIO support and the only thing > > emulated devices do that the SMMU might influence is MSIX. > > > > The one that concerned me is pxb-pci if we only care about pcie. > > I'm not sure if we need to make that distinction or not. > > > Hmm. Shameer pointed out I hallucinated the existence of a PCI only expander bridge. > So ignore that. After some more discussions... Ah but it does - just called "pxb" - but it's irrelevant anyway as you can only add them to a PCI root bridge (Shameer tested with a PCIe to PCI bridge with a pxb below it and it rejects that config). So can't plug them into virt anyway. That's just for the record if I forget the whole discussion in future ;) Jonathan > > > Jonathan > > > > > Anyway currently the full support for CXL on virt is in progress here, > > > https://lore.kernel.org/qemu-devel/20250612134338.1871023-1-Jonathan.Cameron@huawei.com/ > > > > > > Jonathan? > > > > > > Thanks, > > > Shameer > > > > > > > > > > > > >
On Wed, 18 Jun 2025 09:35:35 +0100
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> > -----Original Message-----
> > From: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Sent: Tuesday, June 17, 2025 5:53 PM
> > To: Eric Auger <eric.auger@redhat.com>
> > Cc: Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@huawei.com>; Linuxarm
> > <linuxarm@huawei.com>; qemu-arm@nongnu.org; qemu-
> > devel@nongnu.org; peter.maydell@linaro.org; jgg@nvidia.com;
> > nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com;
> > imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
> > smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>;
> > jiangkunkun <jiangkunkun@huawei.com>; zhangfei.gao@linaro.org
> > Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe
> > Root Complex association
> >
> > On Tue, 17 Jun 2025 09:49:54 +0200
> > Eric Auger <eric.auger@redhat.com> wrote:
> >
> > > On 6/16/25 12:20 PM, Jonathan Cameron wrote:
> > > > On Fri, 13 Jun 2025 15:44:43 +0100
> > > > Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> > > >
> > > >> Although this change does not affect functionality at present, it is
> > > > Patch title says PCIe. This check is vs PCI host bridge.
> > > >
> > > > No idea which one you wanted, but if it is PCIe needs to be
> > > > TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
> > > > I think.
> > > I think we need TYPE_PCI_HOST_BRIDGE as we want to check against pxb
> > >
> > > pci-bridge/pci_expander_bridge.c: .parent =
> > TYPE_PCI_HOST_BRIDGE,
> >
> > Hmm. That's awkward and I'd forgotten that wrinkle.
> > Need a stronger test but which one? The PXB root bus has a parent of
> > TYPE_PCIE_BUS. Maybe we can check that?
>
> Ok. How about we do something like below?
>
>
> @@ -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,10 +939,31 @@ 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), "pxb-pcie-bus")) {
> + error_setg(errp, "SMMU is not attached to pxb-pcie bus!");
> + return;
> + }
> + }
> + pci_setup_iommu(pci_bus, &smmu_ops, s);
> + } else {
> + error_setg(errp, "SMMU should be attached to a default PCIe
> root complex"
> + "(pcie.0) or a pxb-pcie based root complex");
> }
> }
>
> Please let me know if this is good enough or not.
LGTM
>
> Thanks,
> Shameer
On 6/17/25 12:52 PM, Jonathan Cameron wrote:
> On Tue, 17 Jun 2025 09:49:54 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> On 6/16/25 12:20 PM, Jonathan Cameron wrote:
>>> On Fri, 13 Jun 2025 15:44:43 +0100
>>> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
>>>
>>>> Although this change does not affect functionality at present, it is
>>> Patch title says PCIe. This check is vs PCI host bridge.
>>>
>>> No idea which one you wanted, but if it is PCIe needs to be
>>> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
>>> I think.
>> I think we need TYPE_PCI_HOST_BRIDGE as we want to check against pxb
>>
>> pci-bridge/pci_expander_bridge.c: .parent = TYPE_PCI_HOST_BRIDGE,
>
> Hmm. That's awkward and I'd forgotten that wrinkle.
> Need a stronger test but which one? The PXB root bus has a parent of
> TYPE_PCIE_BUS. Maybe we can check that?
>
Yeah, rooted in the problem that we should have pci-switches or pci-rp's,
not just 'pxb'.
>
>>
>> Besides the commit title/desc suggested by Jonathan you may also refine
>> the error msg which can stem now from 2 different causes
>>
>> Thanks
>>
>> Eric
>>>
>>>> required when we add support for user-creatable SMMUv3 devices in
>>>> future patches.
>>>>
>>>> Tested-by: Nathan Chen <nathanc@nvidia.com>
>>>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
>>>> ---
>>>> hw/arm/smmu-common.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>>>> index f39b99e526..7890aa12c1 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"
>>>> @@ -937,7 +938,8 @@ 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) {
>>>> + if (s->primary_bus && object_dynamic_cast(OBJECT(s->primary_bus)->parent,
>>>> + TYPE_PCI_HOST_BRIDGE)) {
>>>> pci_setup_iommu(s->primary_bus, &smmu_ops, s);
>>>> } else {
>>>> error_setg(errp, "SMMU is not attached to any PCI bus!");
>>
>>
>
> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: Monday, June 16, 2025 11:20 AM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Linuxarm
> <linuxarm@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com;
> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe
> Root Complex association
>
> On Fri, 13 Jun 2025 15:44:43 +0100
> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
>
> > Although this change does not affect functionality at present, it is
>
> Patch title says PCIe. This check is vs PCI host bridge.
>
> No idea which one you wanted, but if it is PCIe needs to be
> TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
> I think.
Looking at the code in virt.c and virt-acpi-build.c , this should be
TYPE_PCI_HOST_BRIDGE. I will change the documentation to
refer to PCI host bridge.
Thanks,
Shameer
>
> > required when we add support for user-creatable SMMUv3 devices in
> > future patches.
> >
> > Tested-by: Nathan Chen <nathanc@nvidia.com>
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> > hw/arm/smmu-common.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> > index f39b99e526..7890aa12c1 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"
> > @@ -937,7 +938,8 @@ 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) {
> > + if (s->primary_bus && object_dynamic_cast(OBJECT(s->primary_bus)-
> >parent,
> > + TYPE_PCI_HOST_BRIDGE)) {
> > pci_setup_iommu(s->primary_bus, &smmu_ops, s);
> > } else {
> > error_setg(errp, "SMMU is not attached to any PCI bus!");
On Fri, Jun 13, 2025 at 03:44:43PM +0100, Shameer Kolothum wrote: > Although this change does not affect functionality at present, it is > required when we add support for user-creatable SMMUv3 devices in > future patches. > > Tested-by: Nathan Chen <nathanc@nvidia.com> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
© 2016 - 2025 Red Hat, Inc.