Re: 答复: A bug about cache inconsistency report

[Takashi Iwai <tiwai@suse.de>]

On Tue, 07 Aug 2018 12:59:04 +0200,
Hans Hu(SH-RD) wrote:
> 
> > >Then the next step would be to fake sg-buffer from this straight
> > >buffer.  Revert the above, and modify sgbuf.c to the following:
> > >- Allocate a large continuous buffer
> > >- Assign each page in this large buffer
> >
> > >If this still works, it's not about vmap, but it just means that the
> > >physically ordered pages do matter -- implicitly showing that the
> > >snooping behavior isn't properly turned on / off on the controller.
> >
> > To fake SG-buffer, I did this test: restore all the codes to the original, then added some codes in snd_malloc_sgbuf_pages() like below, the result is badly niose.
> 
> >Not really what I meant.  Rather something like below (totally untested).
> 
> [Hans :] I know what you mean now and complete code like below, but the result is still noise.

OK, so indeed the vmapped address does seem matter.  Interesting.
What kind of user access does produce the noise?  Does it via aplay
mmap mode, too?

In anyway, if the vmap is a problem, it might be worked around a patch
like below (again totally untested and not sure whether it's correct).


Takashi

---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -410,12 +410,23 @@ static void __mark_pages_wc(struct azx *chip, struct snd_dma_buffer *dmab, bool
 #ifdef CONFIG_SND_DMA_SGBUF
 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG) {
 		struct snd_sg_buf *sgbuf = dmab->private_data;
+		pgprot_t prot;
+		void *new_area;
+
 		if (chip->driver_type == AZX_DRIVER_CMEDIA)
 			return; /* deal with only CORB/RIRB buffers */
-		if (on)
+		if (on) {
 			set_pages_array_wc(sgbuf->page_table, sgbuf->pages);
-		else
+			prot = pgprot_noncached(PAGE_KERNEL);
+		} else {
 			set_pages_array_wb(sgbuf->page_table, sgbuf->pages);
+			prot = PAGE_KERNEL;
+		}
+		new_area = vmap(sgbuf->page_table, sgbuf->pages, VM_MAP, prot);
+		if (WARN_ON(!new_area))
+			return;
+		vunmap(dmab->area);
+		dmab->area = new_area;
 		return;
 	}
 #endif