Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-09 Thread Jason Wang


On 2020/9/3 下午1:34, Jie Deng wrote:

--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -6,6 +6,9 @@
  # ACPI drivers
  obj-$(CONFIG_I2C_SCMI)+= i2c-scmi.o
  
+# VIRTIO I2C host controller driver

+obj-$(CONFIG_I2C_VIRTIO)   += i2c-virtio.o
+
  # PC SMBus host controller drivers
  obj-$(CONFIG_I2C_ALI1535) += i2c-ali1535.o
  obj-$(CONFIG_I2C_ALI1563) += i2c-ali1563.o
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
new file mode 100644
index 000..47f9fd1
--- /dev/null
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtio I2C Bus Driver
+ *
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define VIRTIO_I2C_MSG_OK  0
+#define VIRTIO_I2C_MSG_ERR 1
+
+/**
+ * struct virtio_i2c_hdr - the virtio I2C message header structure
+ * @addr: i2c_msg addr, the slave address
+ * @flags: i2c_msg flags
+ * @len: i2c_msg len
+ */
+struct virtio_i2c_hdr {
+   __virtio16 addr;
+   __virtio16 flags;
+   __virtio16 len;
+} __packed;



Btw, this part should belong to uAPI, and you need to define the status 
in uAPI.


Thanks

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-09 Thread Jason Wang


On 2020/9/8 上午9:40, Jie Deng wrote:



On 2020/9/7 13:40, Jason Wang wrote:









