From nobody Wed Dec 4 08:38:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org ARC-Seal: i=1; a=rsa-sha256; t=1725648203; cv=none; d=zohomail.com; s=zohoarc; b=DRs64M8ID6p10JzEQLc/jSzh106jsbUCegRlYTsHgyVBddHnW3R2IKO7d1do/rSaJDEBP56ubbnzwN+X4pqN94L2r0E5vw388oaVgcXdYZPQJX4p2XpURkNHRJ4o+Fp5P6+2lwAoXEeJs+lrk8Zpl9NJ21gBClcNmVs+FStyH6M= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1725648203; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:Subject:To:To:Message-Id:Reply-To; bh=M0Q9XOB/kMF0+B3U+RveaDZ0cH/WiG/ZmzrA54s0hxc=; b=blNHOlGn73av8Pi8IXea/1OUMAFlnRKYCGiXtsF3cZa2h/wW8k90of46mCzuc9TSJalNnhTnigIVQ/aERwLapxqHZPYiFVlWXxcfNwXVLRVx4TVNP6rT64ovygJctPjRDjjPtN/5LX86wHehptk8pSutPGa5AWkrUTHnCFAmFZw= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1725648203508808.3178140041532; Fri, 6 Sep 2024 11:43:23 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.791997.1202044 (Exim 4.92) (envelope-from ) id 1smdul-0000VC-Pb; Fri, 06 Sep 2024 18:42:35 +0000 Received: by outflank-mailman (output) from mailman id 791997.1202044; Fri, 06 Sep 2024 18:42:35 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1smdul-0000V5-Mp; Fri, 06 Sep 2024 18:42:35 +0000 Received: by outflank-mailman (input) for mailman id 791997; Fri, 06 Sep 2024 18:42:34 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1smduk-0000Uz-6P for xen-devel@lists.xenproject.org; Fri, 06 Sep 2024 18:42:34 +0000 Received: from pv50p00im-zteg10021401.me.com (pv50p00im-zteg10021401.me.com [17.58.6.47]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id c5d8016b-6c7f-11ef-99a1-01e77a169b0f; Fri, 06 Sep 2024 20:42:31 +0200 (CEST) Received: from penelo.taild41b8.ts.net (pv50p00im-dlb-asmtp-mailmevip.me.com [17.56.9.10]) by pv50p00im-zteg10021401.me.com (Postfix) with ESMTPSA id 861EE8E07B7; Fri, 6 Sep 2024 18:42:27 +0000 (UTC) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: c5d8016b-6c7f-11ef-99a1-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ariadne.space; s=sig1; t=1725648150; bh=M0Q9XOB/kMF0+B3U+RveaDZ0cH/WiG/ZmzrA54s0hxc=; h=From:To:Subject:Date:Message-Id:MIME-Version; b=no0ngRzzC7FEjI9UXUSQpIWlC1yaJS5visGBOvAG0FbYV5JYtUDSBkK0Ngg23ayPv PLCSRy/P9fBRUssIWTNN4achy78m1NBwFimPEjTwKNHuVOzkp/wKjB2UQ+XXIBnEDn O/Vz2DD0aaHvrm+94h6TnZ0hX/i30W5g1cKGeJqSfnwihJMfsPTxkgEp8lWoooPrQU V8UZuyxFkozTVlHlNcQKW6XWqfZGABG9GagtjLvZEMNnLaIJg46D48FRBf71D/zbT2 W9gMBfQIum1bIPt4lkVhBMx8JTG111wYkb6qukzyzWXuQEsnGgUpBzpLhnL5sVrHUh erCpwRQXipSDw== From: Ariadne Conill To: xen-devel@lists.xenproject.org, alsa-devel@alsa-project.org Cc: Ariadne Conill , stable@vger.kernel.org, Takashi Iwai Subject: [PATCH] Revert "ALSA: memalloc: Workaround for Xen PV" Date: Fri, 6 Sep 2024 11:42:09 -0700 Message-Id: <20240906184209.25423-1-ariadne@ariadne.space> X-Mailer: git-send-email 2.39.3 (Apple Git-146) MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Proofpoint-ORIG-GUID: lk0ouPNqe5kE8xXEoigDnu-oJgDNJMcY X-Proofpoint-GUID: lk0ouPNqe5kE8xXEoigDnu-oJgDNJMcY X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.60.29 definitions=2024-09-06_04,2024-09-06_01,2024-09-02_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 malwarescore=0 bulkscore=0 suspectscore=0 mlxlogscore=999 clxscore=1030 spamscore=0 phishscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2308100000 definitions=main-2409060138 X-ZohoMail-DKIM: pass (identity @ariadne.space) X-ZM-MESSAGEID: 1725648204998116600 Content-Type: text/plain; charset="utf-8" This patch attempted to work around a DMA issue involving Xen, but causes subtle kernel memory corruption. When I brought up this patch in the XenDevel matrix channel, I was told that it had been requested by the Qubes OS developers because they were trying to fix an issue where the sound stack would fail after a few hours of uptime. They wound up disabling SG buffering entirely instead as a workaround. Accordingly, I propose that we should revert this workaround patch, since it causes kernel memory corruption and that the ALSA and Xen communities should collaborate on fixing the underlying problem in such a way that SG buffering works correctly under Xen. This reverts commit 53466ebdec614f915c691809b0861acecb941e30. Signed-off-by: Ariadne Conill Cc: stable@vger.kernel.org Cc: xen-devel@lists.xenproject.org Cc: alsa-devel@alsa-project.org Cc: Takashi Iwai --- sound/core/memalloc.c | 87 +++++++++---------------------------------- 1 file changed, 18 insertions(+), 69 deletions(-) diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index f901504b5afc..81025f50a542 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -541,15 +541,16 @@ static void *snd_dma_noncontig_alloc(struct snd_dma_b= uffer *dmab, size_t size) struct sg_table *sgt; void *p; =20 -#ifdef CONFIG_SND_DMA_SGBUF - if (cpu_feature_enabled(X86_FEATURE_XENPV)) - return snd_dma_sg_fallback_alloc(dmab, size); -#endif sgt =3D dma_alloc_noncontiguous(dmab->dev.dev, size, dmab->dev.dir, DEFAULT_GFP, 0); #ifdef CONFIG_SND_DMA_SGBUF - if (!sgt && !get_dma_ops(dmab->dev.dev)) + if (!sgt && !get_dma_ops(dmab->dev.dev)) { + if (dmab->dev.type =3D=3D SNDRV_DMA_TYPE_DEV_WC_SG) + dmab->dev.type =3D SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK; + else + dmab->dev.type =3D SNDRV_DMA_TYPE_DEV_SG_FALLBACK; return snd_dma_sg_fallback_alloc(dmab, size); + } #endif if (!sgt) return NULL; @@ -716,38 +717,19 @@ static const struct snd_malloc_ops snd_dma_sg_wc_ops = =3D { =20 /* Fallback SG-buffer allocations for x86 */ struct snd_dma_sg_fallback { - bool use_dma_alloc_coherent; size_t count; struct page **pages; - /* DMA address array; the first page contains #pages in ~PAGE_MASK */ - dma_addr_t *addrs; }; =20 static void __snd_dma_sg_fallback_free(struct snd_dma_buffer *dmab, struct snd_dma_sg_fallback *sgbuf) { - size_t i, size; - - if (sgbuf->pages && sgbuf->addrs) { - i =3D 0; - while (i < sgbuf->count) { - if (!sgbuf->pages[i] || !sgbuf->addrs[i]) - break; - size =3D sgbuf->addrs[i] & ~PAGE_MASK; - if (WARN_ON(!size)) - break; - if (sgbuf->use_dma_alloc_coherent) - dma_free_coherent(dmab->dev.dev, size << PAGE_SHIFT, - page_address(sgbuf->pages[i]), - sgbuf->addrs[i] & PAGE_MASK); - else - do_free_pages(page_address(sgbuf->pages[i]), - size << PAGE_SHIFT, false); - i +=3D size; - } - } + bool wc =3D dmab->dev.type =3D=3D SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK; + size_t i; + + for (i =3D 0; i < sgbuf->count && sgbuf->pages[i]; i++) + do_free_pages(page_address(sgbuf->pages[i]), PAGE_SIZE, wc); kvfree(sgbuf->pages); - kvfree(sgbuf->addrs); kfree(sgbuf); } =20 @@ -756,36 +738,24 @@ static void *snd_dma_sg_fallback_alloc(struct snd_dma= _buffer *dmab, size_t size) struct snd_dma_sg_fallback *sgbuf; struct page **pagep, *curp; size_t chunk, npages; - dma_addr_t *addrp; dma_addr_t addr; void *p; - - /* correct the type */ - if (dmab->dev.type =3D=3D SNDRV_DMA_TYPE_DEV_SG) - dmab->dev.type =3D SNDRV_DMA_TYPE_DEV_SG_FALLBACK; - else if (dmab->dev.type =3D=3D SNDRV_DMA_TYPE_DEV_WC_SG) - dmab->dev.type =3D SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK; + bool wc =3D dmab->dev.type =3D=3D SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK; =20 sgbuf =3D kzalloc(sizeof(*sgbuf), GFP_KERNEL); if (!sgbuf) return NULL; - sgbuf->use_dma_alloc_coherent =3D cpu_feature_enabled(X86_FEATURE_XENPV); size =3D PAGE_ALIGN(size); sgbuf->count =3D size >> PAGE_SHIFT; sgbuf->pages =3D kvcalloc(sgbuf->count, sizeof(*sgbuf->pages), GFP_KERNEL= ); - sgbuf->addrs =3D kvcalloc(sgbuf->count, sizeof(*sgbuf->addrs), GFP_KERNEL= ); - if (!sgbuf->pages || !sgbuf->addrs) + if (!sgbuf->pages) goto error; =20 pagep =3D sgbuf->pages; - addrp =3D sgbuf->addrs; - chunk =3D (PAGE_SIZE - 1) << PAGE_SHIFT; /* to fit in low bits in addrs */ + chunk =3D size; while (size > 0) { chunk =3D min(size, chunk); - if (sgbuf->use_dma_alloc_coherent) - p =3D dma_alloc_coherent(dmab->dev.dev, chunk, &addr, DEFAULT_GFP); - else - p =3D do_alloc_pages(dmab->dev.dev, chunk, &addr, false); + p =3D do_alloc_pages(dmab->dev.dev, chunk, &addr, wc); if (!p) { if (chunk <=3D PAGE_SIZE) goto error; @@ -797,25 +767,17 @@ static void *snd_dma_sg_fallback_alloc(struct snd_dma= _buffer *dmab, size_t size) size -=3D chunk; /* fill pages */ npages =3D chunk >> PAGE_SHIFT; - *addrp =3D npages; /* store in lower bits */ curp =3D virt_to_page(p); - while (npages--) { + while (npages--) *pagep++ =3D curp++; - *addrp++ |=3D addr; - addr +=3D PAGE_SIZE; - } } =20 p =3D vmap(sgbuf->pages, sgbuf->count, VM_MAP, PAGE_KERNEL); if (!p) goto error; - - if (dmab->dev.type =3D=3D SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK) - set_pages_array_wc(sgbuf->pages, sgbuf->count); - dmab->private_data =3D sgbuf; /* store the first page address for convenience */ - dmab->addr =3D sgbuf->addrs[0] & PAGE_MASK; + dmab->addr =3D snd_sgbuf_get_addr(dmab, 0); return p; =20 error: @@ -825,23 +787,10 @@ static void *snd_dma_sg_fallback_alloc(struct snd_dma= _buffer *dmab, size_t size) =20 static void snd_dma_sg_fallback_free(struct snd_dma_buffer *dmab) { - struct snd_dma_sg_fallback *sgbuf =3D dmab->private_data; - - if (dmab->dev.type =3D=3D SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK) - set_pages_array_wb(sgbuf->pages, sgbuf->count); vunmap(dmab->area); __snd_dma_sg_fallback_free(dmab, dmab->private_data); } =20 -static dma_addr_t snd_dma_sg_fallback_get_addr(struct snd_dma_buffer *dmab, - size_t offset) -{ - struct snd_dma_sg_fallback *sgbuf =3D dmab->private_data; - size_t index =3D offset >> PAGE_SHIFT; - - return (sgbuf->addrs[index] & PAGE_MASK) | (offset & ~PAGE_MASK); -} - static int snd_dma_sg_fallback_mmap(struct snd_dma_buffer *dmab, struct vm_area_struct *area) { @@ -856,8 +805,8 @@ static const struct snd_malloc_ops snd_dma_sg_fallback_= ops =3D { .alloc =3D snd_dma_sg_fallback_alloc, .free =3D snd_dma_sg_fallback_free, .mmap =3D snd_dma_sg_fallback_mmap, - .get_addr =3D snd_dma_sg_fallback_get_addr, /* reuse vmalloc helpers */ + .get_addr =3D snd_dma_vmalloc_get_addr, .get_page =3D snd_dma_vmalloc_get_page, .get_chunk_size =3D snd_dma_vmalloc_get_chunk_size, }; --=20 2.39.2