On Wed, Sep 7, 2022 at 1:40 PM Robin Murphy [off-list ref] wrote:

On 2022-09-07 19:24, Saravana Kannan wrote:

quoted

On Wed, Sep 7, 2022 at 1:27 AM Robin Murphy [off-list ref] wrote:

quoted

On 2022-09-07 04:17, Gupta, Nipun wrote:

quoted

[AMD Official Use Only - General]

quoted

-----Original Message-----
From: Saravana Kannan <redacted>
Sent: Wednesday, September 7, 2022 5:41 AM
To: Gupta, Nipun <Nipun.Gupta@amd.com>
Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
gregkh@linuxfoundation.org; rafael@kernel.org; eric.auger@redhat.com;
alex.williamson@redhat.com; cohuck@redhat.com; Gupta, Puneet (DCG-ENG)
[off-list ref]; song.bao.hua@hisilicon.com;
mchehab+huawei@kernel.org; maz@kernel.org; f.fainelli@gmail.com;
jeffrey.l.hugo@gmail.com; Michael.Srba@seznam.cz; mani@kernel.org;
yishaih@nvidia.com; jgg@ziepe.ca; jgg@nvidia.com; robin.murphy@arm.com;
will@kernel.org; joro@8bytes.org; masahiroy@kernel.org;
ndesaulniers@google.com; linux-arm-kernel@lists.infradead.org; linux-
kbuild@vger.kernel.org; linux-kernel@vger.kernel.org;
devicetree@vger.kernel.org; kvm@vger.kernel.org; okaya@kernel.org; Anand,
Harpreet [off-list ref]; Agarwal, Nikhil
[off-list ref]; Simek, Michal [off-list ref];
Radovanovic, Aleksandar [off-list ref]; git (AMD-Xilinx)
[off-list ref]
Subject: Re: [RFC PATCH v3 3/7] iommu/arm-smmu-v3: support ops registration
for CDX bus

[CAUTION: External Email]

On Tue, Sep 6, 2022 at 6:48 AM Nipun Gupta [off-list ref] wrote:

quoted

With new CDX bus supported for AMD FPGA devices on ARM
platform, the bus requires registration for the SMMU v3
driver.

Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
---
   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++++++++++++++--
   1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

quoted

index d32b02336411..8ec9f2baf12d 100644

--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -29,6 +29,7 @@
   #include <linux/platform_device.h>

   #include <linux/amba/bus.h>
+#include <linux/cdx/cdx_bus.h>

   #include "arm-smmu-v3.h"
   #include "../../iommu-sva-lib.h"

@@ -3690,16 +3691,27 @@ static int arm_smmu_set_bus_ops(struct

iommu_ops *ops)

quoted

                  if (err)
                          goto err_reset_pci_ops;
          }
+#endif
+#ifdef CONFIG_CDX_BUS
+       if (cdx_bus_type.iommu_ops != ops) {
+               err = bus_set_iommu(&cdx_bus_type, ops);
+               if (err)
+                       goto err_reset_amba_ops;
+       }

I'm not an expert on IOMMUs, so apologies if the question is stupid.

Why does the CDX bus need special treatment here (like PCI) when there
are so many other busses (eg: I2C, SPI, etc) that don't need any
changes here?

AFAIU, the devices on I2C/SPI does not use SMMU. Apart from PCI/AMBA,
FSL-MC is another similar bus (on SMMUv2) which uses SMMU ops.

The devices here are behind SMMU. Robin can kindly correct or add
more here from SMMU perspective.

Indeed, there is no need to describe and handle how DMA may or may not
be translated for I2C/SPI/USB/etc. because they are not DMA-capable
buses (in those cases the relevant bus *controller* often does DMA, but
it does that for itself as the platform/PCI/etc. device it is).

Ok this is what I was guessing was the reason, but didn't want to make
that assumption.

So if there are other cases like AMBA, FSL-MC where the devices can do
direct DMA, why do those buses not need a #ifdef section in this
function like CDX? Or put another way, why does CDX need special treatment?

Er, it doesn't? The only non-optional bus here is platform, since the
others *can* be configured out and *are* #ifdefed accordingly.

Ah ok. Also I somehow missed the #ifdef AMBA there and thought there
was only #ifdef PCI and the rest of the buses somehow got it working
without having to muck around arm-smmu-v3.c.

Thanks for the explanation. I'm done here :)

-Saravana

This
patch is fine for the kernel it was based on, it'll just want rewriting
now that I've cleaned all this horrible driver boilerplate up. And
according to the thread on patch #4 there might need to be additional
changes for CDX to express a reserved MSI region for SMMU support to
actually work properly.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel