Re: [BUG] commit a8d302a0b77057568350fe0123e639d02dba0745 cause IO_PAGE_FAULT and a lot of errors
From: Takashi Iwai
Date: Sun Sep 04 2022 - 04:52:00 EST
On Sun, 04 Sep 2022 09:23:53 +0200,
Takashi Iwai wrote:
>
> On Sat, 03 Sep 2022 20:04:19 +0200,
> Mikhail Gavrilov wrote:
> >
> > Hi, I am bisecting issue that cause errors:
> > [ 57.710235] snd_hda_intel 0000:03:00.1: spurious response
> > 0xeb0cce6a:0x8b612b0d, rp = 1, wp = 1
> > [ 57.710240] ------------[ cut here ]------------
> > [ 57.710241] BUG?
> > [ 57.710257] amd_iommu_report_page_fault: 216 callbacks suppressed
> > [ 57.710260] snd_hda_intel 0000:03:00.1: AMD-Vi: Event logged
> > [IO_PAGE_FAULT domain=0x000e address=0x152848808 flags=0x0020]
>
> Grr... again hitting an issue with AMD IOMMU...
>
> > and bisect said this commit causes it:
> > a8d302a0b77057568350fe0123e639d02dba0745 is the first bad commit
> > commit a8d302a0b77057568350fe0123e639d02dba0745
> > Author: Takashi Iwai <tiwai@xxxxxxx>
> > Date: Sun Aug 21 17:59:11 2022 +0200
> >
> > ALSA: memalloc: Revive x86-specific WC page allocations again
>
> OK, could you try the patch below?
> I wonder whether this is specific to CORB/RIRB mapping or generically
> about the transfer buffers.
Also, please check the patch below instead of the previous one, too.
If this one works, it'd be a better choice.
Takashi
---
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index 9a60bfdb39ba..f00a4819676b 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -46,7 +46,7 @@ void snd_hdac_bus_init_cmd_io(struct hdac_bus *bus)
spin_lock_irq(&bus->reg_lock);
/* CORB set up */
- bus->corb.addr = bus->rb.addr;
+ bus->corb.addr = snd_sgbuf_get_addr(&bus->rb, 0);
bus->corb.buf = (__le32 *)bus->rb.area;
snd_hdac_chip_writel(bus, CORBLBASE, (u32)bus->corb.addr);
snd_hdac_chip_writel(bus, CORBUBASE, upper_32_bits(bus->corb.addr));
@@ -65,7 +65,7 @@ void snd_hdac_bus_init_cmd_io(struct hdac_bus *bus)
snd_hdac_chip_writeb(bus, CORBCTL, AZX_CORBCTL_RUN);
/* RIRB set up */
- bus->rirb.addr = bus->rb.addr + 2048;
+ bus->rirb.addr = snd_sgbuf_get_addr(&bus->rb, 2048);
bus->rirb.buf = (__le32 *)(bus->rb.area + 2048);
bus->rirb.wp = bus->rirb.rp = 0;
memset(bus->rirb.cmds, 0, sizeof(bus->rirb.cmds));
@@ -504,6 +504,8 @@ static void azx_int_clear(struct hdac_bus *bus)
*/
bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset)
{
+ dma_addr_t addr;
+
if (bus->chip_init)
return false;
@@ -520,9 +522,10 @@ bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset)
azx_int_enable(bus);
/* program the position buffer */
- if (bus->use_posbuf && bus->posbuf.addr) {
- snd_hdac_chip_writel(bus, DPLBASE, (u32)bus->posbuf.addr);
- snd_hdac_chip_writel(bus, DPUBASE, upper_32_bits(bus->posbuf.addr));
+ addr = snd_sgbuf_get_addr(&bus->posbuf, 0);
+ if (bus->use_posbuf && addr) {
+ snd_hdac_chip_writel(bus, DPLBASE, (u32)addr);
+ snd_hdac_chip_writel(bus, DPUBASE, upper_32_bits(addr));
}
bus->chip_init = true;
@@ -548,7 +551,7 @@ void snd_hdac_bus_stop_chip(struct hdac_bus *bus)
snd_hdac_bus_stop_cmd_io(bus);
/* disable position buffer */
- if (bus->posbuf.addr) {
+ if (snd_sgbuf_get_addr(&bus->posbuf, 0)) {
snd_hdac_chip_writel(bus, DPLBASE, 0);
snd_hdac_chip_writel(bus, DPUBASE, 0);
}
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index f3582012d22f..f64dfa08ba87 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -212,6 +212,7 @@ int snd_hdac_stream_setup(struct hdac_stream *azx_dev)
{
struct hdac_bus *bus = azx_dev->bus;
struct snd_pcm_runtime *runtime;
+ dma_addr_t addr;
unsigned int val;
if (azx_dev->substream)
@@ -239,17 +240,18 @@ int snd_hdac_stream_setup(struct hdac_stream *azx_dev)
snd_hdac_stream_writew(azx_dev, SD_LVI, azx_dev->frags - 1);
/* program the BDL address */
+ addr = snd_sgbuf_get_addr(&azx_dev->bdl, 0);
/* lower BDL address */
- snd_hdac_stream_writel(azx_dev, SD_BDLPL, (u32)azx_dev->bdl.addr);
+ snd_hdac_stream_writel(azx_dev, SD_BDLPL, (u32)addr);
/* upper BDL address */
- snd_hdac_stream_writel(azx_dev, SD_BDLPU,
- upper_32_bits(azx_dev->bdl.addr));
+ snd_hdac_stream_writel(azx_dev, SD_BDLPU, upper_32_bits(addr));
/* enable the position buffer */
- if (bus->use_posbuf && bus->posbuf.addr) {
+ addr = snd_sgbuf_get_addr(&bus->posbuf, 0);
+ if (bus->use_posbuf && addr) {
if (!(snd_hdac_chip_readl(bus, DPLBASE) & AZX_DPLBASE_ENABLE))
snd_hdac_chip_writel(bus, DPLBASE,
- (u32)bus->posbuf.addr | AZX_DPLBASE_ENABLE);
+ (u32)addr | AZX_DPLBASE_ENABLE);
}
/* set the interrupt enable bits in the descriptor control register */
diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h
index f5bf295eb830..94568d0ab492 100644
--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -28,7 +28,7 @@
#else
#define AZX_DCAPS_I915_COMPONENT 0 /* NOP */
#endif
-/* 14 unused */
+#define AZX_DCAPS_AMD_IOMMU_WORKAROUND (1 << 14) /* workaround for AMD IOMMU page allocations */
#define AZX_DCAPS_CTX_WORKAROUND (1 << 15) /* X-Fi workaround */
#define AZX_DCAPS_POSFIX_LPIB (1 << 16) /* Use LPIB as default */
#define AZX_DCAPS_AMD_WORKAROUND (1 << 17) /* AMD-specific workaround */
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index a77165bd92a9..36408430b7fb 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -302,7 +302,8 @@ enum {
/* quirks for ATI HDMI with snoop off */
#define AZX_DCAPS_PRESET_ATI_HDMI_NS \
- (AZX_DCAPS_PRESET_ATI_HDMI | AZX_DCAPS_SNOOP_OFF)
+ (AZX_DCAPS_PRESET_ATI_HDMI | AZX_DCAPS_SNOOP_OFF |\
+ AZX_DCAPS_AMD_IOMMU_WORKAROUND)
/* quirks for AMD SB */
#define AZX_DCAPS_PRESET_AMD_SB \
@@ -1816,8 +1817,12 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
return err;
/* use the non-cached pages in non-snoop mode */
- if (!azx_snoop(chip))
- azx_bus(chip)->dma_type = SNDRV_DMA_TYPE_DEV_WC;
+ if (!azx_snoop(chip)) {
+ if (chip->driver_caps & AZX_DCAPS_AMD_IOMMU_WORKAROUND)
+ azx_bus(chip)->dma_type = SNDRV_DMA_TYPE_DEV_WC_SG;
+ else
+ azx_bus(chip)->dma_type = SNDRV_DMA_TYPE_DEV_WC;
+ }
if (chip->driver_type == AZX_DRIVER_NVIDIA) {
dev_dbg(chip->card->dev, "Enable delay in RIRB handling\n");