Seeing conflict with IPMMU driver under ACPI

Seeing conflict with IPMMU driver under ACPI

[Jasty, Ananth]

Hi, with your IPMMU driver enabled under 4.13 we’re seeing a crash on boot:

[ 13.785164] Unable to handle kernel NULL pointer dereference at virtual address 00000018
[ 13.793254] [0000000000000018] user address but active_mm is swapper
[ 13.799600] Internal error: Oops: 96000004 [#1] SMP
[ 13.804466] Modules linked in: aes_neon_bs aes_neon_blk crypto_simd cryptd
[ 13.811334] CPU: 152 PID: 1529 Comm: kworker/152:1 Not tainted 4.13.0-9-generic #10-Ubuntu
[ 13.819584] Hardware name: Default string Cavium ThunderX2/Default string, BIOS 5.13 07/20/2017
[ 13.828285] Workqueue: events deferred_probe_work_func
[ 13.833410] task: ffff80bee93d0000 task.stack: ffff80bee93dc000
[ 13.839330] PC is at iommu_ops_from_fwnode+0x4c/0x90
[ 13.844282] LR is at iommu_ops_from_fwnode+0x28/0x90

The ARM SMMUv3 driver (which our platform implements) seems to be losing iommu_ops to the IPMMU driver.

Note: our platform uses ACPI for device enumeration.

I have no way to test this, but is there a reason the set_iommu isn’t in _probe?

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 2a38aa1..d4c72da 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1066,6 +1066,9 @@ static int ipmmu_probe(struct platform_device *pdev)

        platform_set_drvdata(pdev, mmu);

+       if (!iommu_present(&platform_bus_type))
+               bus_set_iommu(&platform_bus_type, &ipmmu_ops);
+
        return 0;
 }

@@ -1108,9 +1111,6 @@ static int __init ipmmu_init(void)
        if (ret < 0)
                return ret;

-       if (!iommu_present(&platform_bus_type))
-               bus_set_iommu(&platform_bus_type, &ipmmu_ops);
-
        return 0;
 }


If I’m wrong, my apologies.

Thank you,

Ananth Jasty
Lead Software Engineer
Cavium

Re: Seeing conflict with IPMMU driver under ACPI

[Laurent Pinchart]

Hi Ananth,

On Tuesday, 19 September 2017 03:43:05 EEST Jasty, Ananth wrote:
> Hi, with your IPMMU driver enabled under 4.13 we’re seeing a crash on boot:
> 
> [ 13.785164] Unable to handle kernel NULL pointer dereference at virtual
> address 00000018
> [ 13.793254] [0000000000000018] user address but active_mm is swapper
> [ 13.799600] Internal error: Oops: 96000004 [#1] SMP
> [ 13.804466] Modules linked in: aes_neon_bs aes_neon_blk crypto_simd cryptd
> [ 13.811334] CPU: 152 PID: 1529 Comm: kworker/152:1 Not tainted
> 4.13.0-9-generic #10-Ubuntu
> [ 13.819584] Hardware name: Default string Cavium ThunderX2/Default string,
> BIOS 5.13 07/20/2017
> [ 13.828285] Workqueue: events deferred_probe_work_func
> [ 13.833410] task: ffff80bee93d0000 task.stack: ffff80bee93dc000
> [ 13.839330] PC is at iommu_ops_from_fwnode+0x4c/0x90
> [ 13.844282] LR is at iommu_ops_from_fwnode+0x28/0x90
> 
> The ARM SMMUv3 driver (which our platform implements) seems to be losing
> iommu_ops to the IPMMU driver.

You seem not to be the first one to notice:

https://patchwork.kernel.org/patch/9956449/

> Note: our platform uses ACPI for device enumeration.
> 
> I have no way to test this, but is there a reason the set_iommu isn’t in
> _probe?

I can't recall what the reason was I'm afraid.

> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 2a38aa1..d4c72da 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1066,6 +1066,9 @@ static int ipmmu_probe(struct platform_device *pdev)
> 
>         platform_set_drvdata(pdev, mmu);
> 
> +       if (!iommu_present(&platform_bus_type))
> +               bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> +
>         return 0;
>  }
> 
> @@ -1108,9 +1111,6 @@ static int __init ipmmu_init(void)
>         if (ret < 0)
>                 return ret;
> 
> -       if (!iommu_present(&platform_bus_type))
> -               bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> -
>         return 0;
>  }
> 
> 
> If I’m wrong, my apologies.

-- 
Regards,

Laurent Pinchart

Re: Seeing conflict with IPMMU driver under ACPI

[Jasty, Ananth]

