Revert "arm64: Make default dma_ops to be noncoherent"
diff mbox

Message ID 1417146214-24621-1-git-send-email-ming.lei@canonical.com
State New
Headers show

Commit Message

Ming Lei Nov. 28, 2014, 3:43 a.m. UTC
This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.

Conflicts:
	arch/arm64/mm/dma-mapping.c

On ARM64, Utopic kernel ships APM's PCI host controller driver
which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
platform. On the other side, ARM64 maintainer[1] said the
commit c7a4a7658 can be reverted safely if booting with ACPI
which can be supported by APM's own kernel release too.

So please revert the commit c7a4a7658 to avoid breaking
PCI function on APM ARM64.

BugLink: https://bugs.launchpad.net/bugs/1386490

[1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2

Cc: Dann Frazier <dann.frazier@canonical.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 arch/arm64/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

dann frazier Dec. 1, 2014, 8 p.m. UTC | #1
On Thu, Nov 27, 2014 at 8:43 PM, Ming Lei <ming.lei@canonical.com> wrote:
> This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.
>
> Conflicts:
>         arch/arm64/mm/dma-mapping.c
>
> On ARM64, Utopic kernel ships APM's PCI host controller driver
> which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
> platform. On the other side, ARM64 maintainer[1] said the
> commit c7a4a7658 can be reverted safely if booting with ACPI
> which can be supported by APM's own kernel release too.
>
> So please revert the commit c7a4a7658 to avoid breaking
> PCI function on APM ARM64.

Thanks Ming.

One question I have (that I also mentioned in the bug), is what will
prevent us from having to carry this forward indefinitely?
The m400 cartridge does not provide ACPI tables (afaik), so we need a
device-tree based boot to continue to work. Reverting this for utopic
might be the right stop-gap, but what's plan for vivid and beyond?

 -dann

> BugLink: https://bugs.launchpad.net/bugs/1386490
>
> [1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2
>
> Cc: Dann Frazier <dann.frazier@canonical.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  arch/arm64/mm/dma-mapping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 4164c5a..acc807a 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -342,7 +342,7 @@ static int __init swiotlb_late_init(void)
>         bus_register_notifier(&platform_bus_type, &platform_bus_nb);
>         bus_register_notifier(&amba_bustype, &amba_bus_nb);
>
> -       dma_ops = &noncoherent_swiotlb_dma_ops;
> +       dma_ops = &coherent_swiotlb_dma_ops;
>
>         return swiotlb_late_init_with_default_size(swiotlb_size);
>  }
> --
> 1.9.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Ming Lei Dec. 2, 2014, 1:17 a.m. UTC | #2
On Tue, Dec 2, 2014 at 4:00 AM, Dann Frazier <dann.frazier@canonical.com> wrote:
> On Thu, Nov 27, 2014 at 8:43 PM, Ming Lei <ming.lei@canonical.com> wrote:
>> This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.
>>
>> Conflicts:
>>         arch/arm64/mm/dma-mapping.c
>>
>> On ARM64, Utopic kernel ships APM's PCI host controller driver
>> which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
>> platform. On the other side, ARM64 maintainer[1] said the
>> commit c7a4a7658 can be reverted safely if booting with ACPI
>> which can be supported by APM's own kernel release too.
>>
>> So please revert the commit c7a4a7658 to avoid breaking
>> PCI function on APM ARM64.
>
> Thanks Ming.

You are welcome, :-)

BTW, this patch is only for utopic.

>
> One question I have (that I also mentioned in the bug), is what will
> prevent us from having to carry this forward indefinitely?
> The m400 cartridge does not provide ACPI tables (afaik), so we need a

Actually the community thought ACPI should be used on server only,
and in case of server, the fact is that devices have cache-coherent DMA
all the time.  Since APM ARM64 is a DMA coherent SoC, it is correct to
apply the revert patch on utopic.

> device-tree based boot to continue to work. Reverting this for utopic
> might be the right stop-gap, but what's plan for vivid and beyond?

For DT based booting, my patch in the link should be correct, and
redhat shipped the similar patch in their internal tree too.  But the kernel
community thought handling the dma coherency(include irq, iommu, dma
offset, dma mask, ..) should be moved to drivers/pci of kernel first, instead
of arch/arm64.  That need cooperation between the two communities,
and I am not sure if the sort of stuff can be merged soon.

Thanks,
Ming Lei

