All of lore.kernel.org
 help / color / mirror / Atom feed
From: sanekf@nxt.ru
To: Takashi Iwai <tiwai@suse.com>
Cc: Jaroslav Kysela <perex@perex.cz>, Mark Brown <broonie@kernel.org>,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org
Subject: WC vs UC mappings in snd_dma_sg_alloc()
Date: Tue, 19 Oct 2021 18:03:08 +0300	[thread overview]
Message-ID: <493661634654791@mail.yandex.ru> (raw)

Hi

I've stumbled across this code in sound/core/sgbuf.c:

66 static void *snd_dma_sg_alloc(struct snd_dma_buffer *dmab, size_t size)
67 {
< ... >
80 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) {
81 		type = SNDRV_DMA_TYPE_DEV_WC;
82 #ifdef pgprot_noncached
83 		prot = pgprot_noncached(PAGE_KERNEL);
84 #endif
85 	}
< ... >
131 	area = vmap(sgbuf->page_table, sgbuf->pages, VM_MAP, prot);

Does not this violate x86 rules about using the same memory mapping type for all mappings? It seems that the following patch should fix it (only compile tested - my x86 PCs are either without Linux or without sound, and probably in practice this might not trigger any problems since both WC and UC-minus are incoherent types):

-----------------------------

ALSA: memalloc: duly use pgprot_writecombine() for WC mapping

x86 has strict rules about not having memory type aliasing (Documentation/x86/pat.rst). snd_dma_sg_alloc() violates them by mapping first as WC (with set_memory_wc()) and then as UC- (with pgprot_noncached() + vmap()). Switching to pgprot_writecombine() should fix this.

Signed-off-by: Aleksandr Fedorov <halcien@gmail.com>
diff --git a/sound/core/sgbuf.c b/sound/core/sgbuf.c
index 8352a5cdb19f..670b30c3b6e5 100644
--- a/sound/core/sgbuf.c
+++ b/sound/core/sgbuf.c
@@ -79,9 +79,7 @@ static void *snd_dma_sg_alloc(struct snd_dma_buffer *dmab, size_t size)
 		return NULL;
 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) {
 		type = SNDRV_DMA_TYPE_DEV_WC;
-#ifdef pgprot_noncached
-		prot = pgprot_noncached(PAGE_KERNEL);
-#endif
+		prot = pgprot_writecombine(PAGE_KERNEL);
 	}
 	sgbuf->dev = dmab->dev.dev;
 	pages = snd_sgbuf_aligned_pages(size);

WARNING: multiple messages have this Message-ID (diff)
From: sanekf@nxt.ru
To: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org, Mark Brown <broonie@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: WC vs UC mappings in snd_dma_sg_alloc()
Date: Tue, 19 Oct 2021 18:03:08 +0300	[thread overview]
Message-ID: <493661634654791@mail.yandex.ru> (raw)

Hi

I've stumbled across this code in sound/core/sgbuf.c:

66 static void *snd_dma_sg_alloc(struct snd_dma_buffer *dmab, size_t size)
67 {
< ... >
80 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) {
81 		type = SNDRV_DMA_TYPE_DEV_WC;
82 #ifdef pgprot_noncached
83 		prot = pgprot_noncached(PAGE_KERNEL);
84 #endif
85 	}
< ... >
131 	area = vmap(sgbuf->page_table, sgbuf->pages, VM_MAP, prot);

Does not this violate x86 rules about using the same memory mapping type for all mappings? It seems that the following patch should fix it (only compile tested - my x86 PCs are either without Linux or without sound, and probably in practice this might not trigger any problems since both WC and UC-minus are incoherent types):

-----------------------------

ALSA: memalloc: duly use pgprot_writecombine() for WC mapping

x86 has strict rules about not having memory type aliasing (Documentation/x86/pat.rst). snd_dma_sg_alloc() violates them by mapping first as WC (with set_memory_wc()) and then as UC- (with pgprot_noncached() + vmap()). Switching to pgprot_writecombine() should fix this.

Signed-off-by: Aleksandr Fedorov <halcien@gmail.com>
diff --git a/sound/core/sgbuf.c b/sound/core/sgbuf.c
index 8352a5cdb19f..670b30c3b6e5 100644
--- a/sound/core/sgbuf.c
+++ b/sound/core/sgbuf.c
@@ -79,9 +79,7 @@ static void *snd_dma_sg_alloc(struct snd_dma_buffer *dmab, size_t size)
 		return NULL;
 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) {
 		type = SNDRV_DMA_TYPE_DEV_WC;
-#ifdef pgprot_noncached
-		prot = pgprot_noncached(PAGE_KERNEL);
-#endif
+		prot = pgprot_writecombine(PAGE_KERNEL);
 	}
 	sgbuf->dev = dmab->dev.dev;
 	pages = snd_sgbuf_aligned_pages(size);

             reply	other threads:[~2021-10-19 15:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-19 15:03 sanekf [this message]
2021-10-19 15:03 ` WC vs UC mappings in snd_dma_sg_alloc() sanekf
2021-10-20 16:52 ` Takashi Iwai
2021-10-20 16:52   ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=493661634654791@mail.yandex.ru \
    --to=sanekf@nxt.ru \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.