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 X-Spam-Level: X-Spam-Status: No, score=-7.5 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 84A30C4338F for ; Wed, 18 Aug 2021 03:17:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5B61061051 for ; Wed, 18 Aug 2021 03:17:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234171AbhHRDRs (ORCPT ); Tue, 17 Aug 2021 23:17:48 -0400 Received: from mail-ed1-f49.google.com ([209.85.208.49]:45755 "EHLO mail-ed1-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229883AbhHRDRr (ORCPT ); Tue, 17 Aug 2021 23:17:47 -0400 Received: by mail-ed1-f49.google.com with SMTP id cq23so793768edb.12 for ; Tue, 17 Aug 2021 20:17:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=DPc7urjlfbXi4JUqkG5U5x2Zz7cdVK7h03OfY0SFENs=; b=oo0bZ9p77le6IOefWW9UBEFofRE3Zx22yMMMu+rQgpkP08fMcp7ciouvbfPEYODIpj tQiw1+emU1sCyH9f6heirZ/VoAUUy7SYAkwhyMw93nWblPG14g4pTLdYBlXn24awJL73 Y3FzU867oFVAH8jnuZsBAB9oXnWAAfUWmyRFOzE2o2EhRdfyDOT5P18e0ncSzcNmrDkO Pvyc6XG4nsQ1fb89sahXZydkoJw4Q8FBzlclaPiVMaJFHmFUnpEh3j73GjcWQ3ztz9tg rVjJAIm+TmpC/DJ0W8QPnO8K1rIqmCoV5V9sVlLZnMFxP6EAMDqLB4ikGJapzHqg6h14 P6zA== X-Gm-Message-State: AOAM533M4m1fHO/J+BcsCKkAKl0B5JaXs8hW06eFlDDKQotDLw8P5InR ArX8FDIQEilQQkqtKCwaggZPksICwfE= X-Google-Smtp-Source: ABdhPJynahJkqFt5MHGBVC0PzthP4x/1xEfwbGX7Wy0iyDYy6rvF8qw+W9lUwtVW7NrWhqwQq8rFRA== X-Received: by 2002:aa7:cb8a:: with SMTP id r10mr7773117edt.237.1629256632601; Tue, 17 Aug 2021 20:17:12 -0700 (PDT) Received: from ?IPv6:2a0b:e7c0:0:107::70f? ([2a0b:e7c0:0:107::70f]) by smtp.gmail.com with ESMTPSA id v13sm1424956ejx.24.2021.08.17.20.17.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 17 Aug 2021 20:17:11 -0700 (PDT) Subject: Re: [PATCH v7 1/2] tty: hvc: pass DMA capable memory to put_chars() To: Xianting Tian , gregkh@linuxfoundation.org, amit@kernel.org, arnd@arndb.de, osandov@fb.com Cc: linuxppc-dev@lists.ozlabs.org, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org References: <20210817132300.165014-1-xianting.tian@linux.alibaba.com> <20210817132300.165014-2-xianting.tian@linux.alibaba.com> From: Jiri Slaby Message-ID: <5b728c71-a754-b3ef-4ad3-6e33db1b7647@kernel.org> Date: Wed, 18 Aug 2021 05:17:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210817132300.165014-2-xianting.tian@linux.alibaba.com> Content-Type: text/plain; charset=iso-8859-2; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 17. 08. 21, 15:22, Xianting Tian wrote: > As well known, hvc backend can register its opertions to hvc backend. > the opertions contain put_chars(), get_chars() and so on. "operations". And there too: > Some hvc backend may do dma in its opertions. eg, put_chars() of > virtio-console. But in the code of hvc framework, it may pass DMA > incapable memory to put_chars() under a specific configuration, which > is explained in commit c4baad5029(virtio-console: avoid DMA from stack): > 1, c[] is on stack, > hvc_console_print(): > char c[N_OUTBUF] __ALIGNED__; > cons_ops[index]->put_chars(vtermnos[index], c, i); > 2, ch is on stack, > static void hvc_poll_put_char(,,char ch) > { > struct tty_struct *tty = driver->ttys[0]; > struct hvc_struct *hp = tty->driver_data; > int n; > > do { > n = hp->ops->put_chars(hp->vtermno, &ch, 1); > } while (n <= 0); > } > > Commit c4baad5029 is just the fix to avoid DMA from stack memory, which > is passed to virtio-console by hvc framework in above code. But I think > the fix is aggressive, it directly uses kmemdup() to alloc new buffer > from kmalloc area and do memcpy no matter the memory is in kmalloc area > or not. But most importantly, it should better be fixed in the hvc > framework, by changing it to never pass stack memory to the put_chars() > function in the first place. Otherwise, we still face the same issue if > a new hvc backend using dma added in the furture. > > We make 'char c[N_OUTBUF]' part of 'struct hvc_struct', so hp->c is no > longer the stack memory. we can use it in above two cases. In fact, you need only a single char for the poll case (hvc_poll_put_char), so hvc_struct needs to contain only c, not an array. OTOH, you need c[N_OUTBUF] in the console case (hvc_console_print), but not whole hvc_struct. So cons_hvcs should be an array of structs composed of only the lock and the buffer. Hum. Or maybe rethink and take care of the console case by kmemdup and be done with that case? What problem do you have with allocating 16 bytes? It should be quite easy and really fast (lockless) in most cases. On the contrary, your solution has to take a spinlock to access the global buffer. > Other fix is use L1_CACHE_BYTES as the alignment, use 'sizeof(long)' as > dma alignment is wrong. And use struct_size() to calculate size of > hvc_struct. This ought to be in separate patches. thanks, -- js suse labs