| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b949c36e-f72c-49be-a243-535c43189d1b@nvidia.com>
Date: Thu, 11 Dec 2025 12:11:15 +0530
From: Ashish Mhetre <amhetre@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>, Robin Murphy <robin.murphy@....com>
Cc: oe-kbuild-all@...ts.linux.dev, thierry.reding@...il.com,
jonathanh@...dia.com, vdumpa@...dia.com, jgg@...pe.ca,
linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-tegra@...r.kernel.org, joro@...tes.org, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, kernel test robot <lkp@...el.com>,
will@...nel.org
Subject: Re: [PATCH V4 1/3] iommu/arm-smmu-v3: Add device-tree support for
CMDQV driver
On 12/11/2025 11:39 AM, Nicolin Chen wrote:
> On Wed, Dec 10, 2025 at 12:39:49PM +0000, Robin Murphy wrote:
>> On 2025-12-10 5:19 am, Ashish Mhetre wrote:
>> [...]
>>> Hi Robin, Nic,
>>> We removed ACPI dependency in Kconfig but driver still depends
>>> on ACPI for these functions. I will be protecting ACPIspecific
>>> tegra241-cmdqv code under CONFIG_ACPI similar to what is done
>>> in arm-smmu-v3 driver. Is this the correct thing to do or do you
>>> have any other suggestions?
>> Yes, when I commented that "depends on ACPI || OF" was functionally the same
>> as just removing "depends on ACPI", I didn't mean to suggest that wasn't
>> necessarily a genuine dependency still.
>>
>> I guess if you can wrap the ACPI-specific functions in a single #ifdef block
>> that's reasonable, however I do now wonder whether things couldn't be
>> factored out a bit more - if it's a standard DSDT/SSDT namespace device,
>> shouldn't there also be a corresponding platform device created for it,
>> which we could look up instead of delving directly into the _CRS of the ACPI
>> node itself? (not sure off-hand if there's a straightforward inverse of
>> ACPI_COMPANION()...)
> Ah, I did a quick tracer at acpi_create_platform_device(). And I
> do see platform devices being created. So, we could have made it
> ACPI-independent from the beginning, as you expected :)
>
> Ashish, let's attach the following patch in your series:
Ack, thanks Robin and Nic. I'll update my patches on top of Nic's patch
and re-post.
> From 7b69a638276e66b16b923b0ce1743d3efc85a04c Mon Sep 17 00:00:00 2001
> From: Nicolin Chen <nicolinc@...dia.com>
> Date: Thu, 11 Dec 2025 05:47:05 +0000
> Subject: [PATCH] iommu/tegra241-cmdqv: Decouple driver from ACPI
>
> A platform device is created by acpi_create_platform_device() per CMDQV's
> adev. That means there is no point in going through _CRS of ACPI.
>
> Replace all the ACPI functions with standard platform functions. And drop
> all ACPI dependencies. This will make the driver compatible with DT also.
>
> Suggested-by: Robin Murphy <robin.murphy@....com>
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
> drivers/iommu/arm/Kconfig | 1 -
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
> .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 84 +++----------------
> 3 files changed, 13 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/iommu/arm/Kconfig b/drivers/iommu/arm/Kconfig
> index ef42bbe07dbef..5fac08b89deea 100644
> --- a/drivers/iommu/arm/Kconfig
> +++ b/drivers/iommu/arm/Kconfig
> @@ -121,7 +121,6 @@ config ARM_SMMU_V3_KUNIT_TEST
>
> config TEGRA241_CMDQV
> bool "NVIDIA Tegra241 CMDQ-V extension support for ARM SMMUv3"
> - depends on ACPI
> help
> Support for NVIDIA CMDQ-Virtualization extension for ARM SMMUv3. The
> CMDQ-V extension is similar to v3.3 ECMDQ for multi command queues
> 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 38028e4a52f7f..0c98be3135c63 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -5280,7 +5280,7 @@ static void acpi_smmu_dsdt_probe_tegra241_cmdqv(struct acpi_iort_node *node,
> adev = acpi_dev_get_first_match_dev("NVDA200C", uid, -1);
> if (adev) {
> /* Tegra241 CMDQV driver is responsible for put_device() */
> - smmu->impl_dev = &adev->dev;
> + smmu->impl_dev = acpi_get_first_physical_node(adev);
> smmu->options |= ARM_SMMU_OPT_TEGRA241_CMDQV;
> dev_info(smmu->dev, "found companion CMDQV device: %s\n",
> dev_name(smmu->impl_dev));
> diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> index 378104cd395e5..1fc03b72beb88 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> @@ -3,17 +3,15 @@
>
> #define dev_fmt(fmt) "tegra241_cmdqv: " fmt
>
> -#include <linux/acpi.h>
> #include <linux/debugfs.h>
> #include <linux/dma-mapping.h>
> #include <linux/interrupt.h>
> #include <linux/iommu.h>
> #include <linux/iommufd.h>
> #include <linux/iopoll.h>
> +#include <linux/platform_device.h>
> #include <uapi/linux/iommufd.h>
>
> -#include <acpi/acpixf.h>
> -
> #include "arm-smmu-v3.h"
>
> /* CMDQV register page base and size defines */
> @@ -854,69 +852,6 @@ static struct arm_smmu_impl_ops tegra241_cmdqv_impl_ops = {
>
> /* Probe Functions */
>
> -static int tegra241_cmdqv_acpi_is_memory(struct acpi_resource *res, void *data)
> -{
> - struct resource_win win;
> -
> - return !acpi_dev_resource_address_space(res, &win);
> -}
> -
> -static int tegra241_cmdqv_acpi_get_irqs(struct acpi_resource *ares, void *data)
> -{
> - struct resource r;
> - int *irq = data;
> -
> - if (*irq <= 0 && acpi_dev_resource_interrupt(ares, 0, &r))
> - *irq = r.start;
> - return 1; /* No need to add resource to the list */
> -}
> -
> -static struct resource *
> -tegra241_cmdqv_find_acpi_resource(struct device *dev, int *irq)
> -{
> - struct acpi_device *adev = to_acpi_device(dev);
> - struct list_head resource_list;
> - struct resource_entry *rentry;
> - struct resource *res = NULL;
> - int ret;
> -
> - INIT_LIST_HEAD(&resource_list);
> - ret = acpi_dev_get_resources(adev, &resource_list,
> - tegra241_cmdqv_acpi_is_memory, NULL);
> - if (ret < 0) {
> - dev_err(dev, "failed to get memory resource: %d\n", ret);
> - return NULL;
> - }
> -
> - rentry = list_first_entry_or_null(&resource_list,
> - struct resource_entry, node);
> - if (!rentry) {
> - dev_err(dev, "failed to get memory resource entry\n");
> - goto free_list;
> - }
> -
> - /* Caller must free the res */
> - res = kzalloc(sizeof(*res), GFP_KERNEL);
> - if (!res)
> - goto free_list;
> -
> - *res = *rentry->res;
> -
> - acpi_dev_free_resource_list(&resource_list);
> -
> - INIT_LIST_HEAD(&resource_list);
> -
> - if (irq)
> - ret = acpi_dev_get_resources(adev, &resource_list,
> - tegra241_cmdqv_acpi_get_irqs, irq);
> - if (ret < 0 || !irq || *irq <= 0)
> - dev_warn(dev, "no interrupt. errors will not be reported\n");
> -
> -free_list:
> - acpi_dev_free_resource_list(&resource_list);
> - return res;
> -}
> -
> static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
> {
> struct tegra241_cmdqv *cmdqv =
> @@ -1042,18 +977,23 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
>
> struct arm_smmu_device *tegra241_cmdqv_probe(struct arm_smmu_device *smmu)
> {
> + struct platform_device *pdev = to_platform_device(smmu->impl_dev);
> struct arm_smmu_device *new_smmu;
> - struct resource *res = NULL;
> + struct resource *res;
> int irq;
>
> - if (!smmu->dev->of_node)
> - res = tegra241_cmdqv_find_acpi_resource(smmu->impl_dev, &irq);
> - if (!res)
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "no memory resource found for CMDQV\n");
> goto out_fallback;
> + }
>
> - new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
> - kfree(res);
> + irq = platform_get_irq_optional(pdev, 0);
> + if (irq <= 0)
> + dev_warn(&pdev->dev,
> + "no interrupt. errors will not be reported\n");
>
> + new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
> if (new_smmu)
> return new_smmu;
>
Powered by blists - more mailing lists