>
>  -dann
>
>> BugLink: https://bugs.launchpad.net/bugs/1386490
>>
>> [1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2
>>
>> Cc: Dann Frazier <dann.frazier@canonical.com>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>>  arch/arm64/mm/dma-mapping.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>> index 4164c5a..acc807a 100644
>> --- a/arch/arm64/mm/dma-mapping.c
>> +++ b/arch/arm64/mm/dma-mapping.c
>> @@ -342,7 +342,7 @@ static int __init swiotlb_late_init(void)
>>         bus_register_notifier(&platform_bus_type, &platform_bus_nb);
>>         bus_register_notifier(&amba_bustype, &amba_bus_nb);
>>
>> -       dma_ops = &noncoherent_swiotlb_dma_ops;
>> +       dma_ops = &coherent_swiotlb_dma_ops;
>>
>>         return swiotlb_late_init_with_default_size(swiotlb_size);
>>  }
>> --
>> 1.9.1
>>
>>
>> --
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
dann frazier Dec. 2, 2014, 2:14 a.m. UTC | #3
On Mon, Dec 1, 2014 at 6:17 PM, Ming Lei <ming.lei@canonical.com> wrote:
> On Tue, Dec 2, 2014 at 4:00 AM, Dann Frazier <dann.frazier@canonical.com> wrote:
>> On Thu, Nov 27, 2014 at 8:43 PM, Ming Lei <ming.lei@canonical.com> wrote:
>>> This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.
>>>
>>> Conflicts:
>>>         arch/arm64/mm/dma-mapping.c
>>>
>>> On ARM64, Utopic kernel ships APM's PCI host controller driver
>>> which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
>>> platform. On the other side, ARM64 maintainer[1] said the
>>> commit c7a4a7658 can be reverted safely if booting with ACPI
>>> which can be supported by APM's own kernel release too.
>>>
>>> So please revert the commit c7a4a7658 to avoid breaking
>>> PCI function on APM ARM64.
>>
>> Thanks Ming.
>
> You are welcome, :-)
>
> BTW, this patch is only for utopic.
>
>>
>> One question I have (that I also mentioned in the bug), is what will
>> prevent us from having to carry this forward indefinitely?
>> The m400 cartridge does not provide ACPI tables (afaik), so we need a
>
> Actually the community thought ACPI should be used on server only,
> and in case of server, the fact is that devices have cache-coherent DMA
> all the time.  Since APM ARM64 is a DMA coherent SoC, it is correct to
> apply the revert patch on utopic.

I think that's a reasonable argument, given that X-Gene (TMK) is the
only hardware supported by utopic's arm64 generic kernel.

Acked-by: dann frazier <dann.frazier@canonical.com>

>> device-tree based boot to continue to work. Reverting this for utopic
>> might be the right stop-gap, but what's plan for vivid and beyond?
>
> For DT based booting, my patch in the link should be correct, and
> redhat shipped the similar patch in their internal tree too.  But the kernel
> community thought handling the dma coherency(include irq, iommu, dma
> offset, dma mask, ..) should be moved to drivers/pci of kernel first, instead
> of arch/arm64.  That need cooperation between the two communities,
> and I am not sure if the sort of stuff can be merged soon.

It seems odd to me that upstream favors a known-broken state for the
only commercially available arm64 server, but at least you've
confirmed they are aware of the issue.

 -dann

