diff mbox

Re: A500 boot crash in 4.2.0-rc3-00246-g763e326

Message ID 20150820054234.GA10267@google.com
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Bjorn Helgaas Aug. 20, 2015, 5:42 a.m. UTC 
On Wed, Aug 19, 2015 at 04:06:38PM +0200, Helge Deller wrote:
> > On 2015-08-19, at 1:30 AM, Yinghai Lu wrote:
> > > On Tue, Aug 18, 2015 at 9:48 PM, Meelis Roos <mroos@linux.ee> wrote:
> > >>> On Tue, Aug 18, 2015 at 12:47 PM, Helge Deller <deller@gmx.de> wrote:
> > >>> 
> > >>> Then we should change to
> > >>> 
> > >>> config PCI_BUS_ADDR_T_64BIT
> > >>>       def_bool y if (ARCH_DMA_ADDR_T_64BIT || SPARC64)
> > >>>       depends on PCI
> > >> 
> > >> Why SPARC64? The problem happened on parisc.
> > > so will not set PCI_BUS_ADDR_T_64BIT for parisc.
> 
> I think given the current time frame it's probably the best approach to fix this problem for kernel 4.2.
> It reverts the behaviour back to how it was before (for all arches beside SPARC64).
> I'm still wondering if/why parisc is the only arch (in the sym53c8xx driver only!) which broke by this change...
> 
> > I'm not sure this is optimal.  While the A500 may only have 32-bit PCI, c8000 appears to have a mix
> > of 32 and 64-bit, and rp34XX is all 64-bit.
> 
> True, but probably nobody of us noticed that we only used the 32-bit PCI interface even with 64bit kernel on parisc up to now?
> pci_bus_alloc_resource() in drivers/pci/bus.c just disabled (flag=0) all 64bit resources for us.
> But I agree, I think we need to fix drivers/parisc/lba_pci.c to correctly cope with 64bit pci addresses.

I doubt we can fix the underlying issue before v4.2, but I'd at least
like to avoid it.  I applied the patch below to for-linus.

Yinghai proposed this:

> > >>>       def_bool y if (ARCH_DMA_ADDR_T_64BIT || SPARC64)

I did this instead:

+	def_bool y if (ARCH_DMA_ADDR_T_64BIT || (64BIT && !PARISC))

because (a) Yinghai's proposal suggests this change is related to
SPARC64, which is misleading, and (b) I want to keep 64-bit bus
addresses for all 64-bit kernels *except* PA-RISC.

I didn't mark it as Tested-by Meelis because I think he tested
Yinghai's proposal.  But I can add that back, given confirmation.


commit 51a660a2732c
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Aug 20 00:08:15 2015 -0500

    PCI: Don't use 64-bit bus addresses on PA-RISC
    
    Meelis and Helge reported that 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
    caused HPMCs on A500 and hangs on rp5470.
    
    PA-RISC does not set ARCH_DMA_ADDR_T_64BIT, even for 64-bit kernels, so
    prior to 3a9ad0b4fdcd, we always used 32-bit PCI addresses.  After
    3a9ad0b4fdcd, we do use 64-bit PCI addresses in 64-bit kernels, and
    apparently there's some PA-RISC problem related to them.
    
    Fixes: 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
    Link: http://lkml.kernel.org/r/alpine.LRH.2.11.1507260929000.30065@math.ut.ee
    Reported-by: Meelis Roos <mroos@linux.ee>
    Reported-by: Helge Deller <deller@gmx.de>
    Based-on-idea-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Helge Deller Aug. 20, 2015, 6:53 a.m. UTC | #1 
On 20.08.2015 07:42, Bjorn Helgaas wrote:
> On Wed, Aug 19, 2015 at 04:06:38PM +0200, Helge Deller wrote:
>>> On 2015-08-19, at 1:30 AM, Yinghai Lu wrote:
>>>> On Tue, Aug 18, 2015 at 9:48 PM, Meelis Roos <mroos@linux.ee> wrote:
>>>>>> On Tue, Aug 18, 2015 at 12:47 PM, Helge Deller <deller@gmx.de> wrote:
>>>>>>
>>>>>> Then we should change to
>>>>>>
>>>>>> config PCI_BUS_ADDR_T_64BIT
>>>>>>        def_bool y if (ARCH_DMA_ADDR_T_64BIT || SPARC64)
>>>>>>        depends on PCI
>>>>>
>>>>> Why SPARC64? The problem happened on parisc.
>>>> so will not set PCI_BUS_ADDR_T_64BIT for parisc.
>>
>> I think given the current time frame it's probably the best approach to fix this problem for kernel 4.2.
>> It reverts the behaviour back to how it was before (for all arches beside SPARC64).
>> I'm still wondering if/why parisc is the only arch (in the sym53c8xx driver only!) which broke by this change...
>>
>>> I'm not sure this is optimal.  While the A500 may only have 32-bit PCI, c8000 appears to have a mix
>>> of 32 and 64-bit, and rp34XX is all 64-bit.
>>
>> True, but probably nobody of us noticed that we only used the 32-bit PCI interface even with 64bit kernel on parisc up to now?
>> pci_bus_alloc_resource() in drivers/pci/bus.c just disabled (flag=0) all 64bit resources for us.
>> But I agree, I think we need to fix drivers/parisc/lba_pci.c to correctly cope with 64bit pci addresses.
>
> I doubt we can fix the underlying issue before v4.2,