+struct virtio_i2c_msg {
+    struct virtio_i2c_hdr hdr;
+    char *buf;
+    u8 status;



Any reason for separating status out of virtio_i2c_hdr?

The status is not from i2c_msg. 



You meant ic2_hdr? You embed status in virtio_i2c_msg anyway.



The "i2c_msg" structure defined in i2c.h.


So I put it out of virtio_i2c_hdr.



Something like status or response is pretty common in virtio request 
(e.g net or scsi), if no special reason, it's better to keep it in 
the hdr.



Mainly based on IN or OUT.

The addr, flags and len are from "i2c_msg". They are put in one 
structure as an OUT**scatterlist.

The buf can be an OUT**or an IN scatterlist depending on write or read.
The status is a result from the backend  which is defined as an IN 
scatterlis. 



Ok. I get this.

Thanks


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-07 Thread Jie Deng


On 2020/9/7 13:40, Jason Wang wrote:









+struct virtio_i2c_msg {
+    struct virtio_i2c_hdr hdr;
+    char *buf;
+    u8 status;



Any reason for separating status out of virtio_i2c_hdr?

The status is not from i2c_msg. 



You meant ic2_hdr? You embed status in virtio_i2c_msg anyway.



The "i2c_msg" structure defined in i2c.h.


So I put it out of virtio_i2c_hdr.



Something like status or response is pretty common in virtio request 
(e.g net or scsi), if no special reason, it's better to keep it in the 
hdr.



Mainly based on IN or OUT.

The addr, flags and len are from "i2c_msg". They are put in one 
structure as an OUT**scatterlist.

The buf can be an OUT**or an IN scatterlist depending on write or read.
The status is a result from the backend  which is defined as an IN 
scatterlis.


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-06 Thread Jason Wang


On 2020/9/4 下午9:21, Jie Deng wrote:


On 2020/9/4 12:06, Jason Wang wrote:



diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0..70c8e30 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -21,6 +21,17 @@ config I2C_ALI1535
    This driver can also be built as a module.  If so, the module
    will be called i2c-ali1535.
  +config I2C_VIRTIO
+    tristate "Virtio I2C Adapter"
+    depends on VIRTIO



I guess it should depend on some I2C module here.


The dependency of I2C is included in the Kconfig in its parent directory.
So there is nothing special to add here.



Ok.









+struct virtio_i2c_msg {
+    struct virtio_i2c_hdr hdr;
+    char *buf;
+    u8 status;



Any reason for separating status out of virtio_i2c_hdr?

The status is not from i2c_msg. 



You meant ic2_hdr? You embed status in virtio_i2c_msg anyway.



So I put it out of virtio_i2c_hdr.



Something like status or response is pretty common in virtio request 
(e.g net or scsi), if no special reason, it's better to keep it in the hdr.








+};
+
+/**
+ * struct virtio_i2c - virtio I2C data
+ * @vdev: virtio device for this controller
+ * @completion: completion of virtio I2C message
+ * @adap: I2C adapter for this controller
+ * @i2c_lock: lock for virtqueue processing
+ * @vq: the virtio virtqueue for communication
+ */
+struct virtio_i2c {
+    struct virtio_device *vdev;
+    struct completion completion;
+    struct i2c_adapter adap;
+    struct mutex i2c_lock;
+    struct virtqueue *vq;
+};
+
+static void virtio_i2c_msg_done(struct virtqueue *vq)
+{
+    struct virtio_i2c *vi = vq->vdev->priv;
+
+    complete(>completion);
+}
+
+static int virtio_i2c_add_msg(struct virtqueue *vq,
+  struct virtio_i2c_msg *vmsg,
+  struct i2c_msg *msg)
+{
+    struct scatterlist *sgs[3], hdr, bout, bin, status;
+    int outcnt = 0, incnt = 0;
+
+    if (!msg->len)
+    return -EINVAL;
+
+    vmsg->hdr.addr = msg->addr;
+    vmsg->hdr.flags = msg->flags;
+    vmsg->hdr.len = msg->len;



Missing endian conversion?


You are right. Need conversion here.



+
+    vmsg->buf = kzalloc(vmsg->hdr.len, GFP_KERNEL);
+    if (!vmsg->buf)
+    return -ENOMEM;
+
+    sg_init_one(, >hdr, sizeof(struct virtio_i2c_hdr));
+    sgs[outcnt++] = 
+    if (vmsg->hdr.flags & I2C_M_RD) {
+    sg_init_one(, vmsg->buf, msg->len);
+    sgs[outcnt + incnt++] = 
+    } else {
+    memcpy(vmsg->buf, msg->buf, msg->len);
+    sg_init_one(, vmsg->buf, msg->len);
+    sgs[outcnt++] = 
+    }
+    sg_init_one(, >status, sizeof(vmsg->status));
+    sgs[outcnt + incnt++] = 
+
+    return virtqueue_add_sgs(vq, sgs, outcnt, incnt, vmsg, 
GFP_KERNEL);

+}
+
+static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
*msgs, int num)

+{
+    struct virtio_i2c *vi = i2c_get_adapdata(adap);
+    struct virtio_i2c_msg *vmsg_o, *vmsg_i;
+    struct virtqueue *vq = vi->vq;
+    unsigned long time_left;
+    int len, i, ret = 0;
+
+    vmsg_o = kzalloc(sizeof(*vmsg_o), GFP_KERNEL);
+    if (!vmsg_o)
+    return -ENOMEM;



It looks to me we can avoid the allocation by embedding 
virtio_i2c_msg into struct virtio_i2c;



Yeah... That's better. Thanks.





+
+    mutex_lock(>i2c_lock);
+    vmsg_o->buf = NULL;
+    for (i = 0; i < num; i++) {
+    ret = virtio_i2c_add_msg(vq, vmsg_o, [i]);
+    if (ret) {
+    dev_err(>dev, "failed to add msg[%d] to 
virtqueue.\n", i);

+    goto err_unlock_free;
+    }
+
+    virtqueue_kick(vq);
+
+    time_left = wait_for_completion_timeout(>completion, 
adap->timeout);

+    if (!time_left) {
+    dev_err(>dev, "msg[%d]: addr=0x%x timeout.\n", i, 
msgs[i].addr);

+    ret = i;
+    goto err_unlock_free;
+    }
+
+    vmsg_i = (struct virtio_i2c_msg *)virtqueue_get_buf(vq, );
+    if (vmsg_i) {
+    /* vmsg_i should point to the same address with vmsg_o */
+    if (vmsg_i != vmsg_o) {
+    dev_err(>dev, "msg[%d]: addr=0x%x virtqueue 
error.\n",

+    i, vmsg_i->hdr.addr);
+    ret = i;
+    goto err_unlock_free;
+    }



Does this imply in order completion of i2c device?  (E.g what happens 
if multiple virtio i2c requests are submitted)


Btw, this always use a single descriptor once a time which makes me 
suspect if a virtqueue(virtio) is really needed. It looks to me we 
can utilize the virtqueue by submit the request in a batch.


I'm afraid not all physical devices support batch. 



Yes but I think I meant for the virtio device not the physical one. It's 
impossible to forbid batching if you have a queue anyway ...




I'd like to keep the current design and consider
your suggestion as a possible optimization in the future.

Thanks.




+}
+
+static void virtio_i2c_del_vqs(struct virtio_device *vdev)
+{
+    vdev->config->reset(vdev);



Why need reset here?


Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-04 Thread Jie Deng


On 2020/9/4 12:06, Jason Wang wrote:



diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0..70c8e30 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -21,6 +21,17 @@ config I2C_ALI1535
    This driver can also be built as a module.  If so, the module
    will be called i2c-ali1535.
  +config I2C_VIRTIO
+    tristate "Virtio I2C Adapter"
+    depends on VIRTIO



I guess it should depend on some I2C module here.


The dependency of I2C is included in the Kconfig in its parent directory.
So there is nothing special to add here.






+struct virtio_i2c_msg {
+    struct virtio_i2c_hdr hdr;
+    char *buf;
+    u8 status;



Any reason for separating status out of virtio_i2c_hdr?


The status is not from i2c_msg. So I put it out of virtio_i2c_hdr.




+};
+
+/**
+ * struct virtio_i2c - virtio I2C data
+ * @vdev: virtio device for this controller
+ * @completion: completion of virtio I2C message
+ * @adap: I2C adapter for this controller
+ * @i2c_lock: lock for virtqueue processing
+ * @vq: the virtio virtqueue for communication
+ */
+struct virtio_i2c {
+    struct virtio_device *vdev;
+    struct completion completion;
+    struct i2c_adapter adap;
+    struct mutex i2c_lock;
+    struct virtqueue *vq;
+};
+
+static void virtio_i2c_msg_done(struct virtqueue *vq)
+{
+    struct virtio_i2c *vi = vq->vdev->priv;
+
+    complete(>completion);
+}
+
+static int virtio_i2c_add_msg(struct virtqueue *vq,
+  struct virtio_i2c_msg *vmsg,
+  struct i2c_msg *msg)
+{
+    struct scatterlist *sgs[3], hdr, bout, bin, status;
+    int outcnt = 0, incnt = 0;
+
+    if (!msg->len)
+    return -EINVAL;
+
+    vmsg->hdr.addr = msg->addr;
+    vmsg->hdr.flags = msg->flags;
+    vmsg->hdr.len = msg->len;



Missing endian conversion?


You are right. Need conversion here.



+
+    vmsg->buf = kzalloc(vmsg->hdr.len, GFP_KERNEL);
+    if (!vmsg->buf)
+    return -ENOMEM;
+
+    sg_init_one(, >hdr, sizeof(struct virtio_i2c_hdr));
+    sgs[outcnt++] = 
+    if (vmsg->hdr.flags & I2C_M_RD) {
+    sg_init_one(, vmsg->buf, msg->len);
+    sgs[outcnt + incnt++] = 
+    } else {
+    memcpy(vmsg->buf, msg->buf, msg->len);
+    sg_init_one(, vmsg->buf, msg->len);
+    sgs[outcnt++] = 
+    }
+    sg_init_one(, >status, sizeof(vmsg->status));
+    sgs[outcnt + incnt++] = 
+
+    return virtqueue_add_sgs(vq, sgs, outcnt, incnt, vmsg, GFP_KERNEL);
+}
+
+static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
*msgs, int num)

+{
+    struct virtio_i2c *vi = i2c_get_adapdata(adap);
+    struct virtio_i2c_msg *vmsg_o, *vmsg_i;
+    struct virtqueue *vq = vi->vq;
+    unsigned long time_left;
+    int len, i, ret = 0;
+
+    vmsg_o = kzalloc(sizeof(*vmsg_o), GFP_KERNEL);
+    if (!vmsg_o)
+    return -ENOMEM;



It looks to me we can avoid the allocation by embedding virtio_i2c_msg 
into struct virtio_i2c;



Yeah... That's better. Thanks.





+
+    mutex_lock(>i2c_lock);
+    vmsg_o->buf = NULL;
+    for (i = 0; i < num; i++) {
+    ret = virtio_i2c_add_msg(vq, vmsg_o, [i]);
+    if (ret) {
+    dev_err(>dev, "failed to add msg[%d] to 
virtqueue.\n", i);

+    goto err_unlock_free;
+    }
+
+    virtqueue_kick(vq);
+
+    time_left = wait_for_completion_timeout(>completion, 
adap->timeout);

+    if (!time_left) {
+    dev_err(>dev, "msg[%d]: addr=0x%x timeout.\n", i, 
msgs[i].addr);

+    ret = i;
+    goto err_unlock_free;
+    }
+
+    vmsg_i = (struct virtio_i2c_msg *)virtqueue_get_buf(vq, );
+    if (vmsg_i) {
+    /* vmsg_i should point to the same address with vmsg_o */
+    if (vmsg_i != vmsg_o) {
+    dev_err(>dev, "msg[%d]: addr=0x%x virtqueue 
error.\n",

+    i, vmsg_i->hdr.addr);
+    ret = i;
+    goto err_unlock_free;
+    }



Does this imply in order completion of i2c device?  (E.g what happens 
if multiple virtio i2c requests are submitted)


Btw, this always use a single descriptor once a time which makes me 
suspect if a virtqueue(virtio) is really needed. It looks to me we can 
utilize the virtqueue by submit the request in a batch.


I'm afraid not all physical devices support batch. I'd like to keep the 
current design and consider

your suggestion as a possible optimization in the future.

Thanks.




+}
+
+static void virtio_i2c_del_vqs(struct virtio_device *vdev)
+{
+    vdev->config->reset(vdev);



Why need reset here?

Thanks


I'm following what other virtio drivers do.
They reset the devices before they clean up the queues.





+    vdev->config->del_vqs(vdev);
+}
+
+static int virtio_i2c_setup_vqs(struct virtio_i2c *vi)
+{
+    struct virtio_device *vdev = vi->vdev;
+
+    vi->vq = virtio_find_single_vq(vdev, virtio_i2c_msg_done, 
"i2c-msg");



We've in the scope of ic2, so "msg" should be sufficient.



OK. Will 

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-04 Thread Jie Deng



On 2020/9/3 18:20, Andy Shevchenko wrote:

On Thu, Sep 03, 2020 at 01:34:45PM +0800, Jie Deng wrote:

Add an I2C bus driver for virtio para-virtualization.

The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the i2c msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.

The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.

Seems it's slightly different version to what I have reviewed internally.
My comments below. (I admit that some of them maybe new)

...


Yeah... Some new devices have been added into the virtio spec during 
these days.




+/**
+ * struct virtio_i2c_hdr - the virtio I2C message header structure
+ * @addr: i2c_msg addr, the slave address
+ * @flags: i2c_msg flags
+ * @len: i2c_msg len
+ */
+struct virtio_i2c_hdr {
+   __virtio16 addr;
+   __virtio16 flags;
+   __virtio16 len;
+} __packed;

As Misha noticed and somewhere I saw 0-day reports these should be carefully
taken care of.

...


Sure. Will fix these.



+static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int 
num)
+{
+   struct virtio_i2c *vi = i2c_get_adapdata(adap);
+   struct virtio_i2c_msg *vmsg_o, *vmsg_i;
+   struct virtqueue *vq = vi->vq;
+   unsigned long time_left;
+   int len, i, ret = 0;
+
+   vmsg_o = kzalloc(sizeof(*vmsg_o), GFP_KERNEL);
+   if (!vmsg_o)
+   return -ENOMEM;
+
+   mutex_lock(>i2c_lock);
+   vmsg_o->buf = NULL;
+   for (i = 0; i < num; i++) {
+   ret = virtio_i2c_add_msg(vq, vmsg_o, [i]);
+   if (ret) {
+   dev_err(>dev, "failed to add msg[%d] to 
virtqueue.\n", i);
+   goto err_unlock_free;

break;


OK.



+   }
+
+   virtqueue_kick(vq);
+
+   time_left = wait_for_completion_timeout(>completion, 
adap->timeout);
+   if (!time_left) {
+   dev_err(>dev, "msg[%d]: addr=0x%x timeout.\n", i, 
msgs[i].addr);
+   ret = i;
+   goto err_unlock_free;

break;

And so on.

OK.

+   }
+
+   vmsg_i = (struct virtio_i2c_msg *)virtqueue_get_buf(vq, );
+   if (vmsg_i) {
+   /* vmsg_i should point to the same address with vmsg_o 
*/
+   if (vmsg_i != vmsg_o) {
+   dev_err(>dev, "msg[%d]: addr=0x%x virtqueue 
error.\n",
+   i, vmsg_i->hdr.addr);
+   ret = i;
+   goto err_unlock_free;
+   }
+   if (vmsg_i->status != VIRTIO_I2C_MSG_OK) {
+   dev_err(>dev, "msg[%d]: addr=0x%x 
error=%d.\n",
+   i, vmsg_i->hdr.addr, vmsg_i->status);
+   ret = i;
+   goto err_unlock_free;
+   }
+   if ((vmsg_i->hdr.flags & I2C_M_RD) && vmsg_i->hdr.len)
+   memcpy(msgs[i].buf, vmsg_i->buf, 
vmsg_i->hdr.len);
+
+   kfree(vmsg_i->buf);
+   vmsg_i->buf = NULL;
+   }
+   reinit_completion(>completion);
+   }
+   if (i == num)
+   ret = num;

And this conditional seems a dup of the for-loop successfully iterating over
entire queue.

You are right.
We may save several lines of code by using "Return (ret<0) ? ret : i" at 
the end.




+err_unlock_free:

Redundant.

OK.

+   mutex_unlock(>i2c_lock);
+   kfree(vmsg_o->buf);
+   kfree(vmsg_o);
+   return ret;
+}

...


+   vi->adap.timeout = HZ / 10;

+ Blank line.

OK.

+   ret = i2c_add_adapter(>adap);
+   if (ret) {
+   dev_err(>dev, "failed to add virtio-i2c adapter.\n");
+   virtio_i2c_del_vqs(vdev);

Usually we do clean up followed by message.

I will change the order. Thank you.

+   }
+
+   return ret;



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Jie Deng



On 2020/9/3 17:58, Michael S. Tsirkin wrote:

On Thu, Sep 03, 2020 at 01:34:45PM +0800, Jie Deng wrote:

Add an I2C bus driver for virtio para-virtualization.

The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the i2c msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.

The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.

Please reserve the ID with the virtio tc so no one conflicts.


Sure. I will send a patch to request the ID.




+
+/**
+ * struct virtio_i2c_hdr - the virtio I2C message header structure
+ * @addr: i2c_msg addr, the slave address
+ * @flags: i2c_msg flags
+ * @len: i2c_msg len
+ */
+struct virtio_i2c_hdr {
+   __virtio16 addr;
+   __virtio16 flags;
+   __virtio16 len;
+} __packed;
virtio16 is for legacy devices, modern ones should be __le.
and  we don't really need to pack it I think.


Right. I will fix these. Thanks.




___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Jason Wang


On 2020/9/3 下午1:34, Jie Deng wrote:

Add an I2C bus driver for virtio para-virtualization.

The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the i2c msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.

The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.

Co-developed-by: Conghui Chen 
Signed-off-by: Conghui Chen 
Signed-off-by: Jie Deng 
Reviewed-by: Shuo Liu 
Reviewed-by: Andy Shevchenko 
---
  drivers/i2c/busses/Kconfig  |  11 ++
  drivers/i2c/busses/Makefile |   3 +
  drivers/i2c/busses/i2c-virtio.c | 276 
  include/uapi/linux/virtio_ids.h |   1 +
  4 files changed, 291 insertions(+)
  create mode 100644 drivers/i2c/busses/i2c-virtio.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0..70c8e30 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -21,6 +21,17 @@ config I2C_ALI1535
  This driver can also be built as a module.  If so, the module
  will be called i2c-ali1535.
  
+config I2C_VIRTIO

+   tristate "Virtio I2C Adapter"
+   depends on VIRTIO



I guess it should depend on some I2C module here.



+   help
+ If you say yes to this option, support will be included for the virtio
+ i2c adapter driver. The hardware can be emulated by any device model
+ software according to the virtio protocol.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-virtio.
+
  config I2C_ALI1563
tristate "ALI 1563"
depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 19aff0e..821acfa 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -6,6 +6,9 @@
  # ACPI drivers
  obj-$(CONFIG_I2C_SCMI)+= i2c-scmi.o
  
+# VIRTIO I2C host controller driver

+obj-$(CONFIG_I2C_VIRTIO)   += i2c-virtio.o
+
  # PC SMBus host controller drivers
  obj-$(CONFIG_I2C_ALI1535) += i2c-ali1535.o
  obj-$(CONFIG_I2C_ALI1563) += i2c-ali1563.o
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
new file mode 100644
index 000..47f9fd1
--- /dev/null
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtio I2C Bus Driver
+ *
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define VIRTIO_I2C_MSG_OK  0
+#define VIRTIO_I2C_MSG_ERR 1
+
+/**
+ * struct virtio_i2c_hdr - the virtio I2C message header structure
+ * @addr: i2c_msg addr, the slave address
+ * @flags: i2c_msg flags
+ * @len: i2c_msg len
+ */
+struct virtio_i2c_hdr {
+   __virtio16 addr;
+   __virtio16 flags;
+   __virtio16 len;
+} __packed;
+
+/**
+ * struct virtio_i2c_msg - the virtio I2C message structure
+ * @hdr: the virtio I2C message header
+ * @buf: virtio I2C message data buffer
+ * @status: the processing result from the backend
+ */
+struct virtio_i2c_msg {
+   struct virtio_i2c_hdr hdr;
+   char *buf;
+   u8 status;



Any reason for separating status out of virtio_i2c_hdr?



+};
+
+/**
+ * struct virtio_i2c - virtio I2C data
+ * @vdev: virtio device for this controller
+ * @completion: completion of virtio I2C message
+ * @adap: I2C adapter for this controller
+ * @i2c_lock: lock for virtqueue processing
+ * @vq: the virtio virtqueue for communication
+ */
+struct virtio_i2c {
+   struct virtio_device *vdev;
+   struct completion completion;
+   struct i2c_adapter adap;
+   struct mutex i2c_lock;
+   struct virtqueue *vq;
+};
+
+static void virtio_i2c_msg_done(struct virtqueue *vq)
+{
+   struct virtio_i2c *vi = vq->vdev->priv;
+
+   complete(>completion);
+}
+
+static int virtio_i2c_add_msg(struct virtqueue *vq,
+ struct virtio_i2c_msg *vmsg,
+ struct i2c_msg *msg)
+{
+   struct scatterlist *sgs[3], hdr, bout, bin, status;
+   int outcnt = 0, incnt = 0;
+
+   if (!msg->len)
+   return -EINVAL;
+
+   vmsg->hdr.addr = msg->addr;
+   vmsg->hdr.flags = msg->flags;
+   vmsg->hdr.len = msg->len;



Missing endian conversion?



+
+   vmsg->buf = kzalloc(vmsg->hdr.len, GFP_KERNEL);
+   if 

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Jason Wang


On 2020/9/3 下午3:19, Jie Deng wrote:


On 2020/9/3 14:12, Jason Wang wrote:


On 2020/9/3 下午1:34, Jie Deng wrote:

Add an I2C bus driver for virtio para-virtualization.

The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the i2c msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.



May I know the reason why don't you use i2c or virtio directly?


We don't want to add virtio drivers for every I2C devices in the guests.
This bus driver is designed to provide a way to flexibly expose the 
physical
I2C slave devices to the guest without adding or changing the drivers 
of the

I2C slave devices in the guest OS.



Ok, if I understand this correctly, this is virtio transport of i2c 
message (similar to virtio-scsi).










The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.



Is there a link to the spec patch?

Thanks


I haven't submitted the patch to reserve the ID in spec yet.
I write the ID here because I want to see your opinions first.



It would be helpful to send a spec draft for early review.

Thanks




Thanks




___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Andy Shevchenko
On Thu, Sep 03, 2020 at 01:34:45PM +0800, Jie Deng wrote:
> Add an I2C bus driver for virtio para-virtualization.
> 
> The controller can be emulated by the backend driver in
> any device model software by following the virtio protocol.
> 
> This driver communicates with the backend driver through a
> virtio I2C message structure which includes following parts:
> 
> - Header: i2c_msg addr, flags, len.
> - Data buffer: the pointer to the i2c msg data.
> - Status: the processing result from the backend.
> 
> People may implement different backend drivers to emulate
> different controllers according to their needs. A backend
> example can be found in the device model of the open source
> project ACRN. For more information, please refer to
> https://projectacrn.org.
> 
> The virtio device ID 34 is used for this I2C adpter since IDs
> before 34 have been reserved by other virtio devices.

Seems it's slightly different version to what I have reviewed internally.
My comments below. (I admit that some of them maybe new)

...

> +/**
> + * struct virtio_i2c_hdr - the virtio I2C message header structure
> + * @addr: i2c_msg addr, the slave address
> + * @flags: i2c_msg flags
> + * @len: i2c_msg len
> + */
> +struct virtio_i2c_hdr {
> + __virtio16 addr;
> + __virtio16 flags;
> + __virtio16 len;
> +} __packed;

As Misha noticed and somewhere I saw 0-day reports these should be carefully
taken care of.

...

> +static int virtio_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, 
> int num)
> +{
> + struct virtio_i2c *vi = i2c_get_adapdata(adap);
> + struct virtio_i2c_msg *vmsg_o, *vmsg_i;
> + struct virtqueue *vq = vi->vq;
> + unsigned long time_left;
> + int len, i, ret = 0;
> +
> + vmsg_o = kzalloc(sizeof(*vmsg_o), GFP_KERNEL);
> + if (!vmsg_o)
> + return -ENOMEM;
> +
> + mutex_lock(>i2c_lock);
> + vmsg_o->buf = NULL;
> + for (i = 0; i < num; i++) {
> + ret = virtio_i2c_add_msg(vq, vmsg_o, [i]);
> + if (ret) {
> + dev_err(>dev, "failed to add msg[%d] to 
> virtqueue.\n", i);

> + goto err_unlock_free;

break;

> + }
> +
> + virtqueue_kick(vq);
> +
> + time_left = wait_for_completion_timeout(>completion, 
> adap->timeout);
> + if (!time_left) {
> + dev_err(>dev, "msg[%d]: addr=0x%x timeout.\n", i, 
> msgs[i].addr);
> + ret = i;

> + goto err_unlock_free;

break;

And so on.

> + }
> +
> + vmsg_i = (struct virtio_i2c_msg *)virtqueue_get_buf(vq, );
> + if (vmsg_i) {
> + /* vmsg_i should point to the same address with vmsg_o 
> */
> + if (vmsg_i != vmsg_o) {
> + dev_err(>dev, "msg[%d]: addr=0x%x 
> virtqueue error.\n",
> + i, vmsg_i->hdr.addr);
> + ret = i;
> + goto err_unlock_free;
> + }
> + if (vmsg_i->status != VIRTIO_I2C_MSG_OK) {
> + dev_err(>dev, "msg[%d]: addr=0x%x 
> error=%d.\n",
> + i, vmsg_i->hdr.addr, vmsg_i->status);
> + ret = i;
> + goto err_unlock_free;
> + }
> + if ((vmsg_i->hdr.flags & I2C_M_RD) && vmsg_i->hdr.len)
> + memcpy(msgs[i].buf, vmsg_i->buf, 
> vmsg_i->hdr.len);
> +
> + kfree(vmsg_i->buf);
> + vmsg_i->buf = NULL;
> + }
> + reinit_completion(>completion);
> + }

> + if (i == num)
> + ret = num;

And this conditional seems a dup of the for-loop successfully iterating over
entire queue.

> +err_unlock_free:

Redundant.

> + mutex_unlock(>i2c_lock);
> + kfree(vmsg_o->buf);
> + kfree(vmsg_o);
> + return ret;
> +}

...

> + vi->adap.timeout = HZ / 10;

+ Blank line.

> + ret = i2c_add_adapter(>adap);
> + if (ret) {

> + dev_err(>dev, "failed to add virtio-i2c adapter.\n");
> + virtio_i2c_del_vqs(vdev);

Usually we do clean up followed by message.

> + }
> +
> + return ret;


-- 
With Best Regards,
Andy Shevchenko


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Michael S. Tsirkin
On Thu, Sep 03, 2020 at 01:34:45PM +0800, Jie Deng wrote:
> Add an I2C bus driver for virtio para-virtualization.
> 
> The controller can be emulated by the backend driver in
> any device model software by following the virtio protocol.
> 
> This driver communicates with the backend driver through a
> virtio I2C message structure which includes following parts:
> 
> - Header: i2c_msg addr, flags, len.
> - Data buffer: the pointer to the i2c msg data.
> - Status: the processing result from the backend.
> 
> People may implement different backend drivers to emulate
> different controllers according to their needs. A backend
> example can be found in the device model of the open source
> project ACRN. For more information, please refer to
> https://projectacrn.org.
> 
> The virtio device ID 34 is used for this I2C adpter since IDs
> before 34 have been reserved by other virtio devices.

Please reserve the ID with the virtio tc so no one conflicts.


> Co-developed-by: Conghui Chen 
> Signed-off-by: Conghui Chen 
> Signed-off-by: Jie Deng 
> Reviewed-by: Shuo Liu 
> Reviewed-by: Andy Shevchenko 
> ---
>  drivers/i2c/busses/Kconfig  |  11 ++
>  drivers/i2c/busses/Makefile |   3 +
>  drivers/i2c/busses/i2c-virtio.c | 276 
> 
>  include/uapi/linux/virtio_ids.h |   1 +
>  4 files changed, 291 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-virtio.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 293e7a0..70c8e30 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -21,6 +21,17 @@ config I2C_ALI1535
> This driver can also be built as a module.  If so, the module
> will be called i2c-ali1535.
>  
> +config I2C_VIRTIO
> + tristate "Virtio I2C Adapter"
> + depends on VIRTIO
> + help
> +   If you say yes to this option, support will be included for the virtio
> +   i2c adapter driver. The hardware can be emulated by any device model
> +   software according to the virtio protocol.
> +
> +   This driver can also be built as a module. If so, the module
> +   will be called i2c-virtio.
> +
>  config I2C_ALI1563
>   tristate "ALI 1563"
>   depends on PCI
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 19aff0e..821acfa 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -6,6 +6,9 @@
>  # ACPI drivers
>  obj-$(CONFIG_I2C_SCMI)   += i2c-scmi.o
>  
> +# VIRTIO I2C host controller driver
> +obj-$(CONFIG_I2C_VIRTIO) += i2c-virtio.o
> +
>  # PC SMBus host controller drivers
>  obj-$(CONFIG_I2C_ALI1535)+= i2c-ali1535.o
>  obj-$(CONFIG_I2C_ALI1563)+= i2c-ali1563.o
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> new file mode 100644
> index 000..47f9fd1
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-virtio.c
> @@ -0,0 +1,276 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Virtio I2C Bus Driver
> + *
> + * Copyright (c) 2020 Intel Corporation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define VIRTIO_I2C_MSG_OK0
> +#define VIRTIO_I2C_MSG_ERR   1
> +
> +/**
> + * struct virtio_i2c_hdr - the virtio I2C message header structure
> + * @addr: i2c_msg addr, the slave address
> + * @flags: i2c_msg flags
> + * @len: i2c_msg len
> + */
> +struct virtio_i2c_hdr {
> + __virtio16 addr;
> + __virtio16 flags;
> + __virtio16 len;
> +} __packed;

virtio16 is for legacy devices, modern ones should be __le.
and  we don't really need to pack it I think.

> +
> +/**
> + * struct virtio_i2c_msg - the virtio I2C message structure
> + * @hdr: the virtio I2C message header
> + * @buf: virtio I2C message data buffer
> + * @status: the processing result from the backend
> + */
> +struct virtio_i2c_msg {
> + struct virtio_i2c_hdr hdr;
> + char *buf;
> + u8 status;
> +};
> +
> +/**
> + * struct virtio_i2c - virtio I2C data
> + * @vdev: virtio device for this controller
> + * @completion: completion of virtio I2C message
> + * @adap: I2C adapter for this controller
> + * @i2c_lock: lock for virtqueue processing
> + * @vq: the virtio virtqueue for communication
> + */
> +struct virtio_i2c {
> + struct virtio_device *vdev;
> + struct completion completion;
> + struct i2c_adapter adap;
> + struct mutex i2c_lock;
> + struct virtqueue *vq;
> +};
> +
> +static void virtio_i2c_msg_done(struct virtqueue *vq)
> +{
> + struct virtio_i2c *vi = vq->vdev->priv;
> +
> + complete(>completion);
> +}
> +
> +static int virtio_i2c_add_msg(struct virtqueue *vq,
> +   struct virtio_i2c_msg *vmsg,
> +   struct i2c_msg *msg)
> +{
> + struct scatterlist *sgs[3], hdr, bout, bin, status;
> + int 

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Jie Deng


On 2020/9/3 14:12, Jason Wang wrote:


On 2020/9/3 下午1:34, Jie Deng wrote:

Add an I2C bus driver for virtio para-virtualization.

The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the i2c msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.



May I know the reason why don't you use i2c or virtio directly?


We don't want to add virtio drivers for every I2C devices in the guests.
This bus driver is designed to provide a way to flexibly expose the 
physical
I2C slave devices to the guest without adding or changing the drivers of 
the

I2C slave devices in the guest OS.






The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.



Is there a link to the spec patch?

Thanks


I haven't submitted the patch to reserve the ID in spec yet.
I write the ID here because I want to see your opinions first.

Thanks


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Jason Wang


On 2020/9/3 下午1:34, Jie Deng wrote:

Add an I2C bus driver for virtio para-virtualization.

The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the i2c msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.



May I know the reason why don't you use i2c or virtio directly?




The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.



Is there a link to the spec patch?

Thanks




Co-developed-by: Conghui Chen 
Signed-off-by: Conghui Chen 
Signed-off-by: Jie Deng 
Reviewed-by: Shuo Liu 
Reviewed-by: Andy Shevchenko 
---
  drivers/i2c/busses/Kconfig  |  11 ++
  drivers/i2c/busses/Makefile |   3 +
  drivers/i2c/busses/i2c-virtio.c | 276 
  include/uapi/linux/virtio_ids.h |   1 +
  4 files changed, 291 insertions(+)
  create mode 100644 drivers/i2c/busses/i2c-virtio.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0..70c8e30 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -21,6 +21,17 @@ config I2C_ALI1535
  This driver can also be built as a module.  If so, the module
  will be called i2c-ali1535.
  
+config I2C_VIRTIO

+   tristate "Virtio I2C Adapter"
+   depends on VIRTIO
+   help
+ If you say yes to this option, support will be included for the virtio
+ i2c adapter driver. The hardware can be emulated by any device model
+ software according to the virtio protocol.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-virtio.
+
  config I2C_ALI1563
tristate "ALI 1563"
depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 19aff0e..821acfa 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -6,6 +6,9 @@
  # ACPI drivers
  obj-$(CONFIG_I2C_SCMI)+= i2c-scmi.o
  
+# VIRTIO I2C host controller driver

+obj-$(CONFIG_I2C_VIRTIO)   += i2c-virtio.o
+
  # PC SMBus host controller drivers
  obj-$(CONFIG_I2C_ALI1535) += i2c-ali1535.o
  obj-$(CONFIG_I2C_ALI1563) += i2c-ali1563.o
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
new file mode 100644
index 000..47f9fd1
--- /dev/null
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtio I2C Bus Driver
+ *
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define VIRTIO_I2C_MSG_OK  0
+#define VIRTIO_I2C_MSG_ERR 1
+
+/**
+ * struct virtio_i2c_hdr - the virtio I2C message header structure
+ * @addr: i2c_msg addr, the slave address
+ * @flags: i2c_msg flags
+ * @len: i2c_msg len
+ */
+struct virtio_i2c_hdr {
+   __virtio16 addr;
+   __virtio16 flags;
+   __virtio16 len;
+} __packed;
+
+/**
+ * struct virtio_i2c_msg - the virtio I2C message structure
+ * @hdr: the virtio I2C message header
+ * @buf: virtio I2C message data buffer
+ * @status: the processing result from the backend
+ */
+struct virtio_i2c_msg {
+   struct virtio_i2c_hdr hdr;
+   char *buf;
+   u8 status;
+};
+
+/**
+ * struct virtio_i2c - virtio I2C data
+ * @vdev: virtio device for this controller
+ * @completion: completion of virtio I2C message
+ * @adap: I2C adapter for this controller
+ * @i2c_lock: lock for virtqueue processing
+ * @vq: the virtio virtqueue for communication
+ */
+struct virtio_i2c {
+   struct virtio_device *vdev;
+   struct completion completion;
+   struct i2c_adapter adap;
+   struct mutex i2c_lock;
+   struct virtqueue *vq;
+};
+
+static void virtio_i2c_msg_done(struct virtqueue *vq)
+{
+   struct virtio_i2c *vi = vq->vdev->priv;
+
+   complete(>completion);
+}
+
+static int virtio_i2c_add_msg(struct virtqueue *vq,
+ struct virtio_i2c_msg *vmsg,
+ struct i2c_msg *msg)
+{
+   struct scatterlist *sgs[3], hdr, bout, bin, status;
+   int outcnt = 0, incnt = 0;
+
+   if (!msg->len)
+   return -EINVAL;
+
+   vmsg->hdr.addr = msg->addr;
+   vmsg->hdr.flags = msg->flags;
+   vmsg->hdr.len = msg->len;
+
+   vmsg->buf = kzalloc(vmsg->hdr.len, GFP_KERNEL);
+   if (!vmsg->buf)
+   return 

[PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-02 Thread Jie Deng
Add an I2C bus driver for virtio para-virtualization.

The controller can be emulated by the backend driver in
any device model software by following the virtio protocol.

This driver communicates with the backend driver through a
virtio I2C message structure which includes following parts:

- Header: i2c_msg addr, flags, len.
- Data buffer: the pointer to the i2c msg data.
- Status: the processing result from the backend.

People may implement different backend drivers to emulate
different controllers according to their needs. A backend
example can be found in the device model of the open source
project ACRN. For more information, please refer to
https://projectacrn.org.

The virtio device ID 34 is used for this I2C adpter since IDs
before 34 have been reserved by other virtio devices.

Co-developed-by: Conghui Chen 
Signed-off-by: Conghui Chen 
Signed-off-by: Jie Deng 
Reviewed-by: Shuo Liu 
Reviewed-by: Andy Shevchenko 
---
 drivers/i2c/busses/Kconfig  |  11 ++
 drivers/i2c/busses/Makefile |   3 +
 drivers/i2c/busses/i2c-virtio.c | 276 
 include/uapi/linux/virtio_ids.h |   1 +
 4 files changed, 291 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-virtio.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0..70c8e30 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -21,6 +21,17 @@ config I2C_ALI1535
  This driver can also be built as a module.  If so, the module
  will be called i2c-ali1535.
 
+config I2C_VIRTIO
+   tristate "Virtio I2C Adapter"
+   depends on VIRTIO
+   help
+ If you say yes to this option, support will be included for the virtio
+ i2c adapter driver. The hardware can be emulated by any device model
+ software according to the virtio protocol.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-virtio.
+
 config I2C_ALI1563
tristate "ALI 1563"
depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 19aff0e..821acfa 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -6,6 +6,9 @@
 # ACPI drivers
 obj-$(CONFIG_I2C_SCMI) += i2c-scmi.o
 
+# VIRTIO I2C host controller driver
+obj-$(CONFIG_I2C_VIRTIO)   += i2c-virtio.o
+
 # PC SMBus host controller drivers
 obj-$(CONFIG_I2C_ALI1535)  += i2c-ali1535.o
 obj-$(CONFIG_I2C_ALI1563)  += i2c-ali1563.o
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
new file mode 100644
index 000..47f9fd1
--- /dev/null
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtio I2C Bus Driver
+ *
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define VIRTIO_I2C_MSG_OK  0
+#define VIRTIO_I2C_MSG_ERR 1
+
+/**
+ * struct virtio_i2c_hdr - the virtio I2C message header structure
+ * @addr: i2c_msg addr, the slave address
+ * @flags: i2c_msg flags
+ * @len: i2c_msg len
+ */
+struct virtio_i2c_hdr {
+   __virtio16 addr;
+   __virtio16 flags;
+   __virtio16 len;
+} __packed;
+
+/**
+ * struct virtio_i2c_msg - the virtio I2C message structure
+ * @hdr: the virtio I2C message header
+ * @buf: virtio I2C message data buffer
+ * @status: the processing result from the backend
+ */
+struct virtio_i2c_msg {
+   struct virtio_i2c_hdr hdr;
+   char *buf;
+   u8 status;
+};
+
+/**
+ * struct virtio_i2c - virtio I2C data
+ * @vdev: virtio device for this controller
+ * @completion: completion of virtio I2C message
+ * @adap: I2C adapter for this controller
+ * @i2c_lock: lock for virtqueue processing
+ * @vq: the virtio virtqueue for communication
+ */
+struct virtio_i2c {
+   struct virtio_device *vdev;
+   struct completion completion;
+   struct i2c_adapter adap;
+   struct mutex i2c_lock;
+   struct virtqueue *vq;
+};
+
+static void virtio_i2c_msg_done(struct virtqueue *vq)
+{
+   struct virtio_i2c *vi = vq->vdev->priv;
+
+   complete(>completion);
+}
+
+static int virtio_i2c_add_msg(struct virtqueue *vq,
+ struct virtio_i2c_msg *vmsg,
+ struct i2c_msg *msg)
+{
+   struct scatterlist *sgs[3], hdr, bout, bin, status;
+   int outcnt = 0, incnt = 0;
+
+   if (!msg->len)
+   return -EINVAL;
+
+   vmsg->hdr.addr = msg->addr;
+   vmsg->hdr.flags = msg->flags;
+   vmsg->hdr.len = msg->len;
+
+   vmsg->buf = kzalloc(vmsg->hdr.len, GFP_KERNEL);
+   if (!vmsg->buf)
+   return -ENOMEM;
+
+   sg_init_one(, >hdr, sizeof(struct virtio_i2c_hdr));
+   sgs[outcnt++] = 
+   if (vmsg->hdr.flags & I2C_M_RD) {
+   sg_init_one(, vmsg->buf,