> Thanks,
> Ming Lei
>
>>
>>  -dann
>>
>>> BugLink: https://bugs.launchpad.net/bugs/1386490
>>>
>>> [1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2
>>>
>>> Cc: Dann Frazier <dann.frazier@canonical.com>
>>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>>> ---
>>>  arch/arm64/mm/dma-mapping.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>> index 4164c5a..acc807a 100644
>>> --- a/arch/arm64/mm/dma-mapping.c
>>> +++ b/arch/arm64/mm/dma-mapping.c
>>> @@ -342,7 +342,7 @@ static int __init swiotlb_late_init(void)
>>>         bus_register_notifier(&platform_bus_type, &platform_bus_nb);
>>>         bus_register_notifier(&amba_bustype, &amba_bus_nb);
>>>
>>> -       dma_ops = &noncoherent_swiotlb_dma_ops;
>>> +       dma_ops = &coherent_swiotlb_dma_ops;
>>>
>>>         return swiotlb_late_init_with_default_size(swiotlb_size);
>>>  }
>>> --
>>> 1.9.1
>>>
>>>
>>> --
>>> kernel-team mailing list
>>> kernel-team@lists.ubuntu.com
>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Ming Lei Dec. 2, 2014, 2:25 a.m. UTC | #4
On Tue, Dec 2, 2014 at 10:14 AM, Dann Frazier
<dann.frazier@canonical.com> wrote:
> On Mon, Dec 1, 2014 at 6:17 PM, Ming Lei <ming.lei@canonical.com> wrote:
>> On Tue, Dec 2, 2014 at 4:00 AM, Dann Frazier <dann.frazier@canonical.com> wrote:
>>> On Thu, Nov 27, 2014 at 8:43 PM, Ming Lei <ming.lei@canonical.com> wrote:
>>>> This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.
>>>>
>>>> Conflicts:
>>>>         arch/arm64/mm/dma-mapping.c
>>>>
>>>> On ARM64, Utopic kernel ships APM's PCI host controller driver
>>>> which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
>>>> platform. On the other side, ARM64 maintainer[1] said the
>>>> commit c7a4a7658 can be reverted safely if booting with ACPI
>>>> which can be supported by APM's own kernel release too.
>>>>
>>>> So please revert the commit c7a4a7658 to avoid breaking
>>>> PCI function on APM ARM64.
>>>
>>> Thanks Ming.
>>
>> You are welcome, :-)
>>
>> BTW, this patch is only for utopic.
>>
>>>
>>> One question I have (that I also mentioned in the bug), is what will
>>> prevent us from having to carry this forward indefinitely?
>>> The m400 cartridge does not provide ACPI tables (afaik), so we need a
>>
>> Actually the community thought ACPI should be used on server only,
>> and in case of server, the fact is that devices have cache-coherent DMA
>> all the time.  Since APM ARM64 is a DMA coherent SoC, it is correct to
>> apply the revert patch on utopic.
>
> I think that's a reasonable argument, given that X-Gene (TMK) is the
> only hardware supported by utopic's arm64 generic kernel.
>
> Acked-by: dann frazier <dann.frazier@canonical.com>

Thanks.

>
>>> device-tree based boot to continue to work. Reverting this for utopic
>>> might be the right stop-gap, but what's plan for vivid and beyond?
>>
>> For DT based booting, my patch in the link should be correct, and
>> redhat shipped the similar patch in their internal tree too.  But the kernel
>> community thought handling the dma coherency(include irq, iommu, dma
>> offset, dma mask, ..) should be moved to drivers/pci of kernel first, instead
>> of arch/arm64.  That need cooperation between the two communities,
>> and I am not sure if the sort of stuff can be merged soon.
>
> It seems odd to me that upstream favors a known-broken state for the
> only commercially available arm64 server, but at least you've
> confirmed they are aware of the issue.

The issue was discussed months ago, :-).

I guess upstream focuses on ARM64 Server with ACPI/UEFI.

Thanks,