Agreed.

> but I'd at least like to avoid it.  I applied the patch below to
> for-linus.
>
> Yinghai proposed this:
>
>>>>>>        def_bool y if (ARCH_DMA_ADDR_T_64BIT || SPARC64)
>
> I did this instead:
>
> +	def_bool y if (ARCH_DMA_ADDR_T_64BIT || (64BIT && !PARISC))
>
> because (a) Yinghai's proposal suggests this change is related to
> SPARC64, which is misleading, and (b) I want to keep 64-bit bus
> addresses for all 64-bit kernels *except* PA-RISC.

Ok.
  
> I didn't mark it as Tested-by Meelis because I think he tested
> Yinghai's proposal.  But I can add that back, given confirmation.
>
>
> commit 51a660a2732c
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Thu Aug 20 00:08:15 2015 -0500
>
>      PCI: Don't use 64-bit bus addresses on PA-RISC
>
>      Meelis and Helge reported that 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
>      caused HPMCs on A500 and hangs on rp5470.
>
>      PA-RISC does not set ARCH_DMA_ADDR_T_64BIT, even for 64-bit kernels, so
>      prior to 3a9ad0b4fdcd, we always used 32-bit PCI addresses.  After
>      3a9ad0b4fdcd, we do use 64-bit PCI addresses in 64-bit kernels, and
>      apparently there's some PA-RISC problem related to them.
>
>      Fixes: 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
>      Link: http://lkml.kernel.org/r/alpine.LRH.2.11.1507260929000.30065@math.ut.ee
>      Reported-by: Meelis Roos <mroos@linux.ee>
>      Reported-by: Helge Deller <deller@gmx.de>
>      Based-on-idea-by: Yinghai Lu <yinghai@kernel.org>
>      Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Looks OK. I will test later today.

Can you please add
  CC: stable@vger.kernel.org  # v3.19+
since commit 3a9ad0b4fdcd had that too and now all kernels down to 3.19+ are
broken on parisc...

Thanks!
Helge  
  
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 73de4ef..944f500 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -2,7 +2,7 @@
>   # PCI configuration
>   #
>   config PCI_BUS_ADDR_T_64BIT
> -	def_bool y if (ARCH_DMA_ADDR_T_64BIT || 64BIT)
> +	def_bool y if (ARCH_DMA_ADDR_T_64BIT || (64BIT && !PARISC))
>   	depends on PCI
>
>   config PCI_MSI

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu Aug. 20, 2015, 7:31 p.m. UTC | #2 
On Wed, Aug 19, 2015 at 10:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:

> commit 51a660a2732c
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Thu Aug 20 00:08:15 2015 -0500
>
>     PCI: Don't use 64-bit bus addresses on PA-RISC
>
>     Meelis and Helge reported that 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
>     caused HPMCs on A500 and hangs on rp5470.
>
>     PA-RISC does not set ARCH_DMA_ADDR_T_64BIT, even for 64-bit kernels, so
>     prior to 3a9ad0b4fdcd, we always used 32-bit PCI addresses.  After
>     3a9ad0b4fdcd, we do use 64-bit PCI addresses in 64-bit kernels, and
>     apparently there's some PA-RISC problem related to them.
>
>     Fixes: 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
>     Link: http://lkml.kernel.org/r/alpine.LRH.2.11.1507260929000.30065@math.ut.ee
>     Reported-by: Meelis Roos <mroos@linux.ee>
>     Reported-by: Helge Deller <deller@gmx.de>
>     Based-on-idea-by: Yinghai Lu <yinghai@kernel.org>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 73de4ef..944f500 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -2,7 +2,7 @@
>  # PCI configuration
>  #
>  config PCI_BUS_ADDR_T_64BIT
> -       def_bool y if (ARCH_DMA_ADDR_T_64BIT || 64BIT)
> +       def_bool y if (ARCH_DMA_ADDR_T_64BIT || (64BIT && !PARISC))
>         depends on PCI
>
>  config PCI_MSI

Yes, that is better.