[-- Attachment #1.1: Type: text/plain, Size: 2394 bytes --]

On Sep 18, 2017, at 11:49 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com<mailto:laurent.pinchart@ideasonboard.com>> wrote:

Hi Ananth,

On Tuesday, 19 September 2017 03:43:05 EEST Jasty, Ananth wrote:
Hi, with your IPMMU driver enabled under 4.13 we’re seeing a crash on boot:

[ 13.785164] Unable to handle kernel NULL pointer dereference at virtual
address 00000018
[ 13.793254] [0000000000000018] user address but active_mm is swapper
[ 13.799600] Internal error: Oops: 96000004 [#1] SMP
[ 13.804466] Modules linked in: aes_neon_bs aes_neon_blk crypto_simd cryptd
[ 13.811334] CPU: 152 PID: 1529 Comm: kworker/152:1 Not tainted
4.13.0-9-generic #10-Ubuntu
[ 13.819584] Hardware name: Default string Cavium ThunderX2/Default string,
BIOS 5.13 07/20/2017
[ 13.828285] Workqueue: events deferred_probe_work_func
[ 13.833410] task: ffff80bee93d0000 task.stack: ffff80bee93dc000
[ 13.839330] PC is at iommu_ops_from_fwnode+0x4c/0x90
[ 13.844282] LR is at iommu_ops_from_fwnode+0x28/0x90

The ARM SMMUv3 driver (which our platform implements) seems to be losing
iommu_ops to the IPMMU driver.

You seem not to be the first one to notice:

https://patchwork.kernel.org/patch/9956449/

Wow, he did not beat me by much.

Note: our platform uses ACPI for device enumeration.

I have no way to test this, but is there a reason the set_iommu isn’t in
_probe?

I can't recall what the reason was I'm afraid.

arm-smmu-v3.c does do the set_iommu in probe, and thus far there have been no confirmed fatalities, but I can't speak authoritatively.

I'll leave you all to it then.

Thanks,

Ananth


diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 2a38aa1..d4c72da 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1066,6 +1066,9 @@ static int ipmmu_probe(struct platform_device *pdev)

       platform_set_drvdata(pdev, mmu);

+       if (!iommu_present(&platform_bus_type))
+               bus_set_iommu(&platform_bus_type, &ipmmu_ops);
+
       return 0;
}

@@ -1108,9 +1111,6 @@ static int __init ipmmu_init(void)
       if (ret < 0)
               return ret;

-       if (!iommu_present(&platform_bus_type))
-               bus_set_iommu(&platform_bus_type, &ipmmu_ops);
-
       return 0;
}


If I’m wrong, my apologies.

--
Regards,

Laurent Pinchart



[-- Attachment #1.2: Type: text/html, Size: 4766 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Seeing conflict with IPMMU driver under ACPI

[Laurent Pinchart]

Hi Ananth,

On Tuesday, 19 September 2017 09:54:49 EEST Jasty, Ananth wrote:
> On Sep 18, 2017, at 11:49 PM, Laurent Pinchart wrote:
> > On Tuesday, 19 September 2017 03:43:05 EEST Jasty, Ananth wrote:
> >> Hi, with your IPMMU driver enabled under 4.13 we’re seeing a crash on
> >> boot:
> >> 
> >> [ 13.785164] Unable to handle kernel NULL pointer dereference at virtual
> >> address 00000018
> >> [ 13.793254] [0000000000000018] user address but active_mm is swapper
> >> [ 13.799600] Internal error: Oops: 96000004 [#1] SMP
> >> [ 13.804466] Modules linked in: aes_neon_bs aes_neon_blk crypto_simd
> >> cryptd
> >> [ 13.811334] CPU: 152 PID: 1529 Comm: kworker/152:1 Not tainted
> >> 4.13.0-9-generic #10-Ubuntu
> >> [ 13.819584] Hardware name: Default string Cavium ThunderX2/Default
> >> string, BIOS 5.13 07/20/2017
> >> [ 13.828285] Workqueue: events deferred_probe_work_func
> >> [ 13.833410] task: ffff80bee93d0000 task.stack: ffff80bee93dc000
> >> [ 13.839330] PC is at iommu_ops_from_fwnode+0x4c/0x90
> >> [ 13.844282] LR is at iommu_ops_from_fwnode+0x28/0x90
> >> 
> >> The ARM SMMUv3 driver (which our platform implements) seems to be losing
> >> iommu_ops to the IPMMU driver.
> > 
> > You seem not to be the first one to notice:
> > 
> > https://patchwork.kernel.org/patch/9956449/\0
> 
> Wow, he did not beat me by much.
> 
> >> Note: our platform uses ACPI for device enumeration.
> >> 
> >> I have no way to test this, but is there a reason the set_iommu isn’t in
> >> _probe?
> > 
> > I can't recall what the reason was I'm afraid.
> 
> arm-smmu-v3.c does do the set_iommu in probe, and thus far there have been
> no confirmed fatalities, but I can't speak authoritatively.
> 
> I'll leave you all to it then.

After digging a bit I realized that calling bus_set_iommu() from the init 
function is needed to support ARM32 (the same driver supports both ARM32 and 
ARM64). Let's discuss the matter in a reply to https://patchwork.kernel.org/
patch/9956449/\0.

-- 
Regards,

Laurent Pinchart