>
>  -dann
>
>> Thanks,
>> Ming Lei
>>
>>>
>>>  -dann
>>>
>>>> BugLink: https://bugs.launchpad.net/bugs/1386490
>>>>
>>>> [1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2
>>>>
>>>> Cc: Dann Frazier <dann.frazier@canonical.com>
>>>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>>>> ---
>>>>  arch/arm64/mm/dma-mapping.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>>> index 4164c5a..acc807a 100644
>>>> --- a/arch/arm64/mm/dma-mapping.c
>>>> +++ b/arch/arm64/mm/dma-mapping.c
>>>> @@ -342,7 +342,7 @@ static int __init swiotlb_late_init(void)
>>>>         bus_register_notifier(&platform_bus_type, &platform_bus_nb);
>>>>         bus_register_notifier(&amba_bustype, &amba_bus_nb);
>>>>
>>>> -       dma_ops = &noncoherent_swiotlb_dma_ops;
>>>> +       dma_ops = &coherent_swiotlb_dma_ops;
>>>>
>>>>         return swiotlb_late_init_with_default_size(swiotlb_size);
>>>>  }
>>>> --
>>>> 1.9.1
>>>>
>>>>
>>>> --
>>>> kernel-team mailing list
>>>> kernel-team@lists.ubuntu.com
>>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Brad Figg Dec. 2, 2014, 4:09 p.m. UTC | #5
On 11/27/2014 07:43 PM, Ming Lei wrote:
> This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.
> 
> Conflicts:
> 	arch/arm64/mm/dma-mapping.c
> 
> On ARM64, Utopic kernel ships APM's PCI host controller driver
> which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
> platform. On the other side, ARM64 maintainer[1] said the
> commit c7a4a7658 can be reverted safely if booting with ACPI
> which can be supported by APM's own kernel release too.
> 
> So please revert the commit c7a4a7658 to avoid breaking
> PCI function on APM ARM64.
> 
> BugLink: https://bugs.launchpad.net/bugs/1386490
> 
> [1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2
> 
> Cc: Dann Frazier <dann.frazier@canonical.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  arch/arm64/mm/dma-mapping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 4164c5a..acc807a 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -342,7 +342,7 @@ static int __init swiotlb_late_init(void)
>  	bus_register_notifier(&platform_bus_type, &platform_bus_nb);
>  	bus_register_notifier(&amba_bustype, &amba_bus_nb);
>  
> -	dma_ops = &noncoherent_swiotlb_dma_ops;
> +	dma_ops = &coherent_swiotlb_dma_ops;
>  
>  	return swiotlb_late_init_with_default_size(swiotlb_size);
>  }
>
Paolo Pisati Dec. 2, 2014, 4:33 p.m. UTC | #6
On Fri, Nov 28, 2014 at 11:43:34AM +0800, Ming Lei wrote:
> This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.
> 
> Conflicts:
> 	arch/arm64/mm/dma-mapping.c
> 
> On ARM64, Utopic kernel ships APM's PCI host controller driver
> which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
> platform. On the other side, ARM64 maintainer[1] said the
> commit c7a4a7658 can be reverted safely if booting with ACPI
> which can be supported by APM's own kernel release too.
> 
> So please revert the commit c7a4a7658 to avoid breaking
> PCI function on APM ARM64.
> 
> BugLink: https://bugs.launchpad.net/bugs/1386490
> 
> [1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2
> 
> Cc: Dann Frazier <dann.frazier@canonical.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  arch/arm64/mm/dma-mapping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 4164c5a..acc807a 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -342,7 +342,7 @@ static int __init swiotlb_late_init(void)
>  	bus_register_notifier(&platform_bus_type, &platform_bus_nb);
>  	bus_register_notifier(&amba_bustype, &amba_bus_nb);
>  
> -	dma_ops = &noncoherent_swiotlb_dma_ops;
> +	dma_ops = &coherent_swiotlb_dma_ops;
>  
>  	return swiotlb_late_init_with_default_size(swiotlb_size);
>  }
> -- 
> 1.9.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Brad Figg Dec. 2, 2014, 4:38 p.m. UTC | #7
On 11/27/2014 07:43 PM, Ming Lei wrote:
> This reverts commit c7a4a7658d689f664050c45493d79adf053f226e.
> 
> Conflicts:
> 	arch/arm64/mm/dma-mapping.c
> 
> On ARM64, Utopic kernel ships APM's PCI host controller driver
> which isn't upstreamed yet, and APM's ARM64 Soc is a coherent
> platform. On the other side, ARM64 maintainer[1] said the
> commit c7a4a7658 can be reverted safely if booting with ACPI
> which can be supported by APM's own kernel release too.
> 
> So please revert the commit c7a4a7658 to avoid breaking
> PCI function on APM ARM64.
> 
> BugLink: https://bugs.launchpad.net/bugs/1386490
> 
> [1] http://marc.info/?l=linux-arm-kernel&m=141708838404470&w=2
> 
> Cc: Dann Frazier <dann.frazier@canonical.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  arch/arm64/mm/dma-mapping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 4164c5a..acc807a 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -342,7 +342,7 @@ static int __init swiotlb_late_init(void)
>  	bus_register_notifier(&platform_bus_type, &platform_bus_nb);
>  	bus_register_notifier(&amba_bustype, &amba_bus_nb);
>  
> -	dma_ops = &noncoherent_swiotlb_dma_ops;
> +	dma_ops = &coherent_swiotlb_dma_ops;
>  
>  	return swiotlb_late_init_with_default_size(swiotlb_size);
>  }
>

Patch
diff mbox

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 4164c5a..acc807a 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -342,7 +342,7 @@  static int __init swiotlb_late_init(void)
 	bus_register_notifier(&platform_bus_type, &platform_bus_nb);
 	bus_register_notifier(&amba_bustype, &amba_bus_nb);
 
-	dma_ops = &noncoherent_swiotlb_dma_ops;
+	dma_ops = &coherent_swiotlb_dma_ops;
 
 	return swiotlb_late_init_with_default_size(swiotlb_size);
 }