On Wed, Sep 7, 2022 at 1:27 AM Robin Murphy <robin.murphy@xxxxxxx> wrote: > > On 2022-09-07 04:17, Gupta, Nipun wrote: > > [AMD Official Use Only - General] > > > > > > > >> -----Original Message----- > >> From: Saravana Kannan <saravanak@xxxxxxxxxx> > >> Sent: Wednesday, September 7, 2022 5:41 AM > >> To: Gupta, Nipun <Nipun.Gupta@xxxxxxx> > >> Cc: robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; > >> gregkh@xxxxxxxxxxxxxxxxxxx; rafael@xxxxxxxxxx; eric.auger@xxxxxxxxxx; > >> alex.williamson@xxxxxxxxxx; cohuck@xxxxxxxxxx; Gupta, Puneet (DCG-ENG) > >> <puneet.gupta@xxxxxxx>; song.bao.hua@xxxxxxxxxxxxx; > >> mchehab+huawei@xxxxxxxxxx; maz@xxxxxxxxxx; f.fainelli@xxxxxxxxx; > >> jeffrey.l.hugo@xxxxxxxxx; Michael.Srba@xxxxxxxxx; mani@xxxxxxxxxx; > >> yishaih@xxxxxxxxxx; jgg@xxxxxxxx; jgg@xxxxxxxxxx; robin.murphy@xxxxxxx; > >> will@xxxxxxxxxx; joro@xxxxxxxxxx; masahiroy@xxxxxxxxxx; > >> ndesaulniers@xxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > >> kbuild@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > >> devicetree@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; okaya@xxxxxxxxxx; Anand, > >> Harpreet <harpreet.anand@xxxxxxx>; Agarwal, Nikhil > >> <nikhil.agarwal@xxxxxxx>; Simek, Michal <michal.simek@xxxxxxx>; > >> Radovanovic, Aleksandar <aleksandar.radovanovic@xxxxxxx>; git (AMD-Xilinx) > >> <git@xxxxxxx> > >> 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 <nipun.gupta@xxxxxxx> wrote: > >>> > >>> 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@xxxxxxx> > >>> --- > >>> 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 > >>> 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) > >>> 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? > Note that I have a series pending[1] that will make this patch a whole > lot simpler. Thanks for the pointer. I'll make some comments in that series about bus notifiers. -Saravana > > Thanks, > Robin. > > [1] > https://lore.kernel.org/linux-iommu/cover.1660572783.git.robin.murphy@xxxxxxx/T/#t > > > > > Thanks, > > Nipun > > > >> > >> -Saravana > >> > >>> #endif > >>> if (platform_bus_type.iommu_ops != ops) { > >>> err = bus_set_iommu(&platform_bus_type, ops); > >>> if (err) > >>> - goto err_reset_amba_ops; > >>> + goto err_reset_cdx_ops; > >>> } > >>> > >>> return 0; > >>> > >>> -err_reset_amba_ops: > >>> +err_reset_cdx_ops: > >>> +#ifdef CONFIG_CDX_BUS > >>> + bus_set_iommu(&cdx_bus_type, NULL); > >>> +#endif > >>> +err_reset_amba_ops: __maybe_unused; > >>> #ifdef CONFIG_ARM_AMBA > >>> bus_set_iommu(&amba_bustype, NULL); > >>> #endif > >>> -- > >>> 2.25.1 > >>>