From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F0B3C433EF for ; Mon, 2 May 2022 14:07:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236997AbiEBOKp (ORCPT ); Mon, 2 May 2022 10:10:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49018 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236886AbiEBOKn (ORCPT ); Mon, 2 May 2022 10:10:43 -0400 Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.17.13]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DB03BE03F for ; Mon, 2 May 2022 07:07:14 -0700 (PDT) Received: from mail-yb1-f182.google.com ([209.85.219.182]) by mrelayeu.kundenserver.de (mreue109 [213.165.67.113]) with ESMTPSA (Nemesis) id 1Mw9Lu-1o1oZQ3RQa-00s6bp; Mon, 02 May 2022 16:07:13 +0200 Received: by mail-yb1-f182.google.com with SMTP id i38so26139539ybj.13; Mon, 02 May 2022 07:07:12 -0700 (PDT) X-Gm-Message-State: AOAM530TBEJ+uhwqm/yj0OXyoVNtp6eHj0U4jrZruDpqv0/kCPvwY6tF JQ9uR74kDPK3tkSYi2bMfaIwAKAzcicztfb2u8s= X-Google-Smtp-Source: ABdhPJxu2kBG0AiERlxFFKfvkpQdfWe88hhTfjwg6Tb4DR2KA4NCnbrpoFdptlIq/zYDIRIluCEYsCm8Yu59T63CSbc= X-Received: by 2002:a25:c604:0:b0:645:d969:97a7 with SMTP id k4-20020a25c604000000b00645d96997a7mr9885111ybf.134.1651500431227; Mon, 02 May 2022 07:07:11 -0700 (PDT) MIME-Version: 1.0 References: <1676021ae8b6d7aada0b1806fed99b1b8359bdc4.1651495112.git.hns@goldelico.com> In-Reply-To: <1676021ae8b6d7aada0b1806fed99b1b8359bdc4.1651495112.git.hns@goldelico.com> From: Arnd Bergmann Date: Mon, 2 May 2022 16:06:55 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] wl1251: dynamically allocate memory used for DMA To: "H. Nikolaus Schaller" Cc: Arnd Bergmann , Tony Lindgren , Kalle Valo , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , linux-wireless , Networking , Linux Kernel Mailing List , Discussions about the Letux Kernel , kernel@pyra-handheld.com, linux-omap , Luca Coelho Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K1:YXa7K6DM7g/inUAliZ1ocSrJWVblp21+WfVrXAVMiOlXhM1agaN IKrTeCQPTkK+N/NiniQumqvXDlPUkABARKw1/mrNoTiPpdPIN98l8ZJfMGk9e4bfeinaSqP B26BbrSKr+GK5va2iC7PRPgpSFuewD/KI2SpN3U+LVJivCIJkblAPRz/eIzpngD5pH3HSXE kG2MX7PUMxHje7oWVMwlA== X-UI-Out-Filterresults: notjunk:1;V03:K0:QA1tjOoBAOw=:14F8n/HIJ2DfSX5pYeLCXN 7+nfJ+kBScFx5v4fbHo7Gi9t5YU7SUxZS1r3JNCfhvr6oEQ3vuWwFFp8+m4HjHCsCmFLIL9iV gUN0/hlw3Q75EbP1UDb2R9tGVqEyKtWwZU4W8EpUpbFyY2A9+qluxxHSH/OpJox8WwcVl/0n6 8sALv/QrePw592O2Y+LFAzMbOrX3YP69wY/6BYF+Lw12B5GLGUxJXubO1Z64ghGsr837xS0sE oizFFj+sG0ltdem2BNaCTIZJ/Tk3rp9wj8pfXsWD5VrzmWz4lftLSfRz0+OXr92NSHkgZBU3s jsw49PIxW8xwy0QqsMPUNgyxuVQRH4CzmZihrbLtb0QRt898S6WNIZlwwONDSmFv0jBK84gPS M/oNK64LXNoJt7U6k20GiYX+2caCSZhW+SPFZlzUkLJi+9oglRShKmrXQb2NyD2P+DXh3k+x2 Wl9SBVg6lM1MLXYAXl45jCvfgwsPbr6RRA996l5bdP+i1XUIxOBGAdAJXN5mZ2MkklR002tuC 6a1DWDsTmGoBnSxaL7qfQccrWpirryOcb/9YrWRRIPy+1dY9ZLQ6eTugt3FSRX1veON97QeDo IvbpUrXLPiW5JalxbUouavt4b2XrVTHXxhknbyq/dR1MSOEpLxu2lH1CF8f+efZwkQLUqxAH2 F6YO4/4SaQ88SRXXFhIeCV9cqOGigd4+kig4GzfEEFG+Y5z7O1dSzd9q1Ih5o+tqFgXPdr7N8 U96vOyzmJdFZBBu4PRpdaSDXcu2qKa5KO5N2zzif+hkx22nNcvsiDssvysbbUT3BI7Qg13tcQ Pm4xzNXA4fmKphnNsnng8ahdZrr7Jr9rOM1rSqWxUw17DzgfgM= Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 2, 2022 at 2:38 PM H. Nikolaus Schaller wrote: > With introduction of vmap'ed stacks, stack parameters can no > longer be used for DMA and now leads to kernel panic. > > It happens at several places for the wl1251 (e.g. when > accessed through SDIO) making it unuseable on e.g. the > OpenPandora. > > We solve this by allocating temporary buffers or use wl1251_read32(). This looks all correct to me. I had another look at the related wlcore driver now, and see that the same problem existed there but was fixed back in 2012 in a different way, see 690142e98826 ("wl12xx: fix DMA-API-related warnings"). The approach in the wlcore driver appears to be simpler because it avoids dynamic memory allocation and the associated error handling. However, it probably makes another problem worse that also exists here: static inline u32 wl1251_read32(struct wl1251 *wl, int addr) { u32 response; wl->if_ops->read(wl, addr, &wl->buffer_32, sizeof(wl->buffer_32)); return le32_to_cpu(wl->buffer_32); } I think the 'buffer_32' member of 'struct wl1251' needs an explicit '__cacheline_aligned' attribute to avoid potentially clobbering some of the structure during a DMA write. I don't know if anyone cares enough about the two drivers to have an opinion. I've added Luca to Cc, but he hasn't maintained the driver since 2013 and probably doesn't. It's probably ok to just apply your patch for the moment to fix the regression we saw on the machines that we know use this. One more detail: > diff --git a/drivers/net/wireless/ti/wl1251/event.c b/drivers/net/wireless/ti/wl1251/event.c > index e6d426edab56b..e945aafd88ee5 100644 > --- a/drivers/net/wireless/ti/wl1251/event.c > +++ b/drivers/net/wireless/ti/wl1251/event.c > @@ -169,11 +169,9 @@ int wl1251_event_wait(struct wl1251 *wl, u32 mask, int timeout_ms) > msleep(1); > > /* read from both event fields */ > - wl1251_mem_read(wl, wl->mbox_ptr[0], &events_vector, > - sizeof(events_vector)); > + events_vector = wl1251_mem_read32(wl, wl->mbox_ptr[0]); > event = events_vector & mask; > - wl1251_mem_read(wl, wl->mbox_ptr[1], &events_vector, > - sizeof(events_vector)); > + events_vector = wl1251_mem_read32(wl, wl->mbox_ptr[1]); > event |= events_vector & mask; This appears to change endianness of the data, on big-endian kernels. Is that intentional? My first guess would be that the driver never worked correctly on big-endian machines, and that the change is indeed correct, but on the other hand the conversion was added in commit ac9e2d9afa90 ("wl1251: convert 32-bit values to le32 before writing to the chip") in a way that suggests it was meant to work on both. Arnd