Re: [BUG] commit a8d302a0b77057568350fe0123e639d02dba0745 cause IO_PAGE_FAULT and a lot of errors

[Takashi Iwai <tiwai@suse.de>]

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@suse.de>
> > 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");