Re: [Bisected] [commit 2ad56efa80dba89162106c06ebc00b611325e584] [linux-next] WARNING while booting to kernel 6.6.0-rc3-next-20230929

Re: [Bisected] [commit 2ad56efa80dba89162106c06ebc00b611325e584] [linux-next] WARNING while booting to kernel 6.6.0-rc3-next-20230929

[Jason Gunthorpe]

On Wed, Oct 04, 2023 at 04:37:10PM +0530, Tasmiya Nalatwad wrote:
>    Greetings,
> 
>    [linux-next] [6.6.0-rc3-next-20230929] [git bisect ->
>    2ad56efa80dba89162106c06ebc00b611325e584]WARNING: CPU: 0 PID: 8 at
>    arch/powerpc/kernel/[1]iommu.c:407 __iommu_free+0x1e4/0x1f0
 
>    gitbisect is pointing to the below commit
>    commit 2ad56efa80dba89162106c06ebc00b611325e584
>        powerpc/iommu: Setup a default domain and remove set_platform_dma_ops
 
I assume this means there are still sequencing problems with power at
boot time. eg we turned on the dma ops in the wrong order or something
like that

As far as I can see the only difference here is that we do the
operation to claim dma ops during the iommu drive probe. We can avoid that.

Does this work for you?

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index d6ad3fde85a212..115b9031badac7 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1280,13 +1280,19 @@ struct iommu_table_group_ops spapr_tce_table_group_ops = {
 /*
  * A simple iommu_ops to allow less cruft in generic VFIO code.
  */
-static int spapr_tce_platform_iommu_attach_dev(struct iommu_domain *dom,
-					       struct device *dev)
+static int
+spapr_tce_platform_iommu_attach_dev(struct iommu_domain *platform_domain,
+				    struct device *dev)
 {
+	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
 	struct iommu_group *grp = iommu_group_get(dev);
 	struct iommu_table_group *table_group;
 	int ret = -EINVAL;
 
+	/* At first attach the ownership is already set */
+	if (!domain)
+		return 0;
+
 	if (!grp)
 		return -ENODEV;
 

Re: [Bisected] [commit 2ad56efa80dba89162106c06ebc00b611325e584] [linux-next] WARNING while booting to kernel 6.6.0-rc3-next-20230929

[Tasmiya Nalatwad]

Thanks Jason. Yes the suggested changes works and Warnings are not seen.

On 10/4/23 17:08, Jason Gunthorpe wrote:
> On Wed, Oct 04, 2023 at 04:37:10PM +0530, Tasmiya Nalatwad wrote:
>>     Greetings,
>>
>>     [linux-next] [6.6.0-rc3-next-20230929] [git bisect ->
>>     2ad56efa80dba89162106c06ebc00b611325e584]WARNING: CPU: 0 PID: 8 at
>>     arch/powerpc/kernel/[1]iommu.c:407 __iommu_free+0x1e4/0x1f0
>>     gitbisect is pointing to the below commit
>>     commit 2ad56efa80dba89162106c06ebc00b611325e584
>>         powerpc/iommu: Setup a default domain and remove set_platform_dma_ops
> I assume this means there are still sequencing problems with power at
> boot time. eg we turned on the dma ops in the wrong order or something
> like that
>
> As far as I can see the only difference here is that we do the
> operation to claim dma ops during the iommu drive probe. We can avoid that.
>
> Does this work for you?
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index d6ad3fde85a212..115b9031badac7 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1280,13 +1280,19 @@ struct iommu_table_group_ops spapr_tce_table_group_ops = {
>   /*
>    * A simple iommu_ops to allow less cruft in generic VFIO code.
>    */
> -static int spapr_tce_platform_iommu_attach_dev(struct iommu_domain *dom,
> -					       struct device *dev)
> +static int
> +spapr_tce_platform_iommu_attach_dev(struct iommu_domain *platform_domain,
> +				    struct device *dev)
>   {
> +	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
>   	struct iommu_group *grp = iommu_group_get(dev);
>   	struct iommu_table_group *table_group;
>   	int ret = -EINVAL;
>
> +	/* At first attach the ownership is already set */
> +	if (!domain)
> +		return 0;
> +
>   	if (!grp)
>   		return -ENODEV;
>
>
-- 
Regards,
Tasmiya Nalatwad
IBM Linux Technology Center