Acked-by: Yinghai Lu <yinghai@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Helge Deller Aug. 20, 2015, 9:15 p.m. UTC | #3 
On 20.08.2015 21:31, Yinghai Lu wrote:
> On Wed, Aug 19, 2015 at 10:42 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>
>> commit 51a660a2732c
>> Author: Bjorn Helgaas <bhelgaas@google.com>
>> Date:   Thu Aug 20 00:08:15 2015 -0500
>>
>>      PCI: Don't use 64-bit bus addresses on PA-RISC
>>
>>      Meelis and Helge reported that 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
>>      caused HPMCs on A500 and hangs on rp5470.
>>
>>      PA-RISC does not set ARCH_DMA_ADDR_T_64BIT, even for 64-bit kernels, so
>>      prior to 3a9ad0b4fdcd, we always used 32-bit PCI addresses.  After
>>      3a9ad0b4fdcd, we do use 64-bit PCI addresses in 64-bit kernels, and
>>      apparently there's some PA-RISC problem related to them.
>>
>>      Fixes: 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
>>      Link: http://lkml.kernel.org/r/alpine.LRH.2.11.1507260929000.30065@math.ut.ee
>>      Reported-by: Meelis Roos <mroos@linux.ee>
>>      Reported-by: Helge Deller <deller@gmx.de>
>>      Based-on-idea-by: Yinghai Lu <yinghai@kernel.org>
>>      Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>
>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
>> index 73de4ef..944f500 100644
>> --- a/drivers/pci/Kconfig
>> +++ b/drivers/pci/Kconfig
>> @@ -2,7 +2,7 @@
>>   # PCI configuration
>>   #
>>   config PCI_BUS_ADDR_T_64BIT
>> -       def_bool y if (ARCH_DMA_ADDR_T_64BIT || 64BIT)
>> +       def_bool y if (ARCH_DMA_ADDR_T_64BIT || (64BIT && !PARISC))
>>          depends on PCI
>>
>>   config PCI_MSI
>
> Yes, that is better.
>
> Acked-by: Yinghai Lu <yinghai@kernel.org>


Tested-by: Helge Deller <deller@gmx.de>
   and please add:
CC: stable@vger.kernel.org  # v3.19+

Helge

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Aug. 20, 2015, 9:30 p.m. UTC | #4 
On Thu, Aug 20, 2015 at 2:15 PM, Helge Deller <deller@gmx.de> wrote:
> On 20.08.2015 21:31, Yinghai Lu wrote:
>>
>> On Wed, Aug 19, 2015 at 10:42 PM, Bjorn Helgaas <bhelgaas@google.com>
>> wrote:
>>
>>> commit 51a660a2732c
>>> Author: Bjorn Helgaas <bhelgaas@google.com>
>>> Date:   Thu Aug 20 00:08:15 2015 -0500
>>>
>>>      PCI: Don't use 64-bit bus addresses on PA-RISC
>>>
>>>      Meelis and Helge reported that 3a9ad0b4fdcd ("PCI: Add
>>> pci_bus_addr_t")
>>>      caused HPMCs on A500 and hangs on rp5470.
>>>
>>>      PA-RISC does not set ARCH_DMA_ADDR_T_64BIT, even for 64-bit kernels,
>>> so
>>>      prior to 3a9ad0b4fdcd, we always used 32-bit PCI addresses.  After
>>>      3a9ad0b4fdcd, we do use 64-bit PCI addresses in 64-bit kernels, and
>>>      apparently there's some PA-RISC problem related to them.
>>>
>>>      Fixes: 3a9ad0b4fdcd ("PCI: Add pci_bus_addr_t")
>>>      Link:
>>> http://lkml.kernel.org/r/alpine.LRH.2.11.1507260929000.30065@math.ut.ee
>>>      Reported-by: Meelis Roos <mroos@linux.ee>
>>>      Reported-by: Helge Deller <deller@gmx.de>
>>>      Based-on-idea-by: Yinghai Lu <yinghai@kernel.org>
>>>      Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>>
>>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
>>> index 73de4ef..944f500 100644
>>> --- a/drivers/pci/Kconfig
>>> +++ b/drivers/pci/Kconfig
>>> @@ -2,7 +2,7 @@
>>>   # PCI configuration
>>>   #
>>>   config PCI_BUS_ADDR_T_64BIT
>>> -       def_bool y if (ARCH_DMA_ADDR_T_64BIT || 64BIT)
>>> +       def_bool y if (ARCH_DMA_ADDR_T_64BIT || (64BIT && !PARISC))
>>>          depends on PCI
>>>
>>>   config PCI_MSI
>>
>>
>> Yes, that is better.
>>
>> Acked-by: Yinghai Lu <yinghai@kernel.org>
>
>
>
> Tested-by: Helge Deller <deller@gmx.de>
>   and please add:
> CC: stable@vger.kernel.org  # v3.19+

Done, thanks for testing this!
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 73de4ef..944f500 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -2,7 +2,7 @@ 
 # PCI configuration
 #
 config PCI_BUS_ADDR_T_64BIT
-	def_bool y if (ARCH_DMA_ADDR_T_64BIT || 64BIT)
+	def_bool y if (ARCH_DMA_ADDR_T_64BIT || (64BIT && !PARISC))
 	depends on PCI
 
 config PCI_MSI