Ccing I2C/Virtio maintainers.
On 09-06-26, 09:43, Gavin Li wrote:
> Additionally, force usage of a bounce buffer even if the i2c_msg buf is
> DMA-safe, since the buffer passed to the virtqueue must remain valid
> even if the transfer is interrupted. Remove usage of
> i2c_get_dma_safe_msg_buf() since it may pass through msg->buf directly.
> This results in an extra allocation+copy when I2C_M_DMA_SAFE is used.
Please always create separate patches for such changes. It would be better to
not fix multiple issues in the same patch. That would also allow for a clean
revert of the change, just in case.
> @@ -82,9 +116,16 @@ static int virtio_i2c_prepare_reqs(struct virtqueue *vq,
> sgs[outcnt++] = &out_hdr;
>
> if (msgs[i].len) {
> - reqs[i].buf = i2c_get_dma_safe_msg_buf(&msgs[i], 1);
> + /*
> + * Even if msg->flags has I2C_M_DMA_SAFE set, a bounce
> + * buffer is required because the transfer may be
> + * interrupted, after which msg->buf is no longer valid.
> + */
> + reqs[i].buf = kzalloc(msgs[i].len, GFP_KERNEL);
> if (!reqs[i].buf)
> break;
> + if (!(msgs[i].flags & I2C_M_RD))
> + memcpy(reqs[i].buf, msgs[i].buf, msgs[i].len);
>
> sg_init_one(&msg_buf, reqs[i].buf, msgs[i].len);
I don't think this is acceptable, bringing the performance down because the
remote device may misbehave.
Maybe its time for I2C and Virtio maintainers to provide some feedback here.
--
viresh
