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. > > Regards.Right, stack is virtually mapped. Thanks
Arnd Bergmann
2021-Mar-10 08:27 UTC
[PATCH v6] i2c: virtio: add a virtio i2c frontend driver
On Wed, Mar 10, 2021 at 4:59 AM Jason Wang <jasowang at 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