Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <9a2086f37c0a62069b67c39a3f75941b78a0039c.1614749417.git.jie.deng@intel.com>
 <43b0842b-8b0f-1979-ed07-d6124e3a6b79@redhat.com> <db9350b3-b847-8f54-546f-9a0bdec425d4@intel.com>
 <a3d4ce55-db25-5f96-ff44-5b76edfe9e08@redhat.com>
In-Reply-To: <a3d4ce55-db25-5f96-ff44-5b76edfe9e08@redhat.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Wed, 10 Mar 2021 09:27:39 +0100
Message-ID: <CAK8P3a04u9Xv15K06zrsOtzZF18+1wDpJbBHUU3wYwU--Hdz5A@mail.gmail.com>
Subject: Re: [PATCH v6] i2c: virtio: add a virtio i2c frontend driver
To:     Jason Wang <jasowang@redhat.com>
Cc:     Jie Deng <jie.deng@intel.com>,
        Linux I2C <linux-i2c@vger.kernel.org>,
        virtualization@lists.linux-foundation.org,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Wolfram Sang <wsa@kernel.org>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        conghui.chen@intel.com, kblaiech@mellanox.com,
        jarkko.nikula@linux.intel.com,
        Sergey Semin <Sergey.Semin@baikalelectronics.ru>,
        Mike Rapoport <rppt@kernel.org>, loic.poulain@linaro.org,
        Tali Perry <tali.perry1@gmail.com>,
        =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        yu1.wang@intel.com, shuo.a.liu@intel.com,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Stefan Hajnoczi <stefanha@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Wed, Mar 10, 2021 at 4:59 AM Jason Wang <jasowang@redhat.com> wrote:
> On 2021/3/10 10:22 上午, Jie Deng wrote:
> > On 2021/3/4 17:15, Jason Wang wrote:
> >>
> >>
> >>> +        }
> >>> +
> >>> +        if (msgs[i].flags & I2C_M_RD)
> >>> +            memcpy(msgs[i].buf, req->buf, msgs[i].len);
> >>
> >>
> >> Sorry if I had asked this before but any rason not to use msg[i].buf
> >> directly?
> >>
> >>
> > The msg[i].buf is passed by the I2C core. I just noticed that these
> > bufs are not
> > always allocated by kmalloc. They may come from the stack, which may
> > cause
> > the check "sg_init_one -> sg_set_buf -> virt_addr_valid"  to fail.
> > Therefore the
> > msg[i].buf is not suitable for direct use here.
>
> Right, stack is virtually mapped.

Maybe there is (or should be) a way to let the i2c core code handle
the bounce buffering in this case. This is surely not a problem that
is unique to this driver, and I'm sure it has come up many times in
the past.

I see that there is a i2c_get_dma_safe_msg_buf() helper for this
purpose, but it has to be called by the driver rather than the core,
so the driver still needs to keep track of each address when it
sends multiple i2c_msg at once, but maybe it can all be done
inside the sg_table instead of yet another structure.

At least this one avoids copying data that is marked with the
I2C_M_DMA_SAFE flag.

       Arnd