Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1502172 > unrolled thread

[PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices

Started byArd Biesheuvel <ard.biesheuvel@linaro.org>
First post2016-10-17 18:30 +0200
Last post2016-10-18 16:20 +0200
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-10-17 18:30 +0200
    Re: [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Takashi Iwai <tiwai@suse.de> - 2016-10-18 12:30 +0200
      Re: [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-10-18 12:30 +0200
      Re: [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-10-18 14:50 +0200
        Re: [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Mike Travis <travis@sgi.com> - 2016-10-18 15:10 +0200
          Re: [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-10-18 15:20 +0200
      Re: [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Mike Travis <travis@sgi.com> - 2016-10-18 14:50 +0200
        Re: [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices Takashi Iwai <tiwai@suse.de> - 2016-10-18 16:20 +0200

#1502172 — [PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2016-10-17 18:30 +0200
Subject[PATCH] ALSA: hda - allow 40 bit DMA mask for NVidia devices
Message-ID<stiXw-5Qr-23@gated-at.bofh.it>
Commit 49d9e77e72cf ("ALSA: hda - Fix system panic when DMA > 40 bits
for Nvidia audio controllers") simply disabled any DMA exceeding 32
bits for NVidia devices, even though they are capable of performing
DMA up to 40 bits. On some architectures (such as arm64), system memory
is not guaranteed to be 32-bit addressable by PCI devices, and so this
change prevents NVidia devices from working on platforms such as AMD
Seattle.

Since the original commit already mentioned that up to 40 bits of DMA
is supported, and given that the code has been updated in the meantime
to support a 40 bit DMA mask on other devices, revert commit 49d9e77e72cf
and explicitly set the DMA mask to 40 bits for NVidia devices.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 sound/pci/hda/hda_intel.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index c3469f756ec2..c64d986009a9 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -341,8 +341,7 @@ enum {
 
 /* quirks for Nvidia */
 #define AZX_DCAPS_PRESET_NVIDIA \
-	(AZX_DCAPS_NO_MSI | /*AZX_DCAPS_ALIGN_BUFSIZE |*/ \
-	 AZX_DCAPS_NO_64BIT | AZX_DCAPS_CORBRP_SELF_CLEAR |\
+	(AZX_DCAPS_NO_MSI | AZX_DCAPS_CORBRP_SELF_CLEAR |\
 	 AZX_DCAPS_SNOOP_TYPE(NVIDIA))
 
 #define AZX_DCAPS_PRESET_CTHDA \
@@ -1716,6 +1715,10 @@ static int azx_first_init(struct azx *chip)
 		}
 	}
 
+	/* NVidia hardware normally only supports up to 40 bits of DMA */
+	if (chip->pci->vendor == PCI_VENDOR_ID_NVIDIA)
+		dma_bits = 40;
+
 	/* disable 64bit DMA address on some devices */
 	if (chip->driver_caps & AZX_DCAPS_NO_64BIT) {
 		dev_dbg(card->dev, "Disabling 64bit DMA\n");
-- 
2.7.4

[toc] | [next] | [standalone]

#1502874

FromTakashi Iwai <tiwai@suse.de>
Date2016-10-18 12:30 +0200
Message-ID<stzOF-lH-1@gated-at.bofh.it>
In reply to#1502172
On Mon, 17 Oct 2016 18:23:59 +0200,
Ard Biesheuvel wrote:
> 
> Commit 49d9e77e72cf ("ALSA: hda - Fix system panic when DMA > 40 bits
> for Nvidia audio controllers") simply disabled any DMA exceeding 32
> bits for NVidia devices, even though they are capable of performing
> DMA up to 40 bits. On some architectures (such as arm64), system memory
> is not guaranteed to be 32-bit addressable by PCI devices, and so this
> change prevents NVidia devices from working on platforms such as AMD
> Seattle.
> 
> Since the original commit already mentioned that up to 40 bits of DMA
> is supported, and given that the code has been updated in the meantime
> to support a 40 bit DMA mask on other devices, revert commit 49d9e77e72cf
> and explicitly set the DMA mask to 40 bits for NVidia devices.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks, applied.  Since this seems causing a real problem, I added Cc
to stable.


Takashi


> ---
>  sound/pci/hda/hda_intel.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index c3469f756ec2..c64d986009a9 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -341,8 +341,7 @@ enum {
>  
>  /* quirks for Nvidia */
>  #define AZX_DCAPS_PRESET_NVIDIA \
> -	(AZX_DCAPS_NO_MSI | /*AZX_DCAPS_ALIGN_BUFSIZE |*/ \
> -	 AZX_DCAPS_NO_64BIT | AZX_DCAPS_CORBRP_SELF_CLEAR |\
> +	(AZX_DCAPS_NO_MSI | AZX_DCAPS_CORBRP_SELF_CLEAR |\
>  	 AZX_DCAPS_SNOOP_TYPE(NVIDIA))
>  
>  #define AZX_DCAPS_PRESET_CTHDA \
> @@ -1716,6 +1715,10 @@ static int azx_first_init(struct azx *chip)
>  		}
>  	}
>  
> +	/* NVidia hardware normally only supports up to 40 bits of DMA */
> +	if (chip->pci->vendor == PCI_VENDOR_ID_NVIDIA)
> +		dma_bits = 40;
> +
>  	/* disable 64bit DMA address on some devices */
>  	if (chip->driver_caps & AZX_DCAPS_NO_64BIT) {
>  		dev_dbg(card->dev, "Disabling 64bit DMA\n");
> -- 
> 2.7.4
> 
>

[toc] | [prev] | [next] | [standalone]

#1502879

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2016-10-18 12:30 +0200
Message-ID<stzOF-lH-11@gated-at.bofh.it>
In reply to#1502874
On 18 October 2016 at 11:25, Takashi Iwai <tiwai@suse.de> wrote:
> On Mon, 17 Oct 2016 18:23:59 +0200,
> Ard Biesheuvel wrote:
>>
>> Commit 49d9e77e72cf ("ALSA: hda - Fix system panic when DMA > 40 bits
>> for Nvidia audio controllers") simply disabled any DMA exceeding 32
>> bits for NVidia devices, even though they are capable of performing
>> DMA up to 40 bits. On some architectures (such as arm64), system memory
>> is not guaranteed to be 32-bit addressable by PCI devices, and so this
>> change prevents NVidia devices from working on platforms such as AMD
>> Seattle.
>>
>> Since the original commit already mentioned that up to 40 bits of DMA
>> is supported, and given that the code has been updated in the meantime
>> to support a 40 bit DMA mask on other devices, revert commit 49d9e77e72cf
>> and explicitly set the DMA mask to 40 bits for NVidia devices.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Thanks, applied.  Since this seems causing a real problem, I added Cc
> to stable.
>

Thanks!

[toc] | [prev] | [next] | [standalone]

#1502957

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2016-10-18 14:50 +0200
Message-ID<stC0f-1Ip-3@gated-at.bofh.it>
In reply to#1502874
On 18 October 2016 at 13:46, Mike Travis <travis@sgi.com> wrote:
>
> Have you tested this change on a system which has physical
> addressing > 40 bits to insure that addresses > 40 bits are
> not simply truncated to an incorrect (mod 40) address?

The device should never see addresses > 40 bits, given that the DMA
mask is set to 40 bits not 64.

> By not restricting the addresses to 32 bits you are not
> restricting the device to the DMA32 zone.
>
> This is what caused the original panic on UV systems.
>
> Btw, the newest nVidia GPUs have > 40 bits of DMA address bits.
>

Thanks,
Ard.

[toc] | [prev] | [next] | [standalone]

#1502979

FromMike Travis <travis@sgi.com>
Date2016-10-18 15:10 +0200
Message-ID<stCjw-25f-19@gated-at.bofh.it>
In reply to#1502957

On 10/18/2016 5:49 AM, Ard Biesheuvel wrote:
> On 18 October 2016 at 13:46, Mike Travis <travis@sgi.com> wrote:
>>
>> Have you tested this change on a system which has physical
>> addressing > 40 bits to insure that addresses > 40 bits are
>> not simply truncated to an incorrect (mod 40) address?
> 
> The device should never see addresses > 40 bits, given that the DMA
> mask is set to 40 bits not 64.

I agree that the device should never see that.  But how is that
restriction imposed?  If a user presents an buffer address > 40 bits
how would that be converted to useful buffer at <= 40 bits?

I believe I tried this (at the time the kernel was at version 2.6)
and it did not work.  Since audio I/O was such a low (relative)
bandwidth, it was not worth the effort to rework the audio code
to allow buffers > 32 bits but <= 40 bits.  It may have changed
since then?

> 
>> By not restricting the addresses to 32 bits you are not
>> restricting the device to the DMA32 zone.
>>
>> This is what caused the original panic on UV systems.
>>
>> Btw, the newest nVidia GPUs have > 40 bits of DMA address bits.
>>
> 
> Thanks,
> Ard.
>

[toc] | [prev] | [next] | [standalone]

#1502984

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2016-10-18 15:20 +0200
Message-ID<stCtb-294-1@gated-at.bofh.it>
In reply to#1502979
On 18 October 2016 at 14:04, Mike Travis <travis@sgi.com> wrote:
>
>
> On 10/18/2016 5:49 AM, Ard Biesheuvel wrote:
>> On 18 October 2016 at 13:46, Mike Travis <travis@sgi.com> wrote:
>>>
>>> Have you tested this change on a system which has physical
>>> addressing > 40 bits to insure that addresses > 40 bits are
>>> not simply truncated to an incorrect (mod 40) address?
>>
>> The device should never see addresses > 40 bits, given that the DMA
>> mask is set to 40 bits not 64.
>
> I agree that the device should never see that.  But how is that
> restriction imposed?  If a user presents an buffer address > 40 bits
> how would that be converted to useful buffer at <= 40 bits?
>
> I believe I tried this (at the time the kernel was at version 2.6)
> and it did not work.  Since audio I/O was such a low (relative)
> bandwidth, it was not worth the effort to rework the audio code
> to allow buffers > 32 bits but <= 40 bits.  It may have changed
> since then?
>

This is very platform dependent. Note that the 40 bit DMA mask
describes the memory view from the PCI side, which may not be mapped
1:1 with system memory (although it usually is on x86)

In general, a platform will wire up the device to a struct dma_map_ops
{} instance that describes how DMA is performed for this particular
device on this platform. Whether it resorts to bounce buffering or
simply maps the buffer into the device accessible window using an
IOMMU is thus also platform dependent. But in general, all these
drivers ensure that the device address that results from a mapping
adheres to the DMA mask that has been set at probe time.

[toc] | [prev] | [next] | [standalone]

#1502959

FromMike Travis <travis@sgi.com>
Date2016-10-18 14:50 +0200
Message-ID<stC0f-1Ip-5@gated-at.bofh.it>
In reply to#1502874
Have you tested this change on a system which has physical
addressing > 40 bits to insure that addresses > 40 bits are
not simply truncated to an incorrect (mod 40) address?
By not restricting the addresses to 32 bits you are not
restricting the device to the DMA32 zone.

This is what caused the original panic on UV systems.

Btw, the newest nVidia GPUs have > 40 bits of DMA address bits.

On 10/18/2016 3:25 AM, Takashi Iwai wrote:
> On Mon, 17 Oct 2016 18:23:59 +0200,
> Ard Biesheuvel wrote:
>>
>> Commit 49d9e77e72cf ("ALSA: hda - Fix system panic when DMA > 40 bits
>> for Nvidia audio controllers") simply disabled any DMA exceeding 32
>> bits for NVidia devices, even though they are capable of performing
>> DMA up to 40 bits. On some architectures (such as arm64), system memory
>> is not guaranteed to be 32-bit addressable by PCI devices, and so this
>> change prevents NVidia devices from working on platforms such as AMD
>> Seattle.
>>
>> Since the original commit already mentioned that up to 40 bits of DMA
>> is supported, and given that the code has been updated in the meantime
>> to support a 40 bit DMA mask on other devices, revert commit 49d9e77e72cf
>> and explicitly set the DMA mask to 40 bits for NVidia devices.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Thanks, applied.  Since this seems causing a real problem, I added Cc
> to stable.
> 
> 
> Takashi
> 
> 
>> ---
>>  sound/pci/hda/hda_intel.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
>> index c3469f756ec2..c64d986009a9 100644
>> --- a/sound/pci/hda/hda_intel.c
>> +++ b/sound/pci/hda/hda_intel.c
>> @@ -341,8 +341,7 @@ enum {
>>  
>>  /* quirks for Nvidia */
>>  #define AZX_DCAPS_PRESET_NVIDIA \
>> -	(AZX_DCAPS_NO_MSI | /*AZX_DCAPS_ALIGN_BUFSIZE |*/ \
>> -	 AZX_DCAPS_NO_64BIT | AZX_DCAPS_CORBRP_SELF_CLEAR |\
>> +	(AZX_DCAPS_NO_MSI | AZX_DCAPS_CORBRP_SELF_CLEAR |\
>>  	 AZX_DCAPS_SNOOP_TYPE(NVIDIA))
>>  
>>  #define AZX_DCAPS_PRESET_CTHDA \
>> @@ -1716,6 +1715,10 @@ static int azx_first_init(struct azx *chip)
>>  		}
>>  	}
>>  
>> +	/* NVidia hardware normally only supports up to 40 bits of DMA */
>> +	if (chip->pci->vendor == PCI_VENDOR_ID_NVIDIA)
>> +		dma_bits = 40;
>> +
>>  	/* disable 64bit DMA address on some devices */
>>  	if (chip->driver_caps & AZX_DCAPS_NO_64BIT) {
>>  		dev_dbg(card->dev, "Disabling 64bit DMA\n");
>> -- 
>> 2.7.4
>>
>>

[toc] | [prev] | [next] | [standalone]

#1503021

FromTakashi Iwai <tiwai@suse.de>
Date2016-10-18 16:20 +0200
Message-ID<stDpf-2Og-11@gated-at.bofh.it>
In reply to#1502959
On Tue, 18 Oct 2016 14:46:29 +0200,
Mike Travis wrote:
> 
> 
> Have you tested this change on a system which has physical
> addressing > 40 bits to insure that addresses > 40 bits are
> not simply truncated to an incorrect (mod 40) address?
> By not restricting the addresses to 32 bits you are not
> restricting the device to the DMA32 zone.
> 
> This is what caused the original panic on UV systems.

But shouldn't it be handled properly by dma_set_mask() and
dma_set_coherent_mask()?  


Takashi

> Btw, the newest nVidia GPUs have > 40 bits of DMA address bits.
> 
> On 10/18/2016 3:25 AM, Takashi Iwai wrote:
> > On Mon, 17 Oct 2016 18:23:59 +0200,
> > Ard Biesheuvel wrote:
> >>
> >> Commit 49d9e77e72cf ("ALSA: hda - Fix system panic when DMA > 40 bits
> >> for Nvidia audio controllers") simply disabled any DMA exceeding 32
> >> bits for NVidia devices, even though they are capable of performing
> >> DMA up to 40 bits. On some architectures (such as arm64), system memory
> >> is not guaranteed to be 32-bit addressable by PCI devices, and so this
> >> change prevents NVidia devices from working on platforms such as AMD
> >> Seattle.
> >>
> >> Since the original commit already mentioned that up to 40 bits of DMA
> >> is supported, and given that the code has been updated in the meantime
> >> to support a 40 bit DMA mask on other devices, revert commit 49d9e77e72cf
> >> and explicitly set the DMA mask to 40 bits for NVidia devices.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > 
> > Thanks, applied.  Since this seems causing a real problem, I added Cc
> > to stable.
> > 
> > 
> > Takashi
> > 
> > 
> >> ---
> >>  sound/pci/hda/hda_intel.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> >> index c3469f756ec2..c64d986009a9 100644
> >> --- a/sound/pci/hda/hda_intel.c
> >> +++ b/sound/pci/hda/hda_intel.c
> >> @@ -341,8 +341,7 @@ enum {
> >>  
> >>  /* quirks for Nvidia */
> >>  #define AZX_DCAPS_PRESET_NVIDIA \
> >> -	(AZX_DCAPS_NO_MSI | /*AZX_DCAPS_ALIGN_BUFSIZE |*/ \
> >> -	 AZX_DCAPS_NO_64BIT | AZX_DCAPS_CORBRP_SELF_CLEAR |\
> >> +	(AZX_DCAPS_NO_MSI | AZX_DCAPS_CORBRP_SELF_CLEAR |\
> >>  	 AZX_DCAPS_SNOOP_TYPE(NVIDIA))
> >>  
> >>  #define AZX_DCAPS_PRESET_CTHDA \
> >> @@ -1716,6 +1715,10 @@ static int azx_first_init(struct azx *chip)
> >>  		}
> >>  	}
> >>  
> >> +	/* NVidia hardware normally only supports up to 40 bits of DMA */
> >> +	if (chip->pci->vendor == PCI_VENDOR_ID_NVIDIA)
> >> +		dma_bits = 40;
> >> +
> >>  	/* disable 64bit DMA address on some devices */
> >>  	if (chip->driver_caps & AZX_DCAPS_NO_64BIT) {
> >>  		dev_dbg(card->dev, "Disabling 64bit DMA\n");
> >> -- 
> >> 2.7.4
> >>
> >>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web