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

2020-09-02 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(&vi->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 

Re: [PATCH v6 2/4] rpmsg: move common structures and defines to headers

2020-09-02 Thread Guennadi Liakhovetski
Hi Mathieu,

On Wed, Sep 02, 2020 at 11:24:37AM -0600, Mathieu Poirier wrote:
> On Wed, Sep 02, 2020 at 07:35:27AM +0200, Guennadi Liakhovetski wrote:
> > Hi Mathew,
> > 
> > On Tue, Sep 01, 2020 at 11:23:21AM -0600, Mathieu Poirier wrote:
> > > On Tue, Sep 01, 2020 at 05:11:51PM +0200, Guennadi Liakhovetski wrote:
> > > > virtio_rpmsg_bus.c keeps RPMsg protocol structure declarations and
> > > > common defines like the ones, needed for name-space announcements,
> > > > internal. Move them to common headers instead.
> > > > 
> > > > Signed-off-by: Guennadi Liakhovetski 
> > > > 
> > > 
> > > I already reviewed this patch and added my RB to it.  Please carry it in 
> > > your
> > > next revision.
> > 
> > But only as long as the patch doesn't change, right? And in fact it did 
> > change 
> > this time - I renamed the header, moving it under include/linux/rpmsg, 
> > actually 
> 
> Patch 2/4 in V5 was identical to what was submitted in V4 and my RB tag was 
> not
> added, nor was the entry for virtio_rpmsg.h added to the MAINTAINERS file.

Right, yes, I forgot about that your request when creating v5, sorry about 
that, 
that's why I made a v6 with the header moved to include/linux/rpmsg/.

> > also following your suggestion. Still, formally the patch has changed, so, 
> > I 
> > didn't include your "Reviewed-by" in this version. And you anyway would be 
> 
> Reviewed-by: Mathieu Poirier 
> 
> > reviewing the other patches in this series to, right?
> 
> As much as I'd like to reviewing the other patches in this series won't be
> possible at this time. 

Ok, understand, I'm wondering then what the path to upstreaming would be then?

Thanks
Guennadi

> > > Thanks,
> > > Mathieu
> > > 
> > > > ---
> > > >  drivers/rpmsg/virtio_rpmsg_bus.c | 78 +-
> > > >  include/linux/rpmsg/virtio.h | 83 
> > > >  include/uapi/linux/rpmsg.h   |  3 ++
> > > >  3 files changed, 88 insertions(+), 76 deletions(-)
> > > >  create mode 100644 include/linux/rpmsg/virtio.h
> > > > 
> > > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c 
> > > > b/drivers/rpmsg/virtio_rpmsg_bus.c
> > > > index 9006fc7f73d0..f39c426f9c5e 100644
> > > > --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> > > > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> > > > @@ -19,6 +19,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > @@ -27,6 +28,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  
> > > >  #include "rpmsg_internal.h"
> > > >  
> > > > @@ -70,58 +72,6 @@ struct virtproc_info {
> > > > struct rpmsg_endpoint *ns_ept;
> > > >  };
> > > >  
> > > > -/* The feature bitmap for virtio rpmsg */
> > > > -#define VIRTIO_RPMSG_F_NS  0 /* RP supports name service 
> > > > notifications */
> > > > -
> > > > -/**
> > > > - * struct rpmsg_hdr - common header for all rpmsg messages
> > > > - * @src: source address
> > > > - * @dst: destination address
> > > > - * @reserved: reserved for future use
> > > > - * @len: length of payload (in bytes)
> > > > - * @flags: message flags
> > > > - * @data: @len bytes of message payload data
> > > > - *
> > > > - * Every message sent(/received) on the rpmsg bus begins with this 
> > > > header.
> > > > - */
> > > > -struct rpmsg_hdr {
> > > > -   __virtio32 src;
> > > > -   __virtio32 dst;
> > > > -   __virtio32 reserved;
> > > > -   __virtio16 len;
> > > > -   __virtio16 flags;
> > > > -   u8 data[];
> > > > -} __packed;
> > > > -
> > > > -/**
> > > > - * struct rpmsg_ns_msg - dynamic name service announcement message
> > > > - * @name: name of remote service that is published
> > > > - * @addr: address of remote service that is published
> > > > - * @flags: indicates whether service is created or destroyed
> > > > - *
> > > > - * This message is sent across to publish a new service, or announce
> > > > - * about its removal. When we receive these messages, an appropriate
> > > > - * rpmsg channel (i.e device) is created/destroyed. In turn, the 
> > > > ->probe()
> > > > - * or ->remove() handler of the appropriate rpmsg driver will be 
> > > > invoked
> > > > - * (if/as-soon-as one is registered).
> > > > - */
> > > > -struct rpmsg_ns_msg {
> > > > -   char name[RPMSG_NAME_SIZE];
> > > > -   __virtio32 addr;
> > > > -   __virtio32 flags;
> > > > -} __packed;
> > > > -
> > > > -/**
> > > > - * enum rpmsg_ns_flags - dynamic name service announcement flags
> > > > - *
> > > > - * @RPMSG_NS_CREATE: a new remote service was just created
> > > > - * @RPMSG_NS_DESTROY: a known remote service was just destroyed
> > > > - */
> > > > -enum rpmsg_ns_flags {
> > > > -   RPMSG_NS_CREATE = 0,
> > > > -   RPMSG_NS_DESTROY= 1,
> > > > -};
> > > > -
> > > >  /**
> > > >   * @vrp: the remote processor this channel belongs to
> > > >   */
> > > > @@ -134,27 +84,6 @@ struct virtio_rpmsg_cha

[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(&vi->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, &vmsg->hdr, sizeof(struct virtio_i2c_hdr));
+   sgs[outcnt++] = &hdr;
+   if (vmsg->hdr.flags & I2C_M_RD) {
+   sg_init_on

Re: remove revalidate_disk()

2020-09-02 Thread Jens Axboe
On 9/1/20 9:57 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series removes the revalidate_disk() function, which has been a
> really odd duck in the last years.  The prime reason why most people
> use it is because it propagates a size change from the gendisk to
> the block_device structure.  But it also calls into the rather ill
> defined ->revalidate_disk method which is rather useless for the
> callers.  So this adds a new helper to just propagate the size, and
> cleans up all kinds of mess around this area.  Follow on patches
> will eventuall kill of ->revalidate_disk entirely, but ther are a lot
> more patches needed for that.

Applied, thanks.

-- 
Jens Axboe

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


Re: [PATCH] char: virtio: Select VIRTIO from VIRTIO_CONSOLE.

2020-09-02 Thread Amit Shah
On Mon, Aug 31, 2020 at 06:58:50PM +0200, Michal Suchanek wrote:
> Make it possible to have virtio console built-in when
> other virtio drivers are modular.
> 
> Signed-off-by: Michal Suchanek 

Reviewed-by: Amit Shah 

> ---
>  drivers/char/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index 3a144c000a38..9bd9917ca9af 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -93,8 +93,9 @@ config PPDEV
>  
>  config VIRTIO_CONSOLE
>   tristate "Virtio console"
> - depends on VIRTIO && TTY
> + depends on TTY
>   select HVC_DRIVER
> + select VIRTIO
>   help
> Virtio console for use with hypervisors.
>  
> -- 
> 2.28.0
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v1 4/5] xen/balloon: try to merge system ram resources

2020-09-02 Thread David Hildenbrand


> Am 02.09.2020 um 12:15 schrieb Jürgen Groß :
> 
> On 21.08.20 12:34, David Hildenbrand wrote:
>> Let's reuse the new mechanism to merge system ram resources below the
>> root. We are the only one hotplugging system ram (e.g., DIMMs don't apply),
>> so this is safe to be used.
>> Cc: Andrew Morton 
>> Cc: Michal Hocko 
>> Cc: Boris Ostrovsky 
>> Cc: Juergen Gross 
>> Cc: Stefano Stabellini 
>> Cc: Roger Pau Monné 
>> Cc: Julien Grall 
>> Cc: Pankaj Gupta 
>> Cc: Baoquan He 
>> Cc: Wei Yang 
>> Signed-off-by: David Hildenbrand 
>> ---
>>  drivers/xen/balloon.c | 4 
>>  1 file changed, 4 insertions(+)
>> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
>> index 37ffccda8bb87..5ec73f752b8a7 100644
>> --- a/drivers/xen/balloon.c
>> +++ b/drivers/xen/balloon.c
>> @@ -338,6 +338,10 @@ static enum bp_state reserve_additional_memory(void)
>>  if (rc) {
>>  pr_warn("Cannot add additional memory (%i)\n", rc);
>>  goto err;
>> +} else {
>> +resource = NULL;
>> +/* Try to reduce the number of system ram resources. */
>> +merge_system_ram_resources(&iomem_resource);
>>  }
> 
> I don't see the need for setting resource to NULL and to use an "else"
> clause here.
> 

I set it to NULL because the pointer may be stale after that call - to avoid 
future bugs. But I can drop it.

Ack to the „else“ case.

Thanks for having a look!

> 
> Juergen
> 

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

Re: [PATCH v1 4/5] xen/balloon: try to merge system ram resources

2020-09-02 Thread Jürgen Groß

On 21.08.20 12:34, David Hildenbrand wrote:

Let's reuse the new mechanism to merge system ram resources below the
root. We are the only one hotplugging system ram (e.g., DIMMs don't apply),
so this is safe to be used.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Cc: Roger Pau Monné 
Cc: Julien Grall 
Cc: Pankaj Gupta 
Cc: Baoquan He 
Cc: Wei Yang 
Signed-off-by: David Hildenbrand 
---
  drivers/xen/balloon.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 37ffccda8bb87..5ec73f752b8a7 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -338,6 +338,10 @@ static enum bp_state reserve_additional_memory(void)
if (rc) {
pr_warn("Cannot add additional memory (%i)\n", rc);
goto err;
+   } else {
+   resource = NULL;
+   /* Try to reduce the number of system ram resources. */
+   merge_system_ram_resources(&iomem_resource);
}


I don't see the need for setting resource to NULL and to use an "else"
clause here.


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