* [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend
@ 2021-03-24 7:33 Viresh Kumar
2021-03-24 7:33 ` [PATCH 1/5] hw/virtio: add boilerplate for vhost-user-i2c device Viresh Kumar
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-03-24 7:33 UTC (permalink / raw)
To: qemu-devel
Cc: Vincent Guittot, Jie Deng, Viresh Kumar, Bill Mills,
Arnd Bergmann, Mike Holmes, Alex Bennée, stratos-dev
Hello,
This is an initial implementation of a generic vhost-user backend for
the I2C bus. This is based of the virtio specifications (already merged)
for the I2C bus.
The kernel virtio I2C driver is still under review, here is the latest
version (v10):
https://lore.kernel.org/lkml/226a8d5663b7bb6f5d06ede7701eedb18d1bafa1.1616493817.git.jie.deng@intel.com/
The backend is implemented as a vhost-user device because we want to
experiment in making portable backends that can be used with multiple
hypervisors. We also want to support backends isolated in their own
separate service VMs with limited memory cross-sections with the
principle guest. This is part of a wider initiative by Linaro called
"project Stratos" for which you can find information here:
https://collaborate.linaro.org/display/STR/Stratos+Home
I mentioned this to explain the decision to write the daemon as a fairly
pure glib application that just depends on libvhost-user.
We are not sure where the vhost-user backend should get queued, qemu or
a separate repository. Similar questions were raised by an earlier
thread by Alex Bennée for his RPMB work:
https://lore.kernel.org/qemu-devel/20200925125147.26943-1-alex.bennee@linaro.org/
Testing:
-------
I didn't have access to a real hardware where I can play with a I2C
client device (like RTC, eeprom, etc) to verify the working of the
backend daemon, so I decided to test it on my x86 box itself with
hierarchy of two ARM64 guests.
The first ARM64 guest was passed "-device ds1338,address=0x20" option,
so it could emulate a ds1338 RTC device, which connects to an I2C bus.
Once the guest came up, ds1338 device instance was created within the
guest kernel by doing:
echo ds1338 0x20 > /sys/bus/i2c/devices/i2c-0/new_device
[
Note that this may end up binding the ds1338 device to its driver,
which won't let our i2c daemon talk to the device. For that we need to
manually unbind the device from the driver:
echo 0-0020 > /sys/bus/i2c/devices/0-0020/driver/unbind
]
After this is done, you will get /dev/rtc1. This is the device we wanted
to emulate, which will be accessed by the vhost-user-i2c backend daemon
via the /dev/i2c-0 file present in the guest VM.
At this point we need to start the backend daemon and give it a
socket-path to talk to from qemu (you can pass -v to it to get more
detailed messages):
vhost-user-i2c --socket-path=vi2c.sock --device-list 0:20
[ Here, 0:20 is the bus/device mapping, 0 for /dev/i2c-0 and 20 is
client address of ds1338 that we used while creating the device. ]
Now we need to start the second level ARM64 guest (from within the first
guest) to get the i2c-virtio.c Linux driver up. The second level guest
is passed the following options to connect to the same socket:
-chardev socket,path=vi2c.sock,id=vi2c \
-device vhost-user-i2c-pci,chardev=vi2c,id=i2c
Once the second level guest boots up, we will see the i2c-virtio bus at
/sys/bus/i2c/devices/i2c-X/. From there we can now make it emulate the
ds1338 device again by doing:
echo ds1338 0x20 > /sys/bus/i2c/devices/i2c-0/new_device
[ This time we want ds1338's driver to be bound to the device, so it
should be enabled in the kernel as well. ]
And we will get /dev/rtc1 device again here in the second level guest.
Now we can play with the rtc device with help of hwclock utility and we
can see the following sequence of transfers happening if we try to
update rtc's time from system time.
hwclock -w -f /dev/rtc1 (in guest2) ->
Reaches i2c-virtio.c (Linux bus driver in guest2) ->
transfer over virtio ->
Reaches the qemu's vhost-i2c device emulation (running over guest1) ->
Reaches the backend daemon vhost-user-i2c started earlier (in guest1) ->
ioctl(/dev/i2c-0, I2C_RDWR, ..); (in guest1) ->
reaches qemu's hw/rtc/ds1338.c (running over host)
I hope I was able to give a clear picture of my test setup here :)
Thanks.
Viresh Kumar (5):
hw/virtio: add boilerplate for vhost-user-i2c device
hw/virtio: add vhost-user-i2c-pci boilerplate
tools/vhost-user-i2c: Add backend driver
docs: add a man page for vhost-user-i2c
MAINTAINERS: Add entry for virtio-i2c
MAINTAINERS | 9 +
docs/tools/index.rst | 1 +
docs/tools/vhost-user-i2c.rst | 75 +++
hw/virtio/Kconfig | 5 +
hw/virtio/meson.build | 2 +
hw/virtio/vhost-user-i2c-pci.c | 79 +++
hw/virtio/vhost-user-i2c.c | 286 +++++++++
include/hw/virtio/vhost-user-i2c.h | 37 ++
include/standard-headers/linux/virtio_ids.h | 1 +
tools/meson.build | 8 +
tools/vhost-user-i2c/50-qemu-i2c.json.in | 5 +
tools/vhost-user-i2c/main.c | 652 ++++++++++++++++++++
tools/vhost-user-i2c/meson.build | 10 +
13 files changed, 1170 insertions(+)
create mode 100644 docs/tools/vhost-user-i2c.rst
create mode 100644 hw/virtio/vhost-user-i2c-pci.c
create mode 100644 hw/virtio/vhost-user-i2c.c
create mode 100644 include/hw/virtio/vhost-user-i2c.h
create mode 100644 tools/vhost-user-i2c/50-qemu-i2c.json.in
create mode 100644 tools/vhost-user-i2c/main.c
create mode 100644 tools/vhost-user-i2c/meson.build
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/5] hw/virtio: add boilerplate for vhost-user-i2c device
2021-03-24 7:33 [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend Viresh Kumar
@ 2021-03-24 7:33 ` Viresh Kumar
2021-03-29 15:13 ` Alex Bennée
2021-03-24 7:33 ` [PATCH 2/5] hw/virtio: add vhost-user-i2c-pci boilerplate Viresh Kumar
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Viresh Kumar @ 2021-03-24 7:33 UTC (permalink / raw)
To: qemu-devel
Cc: Vincent Guittot, Jie Deng, Viresh Kumar, Bill Mills,
Arnd Bergmann, Mike Holmes, Alex Bennée, stratos-dev
This creates the QEMU side of the vhost-user-i2c device which connects
to the remote daemon. It is based of vhost-user-fs code.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
hw/virtio/Kconfig | 5 +
hw/virtio/meson.build | 1 +
hw/virtio/vhost-user-i2c.c | 286 ++++++++++++++++++++
include/hw/virtio/vhost-user-i2c.h | 37 +++
include/standard-headers/linux/virtio_ids.h | 1 +
5 files changed, 330 insertions(+)
create mode 100644 hw/virtio/vhost-user-i2c.c
create mode 100644 include/hw/virtio/vhost-user-i2c.h
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 0eda25c4e1bf..35ab45e2095c 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -58,3 +58,8 @@ config VIRTIO_MEM
depends on LINUX
depends on VIRTIO_MEM_SUPPORTED
select MEM_DEVICE
+
+config VHOST_USER_I2C
+ bool
+ default y
+ depends on VIRTIO && VHOST_USER
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index fbff9bc9d4de..1a0d736a0db5 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -25,6 +25,7 @@ virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.
virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
+virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
virtio_pci_ss = ss.source_set()
virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
new file mode 100644
index 000000000000..7b0dc24412a4
--- /dev/null
+++ b/hw/virtio/vhost-user-i2c.c
@@ -0,0 +1,286 @@
+/*
+ * Vhost-user i2c virtio device
+ *
+ * Copyright (c) 2021 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/vhost-user-i2c.h"
+#include "qemu/error-report.h"
+#include "standard-headers/linux/virtio_ids.h"
+
+static void vu_i2c_start(VirtIODevice *vdev)
+{
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+ int ret;
+ int i;
+
+ if (!k->set_guest_notifiers) {
+ error_report("binding does not support guest notifiers");
+ return;
+ }
+
+ ret = vhost_dev_enable_notifiers(&i2c->vhost_dev, vdev);
+ if (ret < 0) {
+ error_report("Error enabling host notifiers: %d", -ret);
+ return;
+ }
+
+ ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, true);
+ if (ret < 0) {
+ error_report("Error binding guest notifier: %d", -ret);
+ goto err_host_notifiers;
+ }
+
+ i2c->vhost_dev.acked_features = vdev->guest_features;
+ ret = vhost_dev_start(&i2c->vhost_dev, vdev);
+ if (ret < 0) {
+ error_report("Error starting vhost-user-i2c: %d", -ret);
+ goto err_guest_notifiers;
+ }
+
+ /*
+ * guest_notifier_mask/pending not used yet, so just unmask
+ * everything here. virtio-pci will do the right thing by
+ * enabling/disabling irqfd.
+ */
+ for (i = 0; i < i2c->vhost_dev.nvqs; i++) {
+ vhost_virtqueue_mask(&i2c->vhost_dev, vdev, i, false);
+ }
+
+ return;
+
+err_guest_notifiers:
+ k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
+err_host_notifiers:
+ vhost_dev_disable_notifiers(&i2c->vhost_dev, vdev);
+}
+
+static void vu_i2c_stop(VirtIODevice *vdev)
+{
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+ int ret;
+
+ if (!k->set_guest_notifiers) {
+ return;
+ }
+
+ vhost_dev_stop(&i2c->vhost_dev, vdev);
+
+ ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
+ if (ret < 0) {
+ error_report("vhost guest notifier cleanup failed: %d", ret);
+ return;
+ }
+
+ vhost_dev_disable_notifiers(&i2c->vhost_dev, vdev);
+}
+
+static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status)
+{
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+ bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
+
+ if (!vdev->vm_running) {
+ should_start = false;
+ }
+
+ if (i2c->vhost_dev.started == should_start) {
+ return;
+ }
+
+ if (should_start) {
+ vu_i2c_start(vdev);
+ } else {
+ vu_i2c_stop(vdev);
+ }
+}
+
+static uint64_t vu_i2c_get_features(VirtIODevice *vdev,
+ uint64_t requested_features, Error **errp)
+{
+ /* No feature bits used yet */
+ return requested_features;
+}
+
+static void vu_i2c_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+ /*
+ * Not normally called; it's the daemon that handles the queue;
+ * however virtio's cleanup path can call this.
+ */
+}
+
+static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
+{
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+
+ vhost_virtqueue_mask(&i2c->vhost_dev, vdev, idx, mask);
+}
+
+static bool vu_i2c_guest_notifier_pending(VirtIODevice *vdev, int idx)
+{
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+
+ return vhost_virtqueue_pending(&i2c->vhost_dev, idx);
+}
+
+static void do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserI2C *i2c)
+{
+ vhost_user_cleanup(&i2c->vhost_user);
+ virtio_delete_queue(i2c->req_vq);
+ g_free(i2c->vhost_dev.vqs);
+ virtio_cleanup(vdev);
+ g_free(i2c->vhost_dev.vqs);
+ i2c->vhost_dev.vqs = NULL;
+}
+
+static int vu_i2c_connect(DeviceState *dev)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+
+ if (i2c->connected) {
+ return 0;
+ }
+ i2c->connected = true;
+
+ /* restore vhost state */
+ if (virtio_device_started(vdev, vdev->status)) {
+ vu_i2c_start(vdev);
+ }
+
+ return 0;
+}
+
+static void vu_i2c_disconnect(DeviceState *dev)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+
+ if (!i2c->connected) {
+ return;
+ }
+ i2c->connected = false;
+
+ if (i2c->vhost_dev.started) {
+ vu_i2c_stop(vdev);
+ }
+}
+
+static void vu_i2c_event(void *opaque, QEMUChrEvent event)
+{
+ DeviceState *dev = opaque;
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+
+ switch (event) {
+ case CHR_EVENT_OPENED:
+ if (vu_i2c_connect(dev) < 0) {
+ qemu_chr_fe_disconnect(&i2c->conf.chardev);
+ return;
+ }
+ break;
+ case CHR_EVENT_CLOSED:
+ vu_i2c_disconnect(dev);
+ break;
+ case CHR_EVENT_BREAK:
+ case CHR_EVENT_MUX_IN:
+ case CHR_EVENT_MUX_OUT:
+ /* Ignore */
+ break;
+ }
+}
+
+static void vu_i2c_device_realize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VHostUserI2C *i2c = VHOST_USER_I2C(dev);
+ int ret;
+
+ if (!i2c->conf.chardev.chr) {
+ error_setg(errp, "missing chardev");
+ return;
+ }
+
+ if (!vhost_user_init(&i2c->vhost_user, &i2c->conf.chardev, errp)) {
+ return;
+ }
+
+ virtio_init(vdev, "vhost-user-i2c", VIRTIO_ID_I2C, 0);
+
+ i2c->req_vq = virtio_add_queue(vdev, 3, vu_i2c_handle_output);
+ i2c->vhost_dev.nvqs = 1;
+ i2c->vhost_dev.vqs = g_new0(struct vhost_virtqueue, i2c->vhost_dev.nvqs);
+ ret = vhost_dev_init(&i2c->vhost_dev, &i2c->vhost_user,
+ VHOST_BACKEND_TYPE_USER, 0);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "vhost_dev_init() failed");
+ do_vhost_user_cleanup(vdev, i2c);
+ }
+
+ qemu_chr_fe_set_handlers(&i2c->conf.chardev, NULL, NULL, vu_i2c_event, NULL,
+ dev, NULL, true);
+}
+
+static void vu_i2c_device_unrealize(DeviceState *dev)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VHostUserI2C *i2c = VHOST_USER_I2C(dev);
+
+ /* This will stop vhost backend if appropriate. */
+ vu_i2c_set_status(vdev, 0);
+
+ vhost_dev_cleanup(&i2c->vhost_dev);
+
+ do_vhost_user_cleanup(vdev, i2c);
+}
+
+static const VMStateDescription vu_i2c_vmstate = {
+ .name = "vhost-user-i2c",
+ .unmigratable = 1,
+};
+
+static Property vu_i2c_properties[] = {
+ DEFINE_PROP_CHR("chardev", VHostUserI2C, conf.chardev),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vu_i2c_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+ device_class_set_props(dc, vu_i2c_properties);
+ dc->vmsd = &vu_i2c_vmstate;
+ set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+ vdc->realize = vu_i2c_device_realize;
+ vdc->unrealize = vu_i2c_device_unrealize;
+ vdc->get_features = vu_i2c_get_features;
+ vdc->set_status = vu_i2c_set_status;
+ vdc->guest_notifier_mask = vu_i2c_guest_notifier_mask;
+ vdc->guest_notifier_pending = vu_i2c_guest_notifier_pending;
+}
+
+static const TypeInfo vu_i2c_info = {
+ .name = TYPE_VHOST_USER_I2C,
+ .parent = TYPE_VIRTIO_DEVICE,
+ .instance_size = sizeof(VHostUserI2C),
+ .class_init = vu_i2c_class_init,
+};
+
+static void vu_i2c_register_types(void)
+{
+ type_register_static(&vu_i2c_info);
+}
+
+type_init(vu_i2c_register_types)
diff --git a/include/hw/virtio/vhost-user-i2c.h b/include/hw/virtio/vhost-user-i2c.h
new file mode 100644
index 000000000000..a5fffcb6096c
--- /dev/null
+++ b/include/hw/virtio/vhost-user-i2c.h
@@ -0,0 +1,37 @@
+/*
+ * Vhost-user i2c virtio device
+ *
+ * Copyright (c) 2021 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef _QEMU_VHOST_USER_I2C_H
+#define _QEMU_VHOST_USER_I2C_H
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user.h"
+#include "chardev/char-fe.h"
+
+#define TYPE_VHOST_USER_I2C "vhost-user-i2c-device"
+OBJECT_DECLARE_SIMPLE_TYPE(VHostUserI2C, VHOST_USER_I2C)
+
+typedef struct {
+ CharBackend chardev;
+} VHostUserI2CConf;
+
+struct VHostUserI2C {
+ /*< private >*/
+ VirtIODevice parent;
+ VHostUserI2CConf conf;
+ struct vhost_virtqueue *vhost_vq;
+ struct vhost_dev vhost_dev;
+ VhostUserState vhost_user;
+ VirtQueue *req_vq;
+ bool connected;
+
+ /*< public >*/
+};
+
+#endif /* _QEMU_VHOST_USER_I2C_H */
diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
index bc1c0621f5ed..aba00c54b9cd 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -54,5 +54,6 @@
#define VIRTIO_ID_FS 26 /* virtio filesystem */
#define VIRTIO_ID_PMEM 27 /* virtio pmem */
#define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
+#define VIRTIO_ID_I2C 34 /* virtio i2c */
#endif /* _LINUX_VIRTIO_IDS_H */
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] hw/virtio: add vhost-user-i2c-pci boilerplate
2021-03-24 7:33 [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend Viresh Kumar
2021-03-24 7:33 ` [PATCH 1/5] hw/virtio: add boilerplate for vhost-user-i2c device Viresh Kumar
@ 2021-03-24 7:33 ` Viresh Kumar
2021-03-29 15:19 ` Alex Bennée
2021-03-24 7:33 ` [PATCH 3/5] tools/vhost-user-i2c: Add backend driver Viresh Kumar
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Viresh Kumar @ 2021-03-24 7:33 UTC (permalink / raw)
To: qemu-devel
Cc: Vincent Guittot, Jie Deng, Viresh Kumar, Bill Mills,
Arnd Bergmann, Mike Holmes, Alex Bennée, stratos-dev
This allows is to instantiate a vhost-user-i2c device as part of a PCI
bus. It is mostly boilerplate which looks pretty similar to the
vhost-user-fs-pci device.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
hw/virtio/meson.build | 1 +
hw/virtio/vhost-user-i2c-pci.c | 79 ++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
create mode 100644 hw/virtio/vhost-user-i2c-pci.c
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 1a0d736a0db5..bc352a600911 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -26,6 +26,7 @@ virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
+virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_I2C'], if_true: files('vhost-user-i2c-pci.c'))
virtio_pci_ss = ss.source_set()
virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
diff --git a/hw/virtio/vhost-user-i2c-pci.c b/hw/virtio/vhost-user-i2c-pci.c
new file mode 100644
index 000000000000..4bcfeafcb632
--- /dev/null
+++ b/hw/virtio/vhost-user-i2c-pci.c
@@ -0,0 +1,79 @@
+/*
+ * Vhost-user i2c virtio device PCI glue
+ *
+ * Copyright (c) 2021 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/vhost-user-i2c.h"
+#include "virtio-pci.h"
+
+struct VHostUserI2CPCI {
+ VirtIOPCIProxy parent_obj;
+ VHostUserI2C vdev;
+};
+
+typedef struct VHostUserI2CPCI VHostUserI2CPCI;
+
+#define TYPE_VHOST_USER_I2C_PCI "vhost-user-i2c-pci-base"
+
+DECLARE_INSTANCE_CHECKER(VHostUserI2CPCI, VHOST_USER_I2C_PCI,
+ TYPE_VHOST_USER_I2C_PCI)
+
+static Property vhost_user_i2c_pci_properties[] = {
+ DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+ DEV_NVECTORS_UNSPECIFIED),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vhost_user_i2c_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+ VHostUserI2CPCI *dev = VHOST_USER_I2C_PCI(vpci_dev);
+ DeviceState *vdev = DEVICE(&dev->vdev);
+
+ if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+ vpci_dev->nvectors = 1;
+ }
+
+ qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+}
+
+static void vhost_user_i2c_pci_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+ PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+ k->realize = vhost_user_i2c_pci_realize;
+ set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+ device_class_set_props(dc, vhost_user_i2c_pci_properties);
+ pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+ pcidev_k->device_id = 0; /* Set by virtio-pci based on virtio id */
+ pcidev_k->revision = 0x00;
+ pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER;
+}
+
+static void vhost_user_i2c_pci_instance_init(Object *obj)
+{
+ VHostUserI2CPCI *dev = VHOST_USER_I2C_PCI(obj);
+
+ virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+ TYPE_VHOST_USER_I2C);
+}
+
+static const VirtioPCIDeviceTypeInfo vhost_user_i2c_pci_info = {
+ .base_name = TYPE_VHOST_USER_I2C_PCI,
+ .non_transitional_name = "vhost-user-i2c-pci",
+ .instance_size = sizeof(VHostUserI2CPCI),
+ .instance_init = vhost_user_i2c_pci_instance_init,
+ .class_init = vhost_user_i2c_pci_class_init,
+};
+
+static void vhost_user_i2c_pci_register(void)
+{
+ virtio_pci_types_register(&vhost_user_i2c_pci_info);
+}
+
+type_init(vhost_user_i2c_pci_register);
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-24 7:33 [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend Viresh Kumar
2021-03-24 7:33 ` [PATCH 1/5] hw/virtio: add boilerplate for vhost-user-i2c device Viresh Kumar
2021-03-24 7:33 ` [PATCH 2/5] hw/virtio: add vhost-user-i2c-pci boilerplate Viresh Kumar
@ 2021-03-24 7:33 ` Viresh Kumar
2021-03-25 5:09 ` Jie Deng
` (2 more replies)
2021-03-24 7:33 ` [PATCH 4/5] docs: add a man page for vhost-user-i2c Viresh Kumar
` (2 subsequent siblings)
5 siblings, 3 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-03-24 7:33 UTC (permalink / raw)
To: qemu-devel
Cc: Vincent Guittot, Jie Deng, Viresh Kumar, Bill Mills,
Arnd Bergmann, Mike Holmes, Alex Bennée, stratos-dev
This adds the vhost-user backend driver to support virtio based I2C
devices.
vhost-user-i2c --help
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
hw/virtio/vhost-user-i2c.c | 2 +-
tools/meson.build | 8 +
tools/vhost-user-i2c/50-qemu-i2c.json.in | 5 +
tools/vhost-user-i2c/main.c | 652 +++++++++++++++++++++++
tools/vhost-user-i2c/meson.build | 10 +
5 files changed, 676 insertions(+), 1 deletion(-)
create mode 100644 tools/vhost-user-i2c/50-qemu-i2c.json.in
create mode 100644 tools/vhost-user-i2c/main.c
create mode 100644 tools/vhost-user-i2c/meson.build
diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
index 7b0dc24412a4..c67c18ca00fc 100644
--- a/hw/virtio/vhost-user-i2c.c
+++ b/hw/virtio/vhost-user-i2c.c
@@ -218,7 +218,7 @@ static void vu_i2c_device_realize(DeviceState *dev, Error **errp)
virtio_init(vdev, "vhost-user-i2c", VIRTIO_ID_I2C, 0);
- i2c->req_vq = virtio_add_queue(vdev, 3, vu_i2c_handle_output);
+ i2c->req_vq = virtio_add_queue(vdev, 4, vu_i2c_handle_output);
i2c->vhost_dev.nvqs = 1;
i2c->vhost_dev.vqs = g_new0(struct vhost_virtqueue, i2c->vhost_dev.nvqs);
ret = vhost_dev_init(&i2c->vhost_dev, &i2c->vhost_user,
diff --git a/tools/meson.build b/tools/meson.build
index 3e5a0abfa29f..8271e110978b 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -24,3 +24,11 @@ endif
if have_virtiofsd
subdir('virtiofsd')
endif
+
+have_virtioi2c= (have_system and
+ have_tools and
+ 'CONFIG_LINUX' in config_host)
+
+if have_virtioi2c
+ subdir('vhost-user-i2c')
+endif
diff --git a/tools/vhost-user-i2c/50-qemu-i2c.json.in b/tools/vhost-user-i2c/50-qemu-i2c.json.in
new file mode 100644
index 000000000000..dafd1337fa9c
--- /dev/null
+++ b/tools/vhost-user-i2c/50-qemu-i2c.json.in
@@ -0,0 +1,5 @@
+{
+ "description": "QEMU vhost-user-i2c",
+ "type": "bridge",
+ "binary": "@libexecdir@/vhost-user-i2c"
+}
diff --git a/tools/vhost-user-i2c/main.c b/tools/vhost-user-i2c/main.c
new file mode 100644
index 000000000000..20942aeb189a
--- /dev/null
+++ b/tools/vhost-user-i2c/main.c
@@ -0,0 +1,652 @@
+/*
+ * VIRTIO I2C Emulation via vhost-user
+ *
+ * Copyright (c) 2021 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#define G_LOG_DOMAIN "vhost-user-i2c"
+#define G_LOG_USE_STRUCTURED 1
+
+#include <glib.h>
+#include <gio/gio.h>
+#include <gio/gunixsocketaddress.h>
+#include <glib-unix.h>
+#include <glib/gstdio.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <string.h>
+#include <inttypes.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <endian.h>
+#include <assert.h>
+#include <linux/i2c.h>
+#include <linux/i2c-dev.h>
+
+#include "qemu/cutils.h"
+#include "subprojects/libvhost-user/libvhost-user-glib.h"
+#include "subprojects/libvhost-user/libvhost-user.h"
+
+/* Definitions from virtio-i2c specifications */
+#define VHOST_USER_I2C_MAX_QUEUES 1
+
+/* Status */
+#define VIRTIO_I2C_MSG_OK 0
+#define VIRTIO_I2C_MSG_ERR 1
+
+/* The bit 0 of the @virtio_i2c_out_hdr.@flags, used to group the requests */
+#define VIRTIO_I2C_FLAGS_FAIL_NEXT 0x00000001
+
+/**
+ * struct virtio_i2c_out_hdr - the virtio I2C message OUT header
+ * @addr: the controlled device's address
+ * @padding: used to pad to full dword
+ * @flags: used for feature extensibility
+ */
+struct virtio_i2c_out_hdr {
+ uint16_t addr;
+ uint16_t padding;
+ uint32_t flags;
+} __attribute__((packed));
+
+/**
+ * struct virtio_i2c_in_hdr - the virtio I2C message IN header
+ * @status: the processing result from the backend
+ */
+struct virtio_i2c_in_hdr {
+ uint8_t status;
+} __attribute__((packed));
+
+/* vhost-user-i2c definitions */
+
+#ifndef container_of
+#define container_of(ptr, type, member) ({ \
+ const typeof(((type *) 0)->member) *__mptr = (ptr); \
+ (type *) ((char *) __mptr - offsetof(type, member));})
+#endif
+
+#define MAX_I2C_VDEV (1 << 7)
+#define MAX_I2C_ADAPTER 16
+
+typedef struct {
+ int32_t fd;
+ int32_t bus;
+ bool clients[MAX_I2C_VDEV];
+} VI2cAdapter;
+
+typedef struct {
+ VugDev dev;
+ GMainLoop *loop;
+ VI2cAdapter *adapter[MAX_I2C_ADAPTER];
+ uint16_t adapter_map[MAX_I2C_VDEV];
+ uint32_t adapter_num;
+} VuI2c;
+
+static gboolean print_cap, verbose;
+static gchar *socket_path, *device_list;
+static gint socket_fd = -1;
+
+static GOptionEntry options[] = {
+ { "socket-path", 's', 0, G_OPTION_ARG_FILENAME, &socket_path, "Location of vhost-user Unix domain socket, incompatible with --fd", "PATH" },
+ { "fd", 'f', 0, G_OPTION_ARG_INT, &socket_fd, "Specify the file-descriptor of the backend, incompatible with --socket-path", "FD" },
+ { "device-list", 'l', 0, G_OPTION_ARG_STRING, &device_list, "List of i2c-dev bus and attached devices", "I2C Devices" },
+ { "print-capabilities", 'c', 0, G_OPTION_ARG_NONE, &print_cap, "Output to stdout the backend capabilities in JSON format and exit", NULL},
+ { "verbose", 'v', 0, G_OPTION_ARG_NONE, &verbose, "Be more verbose in output", NULL},
+ { NULL }
+};
+
+
+/* I2c helpers */
+static void fmt_bytes(GString *s, uint8_t *bytes, int len)
+{
+ int32_t i;
+ for (i = 0; i < len; i++) {
+ if (i && i % 16 == 0) {
+ g_string_append_c(s, '\n');
+ }
+ g_string_append_printf(s, "%x ", bytes[i]);
+ }
+}
+
+static void vi2c_dump_msg(struct i2c_msg *msg)
+{
+ g_autoptr(GString) s = g_string_new("\nI2c request: ");
+
+ g_string_append_printf(s, "addr: %x\n", msg->addr);
+ g_string_append_printf(s, "transfer len: %x\n", msg->len);
+
+ g_string_append_printf(s, "%s: ", msg->flags & I2C_M_RD ? "Data read" :
+ "Data Written");
+ fmt_bytes(s, (uint8_t *)msg->buf, msg->len);
+ g_string_append_printf(s, "\n");
+
+ g_debug("%s: %s", __func__, s->str);
+}
+
+static int vi2c_map_adapters(VuI2c *i2c)
+{
+ VI2cAdapter *adapter;
+ int32_t i, client_addr;
+
+ /*
+ * Flatten the map for client address and adapter to the array:
+ *
+ * adapter_map[MAX_I2C_VDEV]:
+ *
+ * Adapter | adapter2 | none | adapter1 | adapter3 | none | none| (val)
+ * |----------|-------|----------|----------|------|-----|
+ * Slave Address | addr 1 | none | addr 2 | addr 3 | none | none| (idx)
+ * |<-----------------------MAX_I2C_VDEV---------------->|
+ */
+ for (i = 0; i < i2c->adapter_num; i++) {
+ adapter = i2c->adapter[i];
+
+ for (client_addr = 0; client_addr < MAX_I2C_VDEV; client_addr++) {
+ if (adapter->clients[client_addr]) {
+ if (i2c->adapter_map[client_addr]) {
+ g_printerr("client addr %x repeated, not supported!\n",
+ client_addr);
+ return -1;
+ }
+
+ /* The array is initialized to 0, + 1 for index */
+ i2c->adapter_map[client_addr] = i + 1;
+ if (verbose) {
+ g_print("client: 0x%x -> i2c adapter: %d\n", client_addr,
+ adapter->bus);
+ }
+ }
+ }
+ }
+ return 0;
+}
+
+static VI2cAdapter *vi2c_find_adapter(VuI2c *i2c, uint16_t addr)
+{
+ int32_t idx;
+
+ if (addr < MAX_I2C_VDEV && ((idx = i2c->adapter_map[addr]) != 0)) {
+ return i2c->adapter[idx - 1];
+ }
+
+ return NULL;
+}
+
+static bool vi2c_client_access_ok(VI2cAdapter *adapter, uint16_t addr)
+{
+ if (ioctl(adapter->fd, I2C_SLAVE, addr) < 0) {
+ if (errno == EBUSY) {
+ g_printerr("client device %x is busy!\n", addr);
+ } else {
+ g_printerr("client device %d does not exist!\n", addr);
+ }
+ return false;
+ }
+ return true;
+}
+
+static void vi2c_remove_adapters(VuI2c *i2c)
+{
+ VI2cAdapter *adapter;
+ int32_t i;
+
+ for (i = 0; i < i2c->adapter_num; i++) {
+ adapter = i2c->adapter[i];
+ if (!adapter) {
+ break;
+ }
+
+ if (adapter->fd > 0) {
+ close(adapter->fd);
+ }
+
+ g_free(adapter);
+ i2c->adapter[i] = NULL;
+ }
+}
+
+static VI2cAdapter *vi2c_create_adapter(int32_t bus, uint16_t client_addr[],
+ int32_t n_client)
+{
+ VI2cAdapter *adapter;
+ char path[20];
+ int32_t fd, i;
+
+ if (bus < 0)
+ return NULL;
+
+ adapter = g_malloc0(sizeof(*adapter));
+ if (!adapter) {
+ g_printerr("failed to alloc adapter");
+ return NULL;
+ }
+
+ snprintf(path, sizeof(path), "/dev/i2c-%d", bus);
+ path[sizeof(path) - 1] = '\0';
+
+ fd = open(path, O_RDWR);
+ if (fd < 0) {
+ g_printerr("virtio_i2c: failed to open %s\n", path);
+ goto fail;
+ }
+
+ adapter->fd = fd;
+ adapter->bus = bus;
+
+ for (i = 0; i < n_client; i++) {
+ if (client_addr[i]) {
+ if (!vi2c_client_access_ok(adapter, client_addr[i])) {
+ goto fail;
+ }
+
+ if (adapter->clients[client_addr[i]]) {
+ g_printerr("client addr 0x%x repeat, not allowed.\n", client_addr[i]);
+ goto fail;
+ }
+
+ adapter->clients[client_addr[i]] = true;
+ if (verbose) {
+ g_print("Added client 0x%x to bus %u\n", client_addr[i], bus);
+ }
+ }
+ }
+ return adapter;
+
+fail:
+ g_free(adapter);
+ return NULL;
+}
+
+/*
+ * Virtio I2C device list format.
+ *
+ * <bus>:<client_addr>[:<client_addr>],
+ * [<bus>:<client_addr>[:<client_addr>]]
+ *
+ * bus (dec): adatper bus number.
+ * e.g. 2 for /dev/i2c-2
+ * client_addr (hex): address for client device
+ * e.g. 0x1C or 1C
+ *
+ * Example: --device-list="2:0x1c:0x20,3:0x10:0x2c"
+ *
+ * Note: client address can not repeat.
+ */
+static int vi2c_parse(VuI2c *i2c)
+{
+ uint16_t client_addr[MAX_I2C_VDEV];
+ int32_t n_adapter = 0, n_client;
+ int64_t addr, bus;
+ const char *cp, *t;
+
+ while (device_list) {
+ /* Read <bus>:<client_addr>[:<client_addr>] entries one by one */
+ cp = strsep(&device_list, ",");
+
+ if (!cp || *cp =='\0') {
+ break;
+ }
+
+ if (n_adapter == MAX_I2C_ADAPTER) {
+ g_printerr("too many adapter (%d), only support %d \n", n_adapter,
+ MAX_I2C_ADAPTER);
+ goto out;
+ }
+
+ if (qemu_strtol(cp, &t, 10, &bus) || bus < 0) {
+ g_printerr("Invalid bus number %s\n", cp);
+ goto out;
+ }
+
+ cp = t;
+ n_client = 0;
+
+ /* Parse clients <client_addr>[:<client_addr>] entries one by one */
+ while (cp != NULL && *cp !='\0') {
+ if (*cp == ':')
+ cp++;
+
+ if (n_client == MAX_I2C_VDEV) {
+ g_printerr("too many devices (%d), only support %d \n",
+ n_client, MAX_I2C_VDEV);
+ goto out;
+ }
+
+ if (qemu_strtol(cp, &t, 16, &addr) || addr < 0 || addr > MAX_I2C_VDEV) {
+ g_printerr("Invalid address %s : %lx\n", cp, addr);
+ goto out;
+ }
+
+ client_addr[n_client++] = addr;
+ cp = t;
+ if (verbose) {
+ g_print("i2c adapter %ld:0x%lx\n", bus, addr);
+ }
+ }
+
+ i2c->adapter[n_adapter] = vi2c_create_adapter(bus, client_addr, n_client);
+ if (!i2c->adapter[n_adapter])
+ goto out;
+ n_adapter++;
+ }
+
+ if (!n_adapter) {
+ g_printerr("Failed to add any adapters\n");
+ return -1;
+ }
+
+ i2c->adapter_num = n_adapter;
+
+ if (!vi2c_map_adapters(i2c)) {
+ return 0;
+ }
+
+out:
+ vi2c_remove_adapters(i2c);
+ return -1;
+}
+
+static uint8_t vi2c_xfer(VuDev *dev, struct i2c_msg *msg)
+{
+ VuI2c *i2c = container_of(dev, VuI2c, dev.parent);
+ struct i2c_rdwr_ioctl_data data;
+ VI2cAdapter *adapter;
+
+ adapter = vi2c_find_adapter(i2c, msg->addr);
+ if (!adapter) {
+ g_printerr("Failed to find adapter for address: %x\n", msg->addr);
+ return VIRTIO_I2C_MSG_ERR;
+ }
+
+ data.nmsgs = 1;
+ data.msgs = msg;
+
+ if (ioctl(adapter->fd, I2C_RDWR, &data) < 0) {
+ g_printerr("Failed to transfer data to address %x : %d\n", msg->addr, errno);
+ return VIRTIO_I2C_MSG_ERR;
+ }
+
+ if (verbose) {
+ vi2c_dump_msg(msg);
+ }
+
+ return VIRTIO_I2C_MSG_OK;
+}
+
+
+/* Virtio helpers */
+static uint64_t vi2c_get_features(VuDev *dev)
+{
+ if (verbose) {
+ g_info("%s: replying", __func__);
+ }
+ return 0;
+}
+
+static void vi2c_set_features(VuDev *dev, uint64_t features)
+{
+ if (verbose && features) {
+ g_autoptr(GString) s = g_string_new("Requested un-handled feature");
+ g_string_append_printf(s, " 0x%" PRIx64 "", features);
+ g_info("%s: %s", __func__, s->str);
+ }
+}
+
+static void vi2c_handle_ctrl(VuDev *dev, int qidx)
+{
+ VuVirtq *vq = vu_get_queue(dev, qidx);
+ struct i2c_msg msg;
+ struct virtio_i2c_out_hdr *out_hdr;
+ struct virtio_i2c_in_hdr *in_hdr;
+ bool fail_next = false;
+ size_t len, in_hdr_len;
+
+ for (;;) {
+ VuVirtqElement *elem;
+
+ elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
+ if (!elem) {
+ break;
+ }
+
+ g_debug("%s: got queue (in %d, out %d)", __func__, elem->in_num,
+ elem->out_num);
+
+ /* Validate size of out header */
+ if (elem->out_sg[0].iov_len != sizeof(*out_hdr)) {
+ g_warning("%s: Invalid out hdr %zu : %zu\n", __func__,
+ elem->out_sg[0].iov_len, sizeof(*out_hdr));
+ continue;
+ }
+
+ out_hdr = elem->out_sg[0].iov_base;
+
+ /* Bit 0 is reserved in virtio spec */
+ msg.addr = out_hdr->addr >> 1;
+
+ /* Read Operation */
+ if (elem->out_num == 1 && elem->in_num == 2) {
+ len = elem->in_sg[0].iov_len;
+ if (!len) {
+ g_warning("%s: Read buffer length can't be zero\n", __func__);
+ continue;
+ }
+
+ msg.buf = elem->in_sg[0].iov_base;
+ msg.flags = I2C_M_RD;
+ msg.len = len;
+
+ in_hdr = elem->in_sg[1].iov_base;
+ in_hdr_len = elem->in_sg[1].iov_len;
+ } else if (elem->out_num == 2 && elem->in_num == 1) {
+ /* Write Operation */
+ len = elem->out_sg[1].iov_len;
+ if (!len) {
+ g_warning("%s: Write buffer length can't be zero\n", __func__);
+ continue;
+ }
+
+ msg.buf = elem->out_sg[1].iov_base;
+ msg.flags = 0;
+ msg.len = len;
+
+ in_hdr = elem->in_sg[0].iov_base;
+ in_hdr_len = elem->in_sg[0].iov_len;
+ len = 0;
+ } else {
+ g_warning("%s: Transfer type not supported (in %d, out %d)\n",
+ __func__, elem->in_num, elem->out_num);
+ continue;
+ }
+
+ /* Validate size of in header */
+ if (in_hdr_len != sizeof(*in_hdr)) {
+ g_warning("%s: Invalid in hdr %zu : %zu\n", __func__, in_hdr_len,
+ sizeof(*in_hdr));
+ continue;
+ }
+
+ in_hdr->status = fail_next ? VIRTIO_I2C_MSG_ERR : vi2c_xfer(dev, &msg);
+ if (in_hdr->status == VIRTIO_I2C_MSG_ERR) {
+ /* We need to fail remaining transfers as well */
+ fail_next = out_hdr->flags & VIRTIO_I2C_FLAGS_FAIL_NEXT;
+ }
+
+ vu_queue_push(dev, vq, elem, len + sizeof(*in_hdr));
+ }
+
+ vu_queue_notify(dev, vq);
+}
+
+static void
+vi2c_queue_set_started(VuDev *dev, int qidx, bool started)
+{
+ VuVirtq *vq = vu_get_queue(dev, qidx);
+
+ g_debug("queue started %d:%d\n", qidx, started);
+
+ if (!qidx) {
+ vu_set_queue_handler(dev, vq, started ? vi2c_handle_ctrl : NULL);
+ }
+}
+
+/*
+ * vi2c_process_msg: process messages of vhost-user interface
+ *
+ * Any that are not handled here are processed by the libvhost library
+ * itself.
+ */
+static int vi2c_process_msg(VuDev *dev, VhostUserMsg *msg, int *do_reply)
+{
+ VuI2c *i2c = container_of(dev, VuI2c, dev.parent);
+
+ if (msg->request == VHOST_USER_NONE) {
+ g_main_loop_quit(i2c->loop);
+ return 1;
+ }
+
+ return 0;
+}
+
+static const VuDevIface vuiface = {
+ .set_features = vi2c_set_features,
+ .get_features = vi2c_get_features,
+ .queue_set_started = vi2c_queue_set_started,
+ .process_msg = vi2c_process_msg,
+};
+
+static gboolean hangup(gpointer user_data)
+{
+ GMainLoop *loop = (GMainLoop *) user_data;
+ g_info("%s: caught hangup/quit signal, quitting main loop", __func__);
+ g_main_loop_quit(loop);
+ return true;
+}
+
+static void vi2c_panic(VuDev *dev, const char *msg)
+{
+ g_critical("%s\n", msg);
+ exit(EXIT_FAILURE);
+}
+
+/* Print vhost-user.json backend program capabilities */
+static void print_capabilities(void)
+{
+ printf("{\n");
+ printf(" \"type\": \"i2c\"\n");
+ printf(" \"device-list\"\n");
+ printf("}\n");
+}
+
+static void vi2c_destroy(VuI2c *i2c)
+{
+ vi2c_remove_adapters(i2c);
+ vug_deinit(&i2c->dev);
+ if (socket_path) {
+ unlink(socket_path);
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ GError *error = NULL;
+ GOptionContext *context;
+ g_autoptr(GSocket) socket = NULL;
+ VuI2c i2c = {0};
+
+ context = g_option_context_new("vhost-user emulation of I2C device");
+ g_option_context_add_main_entries(context, options, "vhost-user-i2c");
+ if (!g_option_context_parse(context, &argc, &argv, &error))
+ {
+ g_printerr("option parsing failed: %s\n", error->message);
+ exit(1);
+ }
+
+ if (print_cap) {
+ print_capabilities();
+ exit(0);
+ }
+
+ if (!socket_path && socket_fd < 0) {
+ g_printerr("Please specify either --fd or --socket-path\n");
+ exit(EXIT_FAILURE);
+ }
+
+ if (verbose) {
+ g_log_set_handler(NULL, G_LOG_LEVEL_MASK, g_log_default_handler, NULL);
+ g_setenv("G_MESSAGES_DEBUG", "all", true);
+ } else {
+ g_log_set_handler(NULL,
+ G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_ERROR,
+ g_log_default_handler, NULL);
+ }
+
+ /*
+ * Now create a vhost-user socket that we will receive messages
+ * on. Once we have our handler set up we can enter the glib main
+ * loop.
+ */
+ if (socket_path) {
+ g_autoptr(GSocketAddress) addr = g_unix_socket_address_new(socket_path);
+ g_autoptr(GSocket) bind_socket = g_socket_new(G_SOCKET_FAMILY_UNIX, G_SOCKET_TYPE_STREAM,
+ G_SOCKET_PROTOCOL_DEFAULT, &error);
+
+ if (!g_socket_bind(bind_socket, addr, false, &error)) {
+ g_printerr("Failed to bind to socket at %s (%s).\n",
+ socket_path, error->message);
+ exit(EXIT_FAILURE);
+ }
+ if (!g_socket_listen(bind_socket, &error)) {
+ g_printerr("Failed to listen on socket %s (%s).\n",
+ socket_path, error->message);
+ }
+ g_message("awaiting connection to %s", socket_path);
+ socket = g_socket_accept(bind_socket, NULL, &error);
+ if (!socket) {
+ g_printerr("Failed to accept on socket %s (%s).\n",
+ socket_path, error->message);
+ }
+ } else {
+ socket = g_socket_new_from_fd(socket_fd, &error);
+ if (!socket) {
+ g_printerr("Failed to connect to FD %d (%s).\n",
+ socket_fd, error->message);
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ if (vi2c_parse(&i2c)) {
+ exit(EXIT_FAILURE);
+ }
+
+ /*
+ * Create the main loop first so all the various sources can be
+ * added. As well as catching signals we need to ensure vug_init
+ * can add it's GSource watches.
+ */
+
+ i2c.loop = g_main_loop_new(NULL, FALSE);
+ /* catch exit signals */
+ g_unix_signal_add(SIGHUP, hangup, i2c.loop);
+ g_unix_signal_add(SIGINT, hangup, i2c.loop);
+
+ if (!vug_init(&i2c.dev, VHOST_USER_I2C_MAX_QUEUES, g_socket_get_fd(socket),
+ vi2c_panic, &vuiface)) {
+ g_printerr("Failed to initialize libvhost-user-glib.\n");
+ exit(EXIT_FAILURE);
+ }
+
+
+ g_message("entering main loop, awaiting messages");
+ g_main_loop_run(i2c.loop);
+ g_message("finished main loop, cleaning up");
+
+ g_main_loop_unref(i2c.loop);
+ vi2c_destroy(&i2c);
+}
diff --git a/tools/vhost-user-i2c/meson.build b/tools/vhost-user-i2c/meson.build
new file mode 100644
index 000000000000..f71e9fec7df6
--- /dev/null
+++ b/tools/vhost-user-i2c/meson.build
@@ -0,0 +1,10 @@
+executable('vhost-user-i2c', files(
+ 'main.c'),
+ dependencies: [qemuutil, glib, gio],
+ install: true,
+ install_dir: get_option('libexecdir'))
+
+configure_file(input: '50-qemu-i2c.json.in',
+ output: '50-qemu-i2c.json',
+ configuration: config_host,
+ install_dir: qemu_datadir / 'vhost-user')
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/5] docs: add a man page for vhost-user-i2c
2021-03-24 7:33 [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend Viresh Kumar
` (2 preceding siblings ...)
2021-03-24 7:33 ` [PATCH 3/5] tools/vhost-user-i2c: Add backend driver Viresh Kumar
@ 2021-03-24 7:33 ` Viresh Kumar
2021-03-24 7:33 ` [PATCH 5/5] MAINTAINERS: Add entry for virtio-i2c Viresh Kumar
2021-03-24 7:42 ` [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend no-reply
5 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-03-24 7:33 UTC (permalink / raw)
To: qemu-devel
Cc: Vincent Guittot, Jie Deng, Viresh Kumar, Bill Mills,
Arnd Bergmann, Mike Holmes, Alex Bennée, stratos-dev
Basic usage and example invocation.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
docs/tools/index.rst | 1 +
docs/tools/vhost-user-i2c.rst | 75 +++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
create mode 100644 docs/tools/vhost-user-i2c.rst
diff --git a/docs/tools/index.rst b/docs/tools/index.rst
index 3a5829c17a54..af2519406ddf 100644
--- a/docs/tools/index.rst
+++ b/docs/tools/index.rst
@@ -17,3 +17,4 @@ QEMU Tools Guide
qemu-trace-stap
virtfs-proxy-helper
virtiofsd
+ vhost-user-i2c
diff --git a/docs/tools/vhost-user-i2c.rst b/docs/tools/vhost-user-i2c.rst
new file mode 100644
index 000000000000..8471b39d8b1d
--- /dev/null
+++ b/docs/tools/vhost-user-i2c.rst
@@ -0,0 +1,75 @@
+QEMU vhost-user-i2c - I2C emulation backend
+===========================================
+
+Synopsis
+--------
+
+**vhost-user-i2c** [*OPTIONS*]
+
+Description
+-----------
+
+This program is a vhost-user backend that emulates a VirtIO I2C bus.
+This program takes the layout of the i2c bus and its devices on the host
+OS and then talks to them via the /dev/i2c-X interface when a request
+comes from the guest OS for an I2C device.
+
+This program is designed to work with QEMU's ``-device
+vhost-user-i2c-pci`` but should work with any virtual machine monitor
+(VMM) that supports vhost-user. See the Examples section below.
+
+Options
+-------
+
+.. program:: vhost-user-i2c
+
+.. option:: -h, --help
+
+ Print help.
+
+.. option:: -v, --verbose
+
+ Increase verbosity of output
+
+.. option:: -s, --socket-path=PATH
+
+ Listen on vhost-user UNIX domain socket at PATH. Incompatible with --fd.
+
+.. option:: -f, --fd=FDNUM
+
+ Accept connections from vhost-user UNIX domain socket file descriptor FDNUM.
+ The file descriptor must already be listening for connections.
+ Incompatible with --socket-path.
+
+.. option:: -l, --device-list=I2C-DEVICES
+
+ I2c device list at the host OS in the format:
+ <bus>:<client_addr>[:<client_addr>],[<bus>:<client_addr>[:<client_addr>]]
+
+ Example: --device-list "2:1c:20,3:10:2c"
+
+ Here,
+ bus (decimal): adatper bus number. e.g. 2 for /dev/i2c-2, 3 for /dev/i2c-3.
+ client_addr (hex): address for client device. e.g. 0x1C, 0x20, 0x10, 0x2C.
+
+Examples
+--------
+
+The daemon should be started first:
+
+::
+
+ host# vhost-user-i2c --socket-path=vi2c.sock --device-list 0:20
+
+The QEMU invocation needs to create a chardev socket the device can
+use to communicate as well as share the guests memory over a memfd.
+
+::
+
+ host# qemu-system \
+ -chardev socket,path=vi2c.sock,id=vi2c \
+ -device vhost-user-i2c-pci,chardev=vi2c,id=i2c \
+ -m 4096 \
+ -object memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on \
+ -numa node,memdev=mem \
+ ...
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/5] MAINTAINERS: Add entry for virtio-i2c
2021-03-24 7:33 [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend Viresh Kumar
` (3 preceding siblings ...)
2021-03-24 7:33 ` [PATCH 4/5] docs: add a man page for vhost-user-i2c Viresh Kumar
@ 2021-03-24 7:33 ` Viresh Kumar
2021-03-24 7:42 ` [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend no-reply
5 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-03-24 7:33 UTC (permalink / raw)
To: qemu-devel
Cc: Vincent Guittot, Jie Deng, Viresh Kumar, Bill Mills,
Arnd Bergmann, Mike Holmes, Alex Bennée, stratos-dev
This patch adds entry for virtio-i2c related files in MAINTAINERS.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9147e9a429a0..3a80352fc85b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1967,6 +1967,15 @@ F: hw/virtio/virtio-mem-pci.h
F: hw/virtio/virtio-mem-pci.c
F: include/hw/virtio/virtio-mem.h
+virtio-i2c
+M: Viresh Kumar <viresh.kumar@linaro.org>
+S: Supported
+F: docs/tools/vhost-user-i2c.rst
+F: hw/virtio/vhost-user-i2c.c
+F: hw/virtio/vhost-user-i2c-pci.c
+F: include/hw/virtio/vhost-user-i2c.h
+F: tools/vhost-user-i2c/*
+
nvme
M: Keith Busch <kbusch@kernel.org>
M: Klaus Jensen <its@irrelevant.dk>
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend
2021-03-24 7:33 [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend Viresh Kumar
` (4 preceding siblings ...)
2021-03-24 7:33 ` [PATCH 5/5] MAINTAINERS: Add entry for virtio-i2c Viresh Kumar
@ 2021-03-24 7:42 ` no-reply
2021-03-24 11:05 ` Viresh Kumar
5 siblings, 1 reply; 21+ messages in thread
From: no-reply @ 2021-03-24 7:42 UTC (permalink / raw)
To: viresh.kumar
Cc: vincent.guittot, jie.deng, viresh.kumar, bill.mills, qemu-devel,
arnd.bergmann, mike.holmes, alex.bennee, stratos-dev
Patchew URL: https://patchew.org/QEMU/cover.1616570702.git.viresh.kumar@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: cover.1616570702.git.viresh.kumar@linaro.org
Subject: [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/cover.1616570702.git.viresh.kumar@linaro.org -> patchew/cover.1616570702.git.viresh.kumar@linaro.org
Switched to a new branch 'test'
40ccc27 MAINTAINERS: Add entry for virtio-i2c
9b632f4 docs: add a man page for vhost-user-i2c
5da2cca tools/vhost-user-i2c: Add backend driver
6716f9a hw/virtio: add vhost-user-i2c-pci boilerplate
d6439eb hw/virtio: add boilerplate for vhost-user-i2c device
=== OUTPUT BEGIN ===
1/5 Checking commit d6439ebcaa1d (hw/virtio: add boilerplate for vhost-user-i2c device)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#46:
new file mode 100644
total: 0 errors, 1 warnings, 344 lines checked
Patch 1/5 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/5 Checking commit 6716f9a6b4c1 (hw/virtio: add vhost-user-i2c-pci boilerplate)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31:
new file mode 100644
total: 0 errors, 1 warnings, 86 lines checked
Patch 2/5 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/5 Checking commit 5da2ccae459c (tools/vhost-user-i2c: Add backend driver)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#52:
new file mode 100644
ERROR: spaces required around that '*' (ctx:WxV)
#137: FILE: tools/vhost-user-i2c/main.c:70:
+ const typeof(((type *) 0)->member) *__mptr = (ptr); \
^
ERROR: space required after that ';' (ctx:VxV)
#138: FILE: tools/vhost-user-i2c/main.c:71:
+ (type *) ((char *) __mptr - offsetof(type, member));})
^
ERROR: line over 90 characters
#163: FILE: tools/vhost-user-i2c/main.c:96:
+ { "socket-path", 's', 0, G_OPTION_ARG_FILENAME, &socket_path, "Location of vhost-user Unix domain socket, incompatible with --fd", "PATH" },
ERROR: line over 90 characters
#164: FILE: tools/vhost-user-i2c/main.c:97:
+ { "fd", 'f', 0, G_OPTION_ARG_INT, &socket_fd, "Specify the file-descriptor of the backend, incompatible with --socket-path", "FD" },
ERROR: line over 90 characters
#165: FILE: tools/vhost-user-i2c/main.c:98:
+ { "device-list", 'l', 0, G_OPTION_ARG_STRING, &device_list, "List of i2c-dev bus and attached devices", "I2C Devices" },
ERROR: line over 90 characters
#166: FILE: tools/vhost-user-i2c/main.c:99:
+ { "print-capabilities", 'c', 0, G_OPTION_ARG_NONE, &print_cap, "Output to stdout the backend capabilities in JSON format and exit", NULL},
WARNING: line over 80 characters
#167: FILE: tools/vhost-user-i2c/main.c:100:
+ { "verbose", 'v', 0, G_OPTION_ARG_NONE, &verbose, "Be more verbose in output", NULL},
WARNING: line over 80 characters
#209: FILE: tools/vhost-user-i2c/main.c:142:
+ * Adapter | adapter2 | none | adapter1 | adapter3 | none | none| (val)
WARNING: line over 80 characters
#211: FILE: tools/vhost-user-i2c/main.c:144:
+ * Slave Address | addr 1 | none | addr 2 | addr 3 | none | none| (idx)
ERROR: do not use assignment in if condition
#241: FILE: tools/vhost-user-i2c/main.c:174:
+ if (addr < MAX_I2C_VDEV && ((idx = i2c->adapter_map[addr]) != 0)) {
ERROR: braces {} are necessary for all arms of this statement
#288: FILE: tools/vhost-user-i2c/main.c:221:
+ if (bus < 0)
[...]
WARNING: line over 80 characters
#316: FILE: tools/vhost-user-i2c/main.c:249:
+ g_printerr("client addr 0x%x repeat, not allowed.\n", client_addr[i]);
ERROR: code indent should never use tabs
#340: FILE: tools/vhost-user-i2c/main.c:273:
+ * ^Ie.g. 2 for /dev/i2c-2$
ERROR: code indent should never use tabs
#342: FILE: tools/vhost-user-i2c/main.c:275:
+ * ^Ie.g. 0x1C or 1C$
ERROR: spaces required around that '==' (ctx:WxV)
#359: FILE: tools/vhost-user-i2c/main.c:292:
+ if (!cp || *cp =='\0') {
^
ERROR: unnecessary whitespace before a quoted newline
#364: FILE: tools/vhost-user-i2c/main.c:297:
+ g_printerr("too many adapter (%d), only support %d \n", n_adapter,
ERROR: spaces required around that '!=' (ctx:WxV)
#378: FILE: tools/vhost-user-i2c/main.c:311:
+ while (cp != NULL && *cp !='\0') {
^
ERROR: braces {} are necessary for all arms of this statement
#379: FILE: tools/vhost-user-i2c/main.c:312:
+ if (*cp == ':')
[...]
ERROR: unnecessary whitespace before a quoted newline
#383: FILE: tools/vhost-user-i2c/main.c:316:
+ g_printerr("too many devices (%d), only support %d \n",
WARNING: line over 80 characters
#388: FILE: tools/vhost-user-i2c/main.c:321:
+ if (qemu_strtol(cp, &t, 16, &addr) || addr < 0 || addr > MAX_I2C_VDEV) {
WARNING: line over 80 characters
#400: FILE: tools/vhost-user-i2c/main.c:333:
+ i2c->adapter[n_adapter] = vi2c_create_adapter(bus, client_addr, n_client);
ERROR: braces {} are necessary for all arms of this statement
#401: FILE: tools/vhost-user-i2c/main.c:334:
+ if (!i2c->adapter[n_adapter])
[...]
WARNING: line over 80 characters
#438: FILE: tools/vhost-user-i2c/main.c:371:
+ g_printerr("Failed to transfer data to address %x : %d\n", msg->addr, errno);
ERROR: that open brace { should be on the previous line
#632: FILE: tools/vhost-user-i2c/main.c:565:
+ if (!g_option_context_parse(context, &argc, &argv, &error))
+ {
WARNING: line over 80 characters
#653: FILE: tools/vhost-user-i2c/main.c:586:
+ G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_ERROR,
ERROR: line over 90 characters
#664: FILE: tools/vhost-user-i2c/main.c:597:
+ g_autoptr(GSocket) bind_socket = g_socket_new(G_SOCKET_FAMILY_UNIX, G_SOCKET_TYPE_STREAM,
WARNING: line over 80 characters
#665: FILE: tools/vhost-user-i2c/main.c:598:
+ G_SOCKET_PROTOCOL_DEFAULT, &error);
total: 18 errors, 10 warnings, 686 lines checked
Patch 3/5 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/5 Checking commit 9b632f47c605 (docs: add a man page for vhost-user-i2c)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26:
new file mode 100644
total: 0 errors, 1 warnings, 79 lines checked
Patch 4/5 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/5 Checking commit 40ccc271213e (MAINTAINERS: Add entry for virtio-i2c)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/cover.1616570702.git.viresh.kumar@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend
2021-03-24 7:42 ` [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend no-reply
@ 2021-03-24 11:05 ` Viresh Kumar
0 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-03-24 11:05 UTC (permalink / raw)
To: qemu-devel
Cc: vincent.guittot, jie.deng, bill.mills, arnd.bergmann,
mike.holmes, alex.bennee, stratos-dev
On 24-03-21, 00:42, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/cover.1616570702.git.viresh.kumar@linaro.org/
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
I missed the fact that we have an implementation of checkpatch here as well. I
have updated the patches already after running checkpatch at my end now, and
will fix them while sending V2.
--
viresh
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-24 7:33 ` [PATCH 3/5] tools/vhost-user-i2c: Add backend driver Viresh Kumar
@ 2021-03-25 5:09 ` Jie Deng
2021-03-25 5:22 ` Viresh Kumar
2021-03-25 6:17 ` Jie Deng
2021-03-25 16:16 ` Arnd Bergmann
2 siblings, 1 reply; 21+ messages in thread
From: Jie Deng @ 2021-03-25 5:09 UTC (permalink / raw)
To: Viresh Kumar, qemu-devel
Cc: Vincent Guittot, Bill Mills, Arnd Bergmann, Mike Holmes,
Alex Bennée, stratos-dev
On 2021/3/24 15:33, Viresh Kumar wrote:
> +
> +/* Definitions from virtio-i2c specifications */
> +#define VHOST_USER_I2C_MAX_QUEUES 1
> +
> +/* Status */
> +#define VIRTIO_I2C_MSG_OK 0
> +#define VIRTIO_I2C_MSG_ERR 1
> +
> +/* The bit 0 of the @virtio_i2c_out_hdr.@flags, used to group the requests */
> +#define VIRTIO_I2C_FLAGS_FAIL_NEXT 0x00000001
> +
> +/**
> + * struct virtio_i2c_out_hdr - the virtio I2C message OUT header
> + * @addr: the controlled device's address
> + * @padding: used to pad to full dword
> + * @flags: used for feature extensibility
> + */
> +struct virtio_i2c_out_hdr {
> + uint16_t addr;
> + uint16_t padding;
> + uint32_t flags;
> +} __attribute__((packed));
__le16, __le32 ?
> +
> +/**
> + * struct virtio_i2c_in_hdr - the virtio I2C message IN header
> + * @status: the processing result from the backend
> + */
> +struct virtio_i2c_in_hdr {
> + uint8_t status;
> +} __attribute__((packed));
> +
I understand these definitions can be removed once the frontend driver
is merged by the Linux ?
> +/* vhost-user-i2c definitions */
> +
> +#ifndef container_of
> +#define container_of(ptr, type, member) ({ \
> + const typeof(((type *) 0)->member) *__mptr = (ptr); \
> + (type *) ((char *) __mptr - offsetof(type, member));})
> +#endif
This seems to be a general interface. I see there is a definition in
qemu/compiler.h.
Can we reuse it ?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-25 5:09 ` Jie Deng
@ 2021-03-25 5:22 ` Viresh Kumar
2021-03-25 12:22 ` Alex Bennée
2021-03-30 12:49 ` Alex Bennée
0 siblings, 2 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-03-25 5:22 UTC (permalink / raw)
To: Jie Deng
Cc: Vincent Guittot, Bill Mills, qemu-devel, Arnd Bergmann,
Mike Holmes, Alex Bennée, stratos-dev
On 25-03-21, 13:09, Jie Deng wrote:
>
> On 2021/3/24 15:33, Viresh Kumar wrote:
> > +
> > +/* Definitions from virtio-i2c specifications */
> > +#define VHOST_USER_I2C_MAX_QUEUES 1
> > +
> > +/* Status */
> > +#define VIRTIO_I2C_MSG_OK 0
> > +#define VIRTIO_I2C_MSG_ERR 1
> > +
> > +/* The bit 0 of the @virtio_i2c_out_hdr.@flags, used to group the requests */
> > +#define VIRTIO_I2C_FLAGS_FAIL_NEXT 0x00000001
> > +
> > +/**
> > + * struct virtio_i2c_out_hdr - the virtio I2C message OUT header
> > + * @addr: the controlled device's address
> > + * @padding: used to pad to full dword
> > + * @flags: used for feature extensibility
> > + */
> > +struct virtio_i2c_out_hdr {
> > + uint16_t addr;
> > + uint16_t padding;
> > + uint32_t flags;
> > +} __attribute__((packed));
>
>
> __le16, __le32 ?
Maybe, but I didn't do them because of this:
docs/devel/style.rst:
"Don't use Linux kernel internal types like u32, __u32 or __le32."
> > +
> > +/**
> > + * struct virtio_i2c_in_hdr - the virtio I2C message IN header
> > + * @status: the processing result from the backend
> > + */
> > +struct virtio_i2c_in_hdr {
> > + uint8_t status;
> > +} __attribute__((packed));
> > +
>
> I understand these definitions can be removed once the frontend driver is
> merged by the Linux ?
Yes, we would be required to somehow include the uapi header that
kernel is adding and then this won't be required.
> > +/* vhost-user-i2c definitions */
> > +
> > +#ifndef container_of
> > +#define container_of(ptr, type, member) ({ \
> > + const typeof(((type *) 0)->member) *__mptr = (ptr); \
> > + (type *) ((char *) __mptr - offsetof(type, member));})
> > +#endif
>
>
> This seems to be a general interface. I see there is a definition in
> qemu/compiler.h.
>
> Can we reuse it ?
Damn. My bad (maybe not). I picked this part from the RPMB patchset
that Alex sent and didn't bother looking for it.
Though on the other hand, we are looking to make this file independent
of qemu so it can be used by other hypervisors without any (or much)
modifications, and maybe so it was done so.
Alex ?
--
viresh
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-24 7:33 ` [PATCH 3/5] tools/vhost-user-i2c: Add backend driver Viresh Kumar
2021-03-25 5:09 ` Jie Deng
@ 2021-03-25 6:17 ` Jie Deng
2021-03-25 6:36 ` Viresh Kumar
2021-03-25 16:16 ` Arnd Bergmann
2 siblings, 1 reply; 21+ messages in thread
From: Jie Deng @ 2021-03-25 6:17 UTC (permalink / raw)
To: Viresh Kumar, qemu-devel
Cc: Vincent Guittot, Bill Mills, Arnd Bergmann, Mike Holmes,
Alex Bennée, stratos-dev
On 2021/3/24 15:33, Viresh Kumar wrote:
> +static int vi2c_parse(VuI2c *i2c)
> +{
> + uint16_t client_addr[MAX_I2C_VDEV];
> + int32_t n_adapter = 0, n_client;
> + int64_t addr, bus;
> + const char *cp, *t;
> +
> + while (device_list) {
> + /* Read <bus>:<client_addr>[:<client_addr>] entries one by one */
> + cp = strsep(&device_list, ",");
> +
> + if (!cp || *cp =='\0') {
> + break;
> + }
> +
> + if (n_adapter == MAX_I2C_ADAPTER) {
> + g_printerr("too many adapter (%d), only support %d \n", n_adapter,
> + MAX_I2C_ADAPTER);
> + goto out;
> + }
> +
> + if (qemu_strtol(cp, &t, 10, &bus) || bus < 0) {
> + g_printerr("Invalid bus number %s\n", cp);
> + goto out;
> + }
> +
> + cp = t;
> + n_client = 0;
> +
> + /* Parse clients <client_addr>[:<client_addr>] entries one by one */
> + while (cp != NULL && *cp !='\0') {
> + if (*cp == ':')
> + cp++;
> +
> + if (n_client == MAX_I2C_VDEV) {
> + g_printerr("too many devices (%d), only support %d \n",
> + n_client, MAX_I2C_VDEV);
> + goto out;
> + }
> +
> + if (qemu_strtol(cp, &t, 16, &addr) || addr < 0 || addr > MAX_I2C_VDEV) {
> + g_printerr("Invalid address %s : %lx\n", cp, addr);
> + goto out;
> + }
> +
> + client_addr[n_client++] = addr;
> + cp = t;
> + if (verbose) {
> + g_print("i2c adapter %ld:0x%lx\n", bus, addr);
> + }
> + }
> +
> + i2c->adapter[n_adapter] = vi2c_create_adapter(bus, client_addr, n_client);
> + if (!i2c->adapter[n_adapter])
> + goto out;
> + n_adapter++;
> + }
> +
> + if (!n_adapter) {
> + g_printerr("Failed to add any adapters\n");
> + return -1;
> + }
> +
> + i2c->adapter_num = n_adapter;
i2c->adapter_num is set here, but used in vi2c_remove_adapters.
when you goto out from while {...}, i2c->adapter_num is always 0,
May be a bug ?
> +
> + if (!vi2c_map_adapters(i2c)) {
> + return 0;
> + }
> +
> +out:
> + vi2c_remove_adapters(i2c);
> + return -1;
> +}
> +
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-25 6:17 ` Jie Deng
@ 2021-03-25 6:36 ` Viresh Kumar
0 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-03-25 6:36 UTC (permalink / raw)
To: Jie Deng
Cc: Vincent Guittot, Bill Mills, qemu-devel, Arnd Bergmann,
Mike Holmes, Alex Bennée, stratos-dev
On 25-03-21, 14:17, Jie Deng wrote:
> i2c->adapter_num is set here, but used in vi2c_remove_adapters.
> when you goto out from while {...}, i2c->adapter_num is always 0,
> May be a bug ?
It certainly is, this should fix it:
diff --git a/tools/vhost-user-i2c/main.c b/tools/vhost-user-i2c/main.c
index 071493cbd5c5..65d27ef04d42 100644
--- a/tools/vhost-user-i2c/main.c
+++ b/tools/vhost-user-i2c/main.c
@@ -202,7 +202,7 @@ static void vi2c_remove_adapters(VuI2c *i2c)
VI2cAdapter *adapter;
int32_t i;
- for (i = 0; i < i2c->adapter_num; i++) {
+ for (i = 0; i < MAX_I2C_ADAPTER; i++) {
adapter = i2c->adapter[i];
if (!adapter) {
break;
--
viresh
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-25 5:22 ` Viresh Kumar
@ 2021-03-25 12:22 ` Alex Bennée
2021-03-30 12:49 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-03-25 12:22 UTC (permalink / raw)
To: Viresh Kumar
Cc: Vincent Guittot, Jie Deng, Bill Mills, qemu-devel, Arnd Bergmann,
Mike Holmes, stratos-dev
Viresh Kumar <viresh.kumar@linaro.org> writes:
> On 25-03-21, 13:09, Jie Deng wrote:
>>
>> On 2021/3/24 15:33, Viresh Kumar wrote:
>> > +
>> > +/* Definitions from virtio-i2c specifications */
>> > +#define VHOST_USER_I2C_MAX_QUEUES 1
>> > +
>> > +/* Status */
>> > +#define VIRTIO_I2C_MSG_OK 0
>> > +#define VIRTIO_I2C_MSG_ERR 1
>> > +
>> > +/* The bit 0 of the @virtio_i2c_out_hdr.@flags, used to group the requests */
>> > +#define VIRTIO_I2C_FLAGS_FAIL_NEXT 0x00000001
>> > +
>> > +/**
>> > + * struct virtio_i2c_out_hdr - the virtio I2C message OUT header
>> > + * @addr: the controlled device's address
>> > + * @padding: used to pad to full dword
>> > + * @flags: used for feature extensibility
>> > + */
>> > +struct virtio_i2c_out_hdr {
>> > + uint16_t addr;
>> > + uint16_t padding;
>> > + uint32_t flags;
>> > +} __attribute__((packed));
>>
>>
>> __le16, __le32 ?
>
> Maybe, but I didn't do them because of this:
>
> docs/devel/style.rst:
>
> "Don't use Linux kernel internal types like u32, __u32 or __le32."
>
>> > +
>> > +/**
>> > + * struct virtio_i2c_in_hdr - the virtio I2C message IN header
>> > + * @status: the processing result from the backend
>> > + */
>> > +struct virtio_i2c_in_hdr {
>> > + uint8_t status;
>> > +} __attribute__((packed));
>> > +
>>
>> I understand these definitions can be removed once the frontend driver is
>> merged by the Linux ?
>
> Yes, we would be required to somehow include the uapi header that
> kernel is adding and then this won't be required.
What I often do is include a temporary patch in my series that includes
the updated uapi headers from my Linux branch and mark it as not for
merge until the uapi headers make it into a released tree.
>
>> > +/* vhost-user-i2c definitions */
>> > +
>> > +#ifndef container_of
>> > +#define container_of(ptr, type, member) ({ \
>> > + const typeof(((type *) 0)->member) *__mptr = (ptr); \
>> > + (type *) ((char *) __mptr - offsetof(type, member));})
>> > +#endif
>>
>>
>> This seems to be a general interface. I see there is a definition in
>> qemu/compiler.h.
>>
>> Can we reuse it ?
>
> Damn. My bad (maybe not). I picked this part from the RPMB patchset
> that Alex sent and didn't bother looking for it.
>
> Though on the other hand, we are looking to make this file independent
> of qemu so it can be used by other hypervisors without any (or much)
> modifications, and maybe so it was done so.
>
> Alex ?
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-24 7:33 ` [PATCH 3/5] tools/vhost-user-i2c: Add backend driver Viresh Kumar
2021-03-25 5:09 ` Jie Deng
2021-03-25 6:17 ` Jie Deng
@ 2021-03-25 16:16 ` Arnd Bergmann
2021-03-26 6:01 ` Viresh Kumar
2021-03-26 7:14 ` Viresh Kumar
2 siblings, 2 replies; 21+ messages in thread
From: Arnd Bergmann @ 2021-03-25 16:16 UTC (permalink / raw)
To: Viresh Kumar
Cc: Vincent Guittot, Jie Deng, Bill Mills, QEMU Developers,
Arnd Bergmann, Mike Holmes, Alex Bennée,
Stratos Mailing List
On Wed, Mar 24, 2021 at 8:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> +static uint8_t vi2c_xfer(VuDev *dev, struct i2c_msg *msg)
> +{
> + VuI2c *i2c = container_of(dev, VuI2c, dev.parent);
> + struct i2c_rdwr_ioctl_data data;
> + VI2cAdapter *adapter;
> +
> + adapter = vi2c_find_adapter(i2c, msg->addr);
> + if (!adapter) {
> + g_printerr("Failed to find adapter for address: %x\n", msg->addr);
> + return VIRTIO_I2C_MSG_ERR;
> + }
> +
> + data.nmsgs = 1;
> + data.msgs = msg;
> +
> + if (ioctl(adapter->fd, I2C_RDWR, &data) < 0) {
> + g_printerr("Failed to transfer data to address %x : %d\n", msg->addr, errno);
> + return VIRTIO_I2C_MSG_ERR;
> + }
As you found during testing, this doesn't work for host kernels
that only implement the SMBUS protocol. Since most i2c clients
only need simple register read/write operations, I think you should
at least handle the common ones (and one two byte read/write)
here to make it more useful.
> +static void vi2c_handle_ctrl(VuDev *dev, int qidx)
> +{
> + VuVirtq *vq = vu_get_queue(dev, qidx);
> + struct i2c_msg msg;
> + struct virtio_i2c_out_hdr *out_hdr;
> + struct virtio_i2c_in_hdr *in_hdr;
> + bool fail_next = false;
> + size_t len, in_hdr_len;
> +
> + for (;;) {
> + VuVirtqElement *elem;
> +
> + elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
> + if (!elem) {
> + break;
> + }
> +
> + g_debug("%s: got queue (in %d, out %d)", __func__, elem->in_num,
> + elem->out_num);
> +
> + /* Validate size of out header */
> + if (elem->out_sg[0].iov_len != sizeof(*out_hdr)) {
> + g_warning("%s: Invalid out hdr %zu : %zu\n", __func__,
> + elem->out_sg[0].iov_len, sizeof(*out_hdr));
> + continue;
> + }
> +
> + out_hdr = elem->out_sg[0].iov_base;
> +
> + /* Bit 0 is reserved in virtio spec */
> + msg.addr = out_hdr->addr >> 1;
> +
> + /* Read Operation */
> + if (elem->out_num == 1 && elem->in_num == 2) {
> + len = elem->in_sg[0].iov_len;
> + if (!len) {
> + g_warning("%s: Read buffer length can't be zero\n", __func__);
> + continue;
> + }
It looks like you are not handling endianness conversion here. As far as I
can tell, the protocol requires little-endian data, but the code might
run on a big-endian CPU.
Jie Deng also pointed out the type differences, but actually handling
them correctly is more important that describing them the right way.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-25 16:16 ` Arnd Bergmann
@ 2021-03-26 6:01 ` Viresh Kumar
2021-03-26 8:33 ` Arnd Bergmann
2021-03-26 7:14 ` Viresh Kumar
1 sibling, 1 reply; 21+ messages in thread
From: Viresh Kumar @ 2021-03-26 6:01 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vincent Guittot, Jie Deng, Bill Mills, QEMU Developers,
Arnd Bergmann, Mike Holmes, Alex Bennée,
Stratos Mailing List
On 25-03-21, 17:16, Arnd Bergmann wrote:
> On Wed, Mar 24, 2021 at 8:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> > +static uint8_t vi2c_xfer(VuDev *dev, struct i2c_msg *msg)
> > +{
> > + VuI2c *i2c = container_of(dev, VuI2c, dev.parent);
> > + struct i2c_rdwr_ioctl_data data;
> > + VI2cAdapter *adapter;
> > +
> > + adapter = vi2c_find_adapter(i2c, msg->addr);
> > + if (!adapter) {
> > + g_printerr("Failed to find adapter for address: %x\n", msg->addr);
> > + return VIRTIO_I2C_MSG_ERR;
> > + }
> > +
> > + data.nmsgs = 1;
> > + data.msgs = msg;
> > +
> > + if (ioctl(adapter->fd, I2C_RDWR, &data) < 0) {
> > + g_printerr("Failed to transfer data to address %x : %d\n", msg->addr, errno);
> > + return VIRTIO_I2C_MSG_ERR;
> > + }
>
> As you found during testing, this doesn't work for host kernels
> that only implement the SMBUS protocol. Since most i2c clients
> only need simple register read/write operations, I think you should
> at least handle the common ones (and one two byte read/write)
> here to make it more useful.
I am thinking if that is what we really want to support, then
shouldn't the i2c virtio spec be updated first to support SMBUS type
transfers as well?
> > +static void vi2c_handle_ctrl(VuDev *dev, int qidx)
> > +{
> > + VuVirtq *vq = vu_get_queue(dev, qidx);
> > + struct i2c_msg msg;
> > + struct virtio_i2c_out_hdr *out_hdr;
> > + struct virtio_i2c_in_hdr *in_hdr;
> > + bool fail_next = false;
> > + size_t len, in_hdr_len;
> > +
> > + for (;;) {
> > + VuVirtqElement *elem;
> > +
> > + elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
> > + if (!elem) {
> > + break;
> > + }
> > +
> > + g_debug("%s: got queue (in %d, out %d)", __func__, elem->in_num,
> > + elem->out_num);
> > +
> > + /* Validate size of out header */
> > + if (elem->out_sg[0].iov_len != sizeof(*out_hdr)) {
> > + g_warning("%s: Invalid out hdr %zu : %zu\n", __func__,
> > + elem->out_sg[0].iov_len, sizeof(*out_hdr));
> > + continue;
> > + }
> > +
> > + out_hdr = elem->out_sg[0].iov_base;
> > +
> > + /* Bit 0 is reserved in virtio spec */
> > + msg.addr = out_hdr->addr >> 1;
> > +
> > + /* Read Operation */
> > + if (elem->out_num == 1 && elem->in_num == 2) {
> > + len = elem->in_sg[0].iov_len;
> > + if (!len) {
> > + g_warning("%s: Read buffer length can't be zero\n", __func__);
> > + continue;
> > + }
>
>
> It looks like you are not handling endianness conversion here. As far as I
> can tell, the protocol requires little-endian data, but the code might
> run on a big-endian CPU.
>
> Jie Deng also pointed out the type differences, but actually handling
> them correctly is more important that describing them the right way.
Right, I missed that.
--
viresh
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-25 16:16 ` Arnd Bergmann
2021-03-26 6:01 ` Viresh Kumar
@ 2021-03-26 7:14 ` Viresh Kumar
2021-03-26 8:24 ` Arnd Bergmann
1 sibling, 1 reply; 21+ messages in thread
From: Viresh Kumar @ 2021-03-26 7:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vincent Guittot, Jie Deng, Bill Mills, QEMU Developers,
Arnd Bergmann, Mike Holmes, Alex Bennée,
Stratos Mailing List
On 25-03-21, 17:16, Arnd Bergmann wrote:
> On Wed, Mar 24, 2021 at 8:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > +static void vi2c_handle_ctrl(VuDev *dev, int qidx)
> > +{
> > + VuVirtq *vq = vu_get_queue(dev, qidx);
> > + struct i2c_msg msg;
> > + struct virtio_i2c_out_hdr *out_hdr;
> > + struct virtio_i2c_in_hdr *in_hdr;
> > + bool fail_next = false;
> > + size_t len, in_hdr_len;
> > +
> > + for (;;) {
> > + VuVirtqElement *elem;
> > +
> > + elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
> > + if (!elem) {
> > + break;
> > + }
> > +
> > + g_debug("%s: got queue (in %d, out %d)", __func__, elem->in_num,
> > + elem->out_num);
> > +
> > + /* Validate size of out header */
> > + if (elem->out_sg[0].iov_len != sizeof(*out_hdr)) {
> > + g_warning("%s: Invalid out hdr %zu : %zu\n", __func__,
> > + elem->out_sg[0].iov_len, sizeof(*out_hdr));
> > + continue;
> > + }
> > +
> > + out_hdr = elem->out_sg[0].iov_base;
> > +
> > + /* Bit 0 is reserved in virtio spec */
> > + msg.addr = out_hdr->addr >> 1;
> > +
> > + /* Read Operation */
> > + if (elem->out_num == 1 && elem->in_num == 2) {
> > + len = elem->in_sg[0].iov_len;
> > + if (!len) {
> > + g_warning("%s: Read buffer length can't be zero\n", __func__);
> > + continue;
> > + }
>
>
> It looks like you are not handling endianness conversion here. As far as I
> can tell, the protocol requires little-endian data, but the code might
> run on a big-endian CPU.
I hope this is all we are required to do here, right ?
@@ -442,7 +421,7 @@ static void vi2c_handle_ctrl(VuDev *dev, int qidx)
out_hdr = elem->out_sg[0].iov_base;
/* Bit 0 is reserved in virtio spec */
- msg.addr = out_hdr->addr >> 1;
+ msg.addr = le16toh(out_hdr->addr) >> 1;
/* Read Operation */
if (elem->out_num == 1 && elem->in_num == 2) {
@@ -489,7 +468,7 @@ static void vi2c_handle_ctrl(VuDev *dev, int qidx)
in_hdr->status = fail_next ? VIRTIO_I2C_MSG_ERR : vi2c_xfer(dev, &msg);
if (in_hdr->status == VIRTIO_I2C_MSG_ERR) {
/* We need to fail remaining transfers as well */
- fail_next = out_hdr->flags & VIRTIO_I2C_FLAGS_FAIL_NEXT;
+ fail_next = le32toh(out_hdr->flags) & VIRTIO_I2C_FLAGS_FAIL_NEXT;
}
These are the only fields we are passing apart from buf, which goes
directly to the client device.
--
viresh
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-26 7:14 ` Viresh Kumar
@ 2021-03-26 8:24 ` Arnd Bergmann
0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2021-03-26 8:24 UTC (permalink / raw)
To: Viresh Kumar
Cc: Vincent Guittot, Jie Deng, Bill Mills, QEMU Developers,
Arnd Bergmann, Mike Holmes, Alex Bennée,
Stratos Mailing List
On Fri, Mar 26, 2021 at 8:14 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 25-03-21, 17:16, Arnd Bergmann wrote:
> > On Wed, Mar 24, 2021 at 8:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > It looks like you are not handling endianness conversion here. As far as I
> > can tell, the protocol requires little-endian data, but the code might
> > run on a big-endian CPU.
>
> I hope this is all we are required to do here, right ?
>
> @@ -442,7 +421,7 @@ static void vi2c_handle_ctrl(VuDev *dev, int qidx)
> out_hdr = elem->out_sg[0].iov_base;
>
> /* Bit 0 is reserved in virtio spec */
> - msg.addr = out_hdr->addr >> 1;
> + msg.addr = le16toh(out_hdr->addr) >> 1;
>
> /* Read Operation */
> if (elem->out_num == 1 && elem->in_num == 2) {
> @@ -489,7 +468,7 @@ static void vi2c_handle_ctrl(VuDev *dev, int qidx)
> in_hdr->status = fail_next ? VIRTIO_I2C_MSG_ERR : vi2c_xfer(dev, &msg);
> if (in_hdr->status == VIRTIO_I2C_MSG_ERR) {
> /* We need to fail remaining transfers as well */
> - fail_next = out_hdr->flags & VIRTIO_I2C_FLAGS_FAIL_NEXT;
> + fail_next = le32toh(out_hdr->flags) & VIRTIO_I2C_FLAGS_FAIL_NEXT;
> }
>
> These are the only fields we are passing apart from buf, which goes
> directly to the client device.
I think so, the in_hdr is only one byte long, so it doesn't have an
endianness.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-26 6:01 ` Viresh Kumar
@ 2021-03-26 8:33 ` Arnd Bergmann
0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2021-03-26 8:33 UTC (permalink / raw)
To: Viresh Kumar
Cc: Vincent Guittot, Jie Deng, Bill Mills, QEMU Developers,
Arnd Bergmann, Mike Holmes, Alex Bennée,
Stratos Mailing List
On Fri, Mar 26, 2021 at 7:01 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 25-03-21, 17:16, Arnd Bergmann wrote:
> > On Wed, Mar 24, 2021 at 8:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > > +static uint8_t vi2c_xfer(VuDev *dev, struct i2c_msg *msg)
> > > +{
> > > + VuI2c *i2c = container_of(dev, VuI2c, dev.parent);
> > > + struct i2c_rdwr_ioctl_data data;
> > > + VI2cAdapter *adapter;
> > > +
> > > + adapter = vi2c_find_adapter(i2c, msg->addr);
> > > + if (!adapter) {
> > > + g_printerr("Failed to find adapter for address: %x\n", msg->addr);
> > > + return VIRTIO_I2C_MSG_ERR;
> > > + }
> > > +
> > > + data.nmsgs = 1;
> > > + data.msgs = msg;
> > > +
> > > + if (ioctl(adapter->fd, I2C_RDWR, &data) < 0) {
> > > + g_printerr("Failed to transfer data to address %x : %d\n", msg->addr, errno);
> > > + return VIRTIO_I2C_MSG_ERR;
> > > + }
> >
> > As you found during testing, this doesn't work for host kernels
> > that only implement the SMBUS protocol. Since most i2c clients
> > only need simple register read/write operations, I think you should
> > at least handle the common ones (and one two byte read/write)
> > here to make it more useful.
>
> I am thinking if that is what we really want to support, then
> shouldn't the i2c virtio spec be updated first to support SMBUS type
> transfers as well?
As far as I can tell, all the simple devices should just work, with
I2C_FUNC_SMBUS_READ_BLOCK_DATA being the main exception,
but it seems that has practically no users.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] hw/virtio: add boilerplate for vhost-user-i2c device
2021-03-24 7:33 ` [PATCH 1/5] hw/virtio: add boilerplate for vhost-user-i2c device Viresh Kumar
@ 2021-03-29 15:13 ` Alex Bennée
0 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-03-29 15:13 UTC (permalink / raw)
To: Viresh Kumar
Cc: Vincent Guittot, Jie Deng, Bill Mills, qemu-devel, Arnd Bergmann,
Mike Holmes, stratos-dev
Viresh Kumar <viresh.kumar@linaro.org> writes:
> This creates the QEMU side of the vhost-user-i2c device which connects
> to the remote daemon. It is based of vhost-user-fs code.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> hw/virtio/Kconfig | 5 +
> hw/virtio/meson.build | 1 +
> hw/virtio/vhost-user-i2c.c | 286 ++++++++++++++++++++
> include/hw/virtio/vhost-user-i2c.h | 37 +++
> include/standard-headers/linux/virtio_ids.h | 1 +
> 5 files changed, 330 insertions(+)
> create mode 100644 hw/virtio/vhost-user-i2c.c
> create mode 100644 include/hw/virtio/vhost-user-i2c.h
>
> diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
> index 0eda25c4e1bf..35ab45e2095c 100644
> --- a/hw/virtio/Kconfig
> +++ b/hw/virtio/Kconfig
> @@ -58,3 +58,8 @@ config VIRTIO_MEM
> depends on LINUX
> depends on VIRTIO_MEM_SUPPORTED
> select MEM_DEVICE
> +
> +config VHOST_USER_I2C
> + bool
> + default y
> + depends on VIRTIO && VHOST_USER
> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> index fbff9bc9d4de..1a0d736a0db5 100644
> --- a/hw/virtio/meson.build
> +++ b/hw/virtio/meson.build
> @@ -25,6 +25,7 @@ virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.
> virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
> virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
> virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
> +virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
>
> virtio_pci_ss = ss.source_set()
> virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
> diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
> new file mode 100644
> index 000000000000..7b0dc24412a4
> --- /dev/null
> +++ b/hw/virtio/vhost-user-i2c.c
> @@ -0,0 +1,286 @@
> +/*
> + * Vhost-user i2c virtio device
> + *
> + * Copyright (c) 2021 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/virtio/virtio-bus.h"
> +#include "hw/virtio/vhost-user-i2c.h"
> +#include "qemu/error-report.h"
> +#include "standard-headers/linux/virtio_ids.h"
> +
> +static void vu_i2c_start(VirtIODevice *vdev)
> +{
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> + int ret;
> + int i;
> +
> + if (!k->set_guest_notifiers) {
> + error_report("binding does not support guest notifiers");
> + return;
> + }
> +
> + ret = vhost_dev_enable_notifiers(&i2c->vhost_dev, vdev);
> + if (ret < 0) {
> + error_report("Error enabling host notifiers: %d", -ret);
> + return;
> + }
> +
> + ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, true);
> + if (ret < 0) {
> + error_report("Error binding guest notifier: %d", -ret);
> + goto err_host_notifiers;
> + }
> +
> + i2c->vhost_dev.acked_features = vdev->guest_features;
> + ret = vhost_dev_start(&i2c->vhost_dev, vdev);
> + if (ret < 0) {
> + error_report("Error starting vhost-user-i2c: %d", -ret);
> + goto err_guest_notifiers;
> + }
> +
> + /*
> + * guest_notifier_mask/pending not used yet, so just unmask
> + * everything here. virtio-pci will do the right thing by
> + * enabling/disabling irqfd.
> + */
> + for (i = 0; i < i2c->vhost_dev.nvqs; i++) {
> + vhost_virtqueue_mask(&i2c->vhost_dev, vdev, i, false);
> + }
> +
> + return;
> +
> +err_guest_notifiers:
> + k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
> +err_host_notifiers:
> + vhost_dev_disable_notifiers(&i2c->vhost_dev, vdev);
> +}
> +
> +static void vu_i2c_stop(VirtIODevice *vdev)
> +{
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> + int ret;
> +
> + if (!k->set_guest_notifiers) {
> + return;
> + }
> +
> + vhost_dev_stop(&i2c->vhost_dev, vdev);
> +
> + ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
> + if (ret < 0) {
> + error_report("vhost guest notifier cleanup failed: %d", ret);
> + return;
> + }
> +
> + vhost_dev_disable_notifiers(&i2c->vhost_dev, vdev);
> +}
> +
> +static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status)
> +{
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> + bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
> +
> + if (!vdev->vm_running) {
> + should_start = false;
> + }
> +
> + if (i2c->vhost_dev.started == should_start) {
> + return;
> + }
> +
> + if (should_start) {
> + vu_i2c_start(vdev);
> + } else {
> + vu_i2c_stop(vdev);
> + }
> +}
> +
> +static uint64_t vu_i2c_get_features(VirtIODevice *vdev,
> + uint64_t requested_features, Error **errp)
> +{
> + /* No feature bits used yet */
> + return requested_features;
> +}
> +
> +static void vu_i2c_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + /*
> + * Not normally called; it's the daemon that handles the queue;
> + * however virtio's cleanup path can call this.
> + */
> +}
> +
> +static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
> +{
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> +
> + vhost_virtqueue_mask(&i2c->vhost_dev, vdev, idx, mask);
> +}
> +
> +static bool vu_i2c_guest_notifier_pending(VirtIODevice *vdev, int idx)
> +{
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> +
> + return vhost_virtqueue_pending(&i2c->vhost_dev, idx);
> +}
> +
> +static void do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserI2C *i2c)
> +{
> + vhost_user_cleanup(&i2c->vhost_user);
> + virtio_delete_queue(i2c->req_vq);
> + g_free(i2c->vhost_dev.vqs);
> + virtio_cleanup(vdev);
> + g_free(i2c->vhost_dev.vqs);
This is a double free of the same queue.
> + i2c->vhost_dev.vqs = NULL;
> +}
> +
> +static int vu_i2c_connect(DeviceState *dev)
> +{
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> +
> + if (i2c->connected) {
> + return 0;
> + }
> + i2c->connected = true;
> +
> + /* restore vhost state */
> + if (virtio_device_started(vdev, vdev->status)) {
> + vu_i2c_start(vdev);
> + }
> +
> + return 0;
> +}
> +
> +static void vu_i2c_disconnect(DeviceState *dev)
> +{
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> +
> + if (!i2c->connected) {
> + return;
> + }
> + i2c->connected = false;
> +
> + if (i2c->vhost_dev.started) {
> + vu_i2c_stop(vdev);
> + }
> +}
> +
> +static void vu_i2c_event(void *opaque, QEMUChrEvent event)
> +{
> + DeviceState *dev = opaque;
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
> +
> + switch (event) {
> + case CHR_EVENT_OPENED:
> + if (vu_i2c_connect(dev) < 0) {
> + qemu_chr_fe_disconnect(&i2c->conf.chardev);
> + return;
> + }
> + break;
> + case CHR_EVENT_CLOSED:
> + vu_i2c_disconnect(dev);
> + break;
> + case CHR_EVENT_BREAK:
> + case CHR_EVENT_MUX_IN:
> + case CHR_EVENT_MUX_OUT:
> + /* Ignore */
> + break;
> + }
> +}
> +
> +static void vu_i2c_device_realize(DeviceState *dev, Error **errp)
> +{
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VHostUserI2C *i2c = VHOST_USER_I2C(dev);
> + int ret;
> +
> + if (!i2c->conf.chardev.chr) {
> + error_setg(errp, "missing chardev");
> + return;
> + }
> +
> + if (!vhost_user_init(&i2c->vhost_user, &i2c->conf.chardev, errp)) {
> + return;
> + }
> +
> + virtio_init(vdev, "vhost-user-i2c", VIRTIO_ID_I2C, 0);
> +
> + i2c->req_vq = virtio_add_queue(vdev, 3, vu_i2c_handle_output);
> + i2c->vhost_dev.nvqs = 1;
> + i2c->vhost_dev.vqs = g_new0(struct vhost_virtqueue, i2c->vhost_dev.nvqs);
> + ret = vhost_dev_init(&i2c->vhost_dev, &i2c->vhost_user,
> + VHOST_BACKEND_TYPE_USER, 0);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "vhost_dev_init() failed");
> + do_vhost_user_cleanup(vdev, i2c);
> + }
> +
> + qemu_chr_fe_set_handlers(&i2c->conf.chardev, NULL, NULL, vu_i2c_event, NULL,
> + dev, NULL, true);
> +}
> +
> +static void vu_i2c_device_unrealize(DeviceState *dev)
> +{
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VHostUserI2C *i2c = VHOST_USER_I2C(dev);
> +
> + /* This will stop vhost backend if appropriate. */
> + vu_i2c_set_status(vdev, 0);
> +
> + vhost_dev_cleanup(&i2c->vhost_dev);
> +
> + do_vhost_user_cleanup(vdev, i2c);
> +}
> +
> +static const VMStateDescription vu_i2c_vmstate = {
> + .name = "vhost-user-i2c",
> + .unmigratable = 1,
> +};
> +
> +static Property vu_i2c_properties[] = {
> + DEFINE_PROP_CHR("chardev", VHostUserI2C, conf.chardev),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vu_i2c_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +
> + device_class_set_props(dc, vu_i2c_properties);
> + dc->vmsd = &vu_i2c_vmstate;
> + set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> + vdc->realize = vu_i2c_device_realize;
> + vdc->unrealize = vu_i2c_device_unrealize;
> + vdc->get_features = vu_i2c_get_features;
> + vdc->set_status = vu_i2c_set_status;
> + vdc->guest_notifier_mask = vu_i2c_guest_notifier_mask;
> + vdc->guest_notifier_pending = vu_i2c_guest_notifier_pending;
> +}
> +
> +static const TypeInfo vu_i2c_info = {
> + .name = TYPE_VHOST_USER_I2C,
> + .parent = TYPE_VIRTIO_DEVICE,
> + .instance_size = sizeof(VHostUserI2C),
> + .class_init = vu_i2c_class_init,
> +};
> +
> +static void vu_i2c_register_types(void)
> +{
> + type_register_static(&vu_i2c_info);
> +}
> +
> +type_init(vu_i2c_register_types)
> diff --git a/include/hw/virtio/vhost-user-i2c.h b/include/hw/virtio/vhost-user-i2c.h
> new file mode 100644
> index 000000000000..a5fffcb6096c
> --- /dev/null
> +++ b/include/hw/virtio/vhost-user-i2c.h
> @@ -0,0 +1,37 @@
> +/*
> + * Vhost-user i2c virtio device
> + *
> + * Copyright (c) 2021 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef _QEMU_VHOST_USER_I2C_H
> +#define _QEMU_VHOST_USER_I2C_H
> +
> +#include "hw/virtio/virtio.h"
> +#include "hw/virtio/vhost.h"
> +#include "hw/virtio/vhost-user.h"
> +#include "chardev/char-fe.h"
> +
> +#define TYPE_VHOST_USER_I2C "vhost-user-i2c-device"
> +OBJECT_DECLARE_SIMPLE_TYPE(VHostUserI2C, VHOST_USER_I2C)
> +
> +typedef struct {
> + CharBackend chardev;
> +} VHostUserI2CConf;
> +
> +struct VHostUserI2C {
> + /*< private >*/
> + VirtIODevice parent;
> + VHostUserI2CConf conf;
> + struct vhost_virtqueue *vhost_vq;
> + struct vhost_dev vhost_dev;
> + VhostUserState vhost_user;
> + VirtQueue *req_vq;
> + bool connected;
> +
> + /*< public >*/
> +};
> +
> +#endif /* _QEMU_VHOST_USER_I2C_H */
> diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
> index bc1c0621f5ed..aba00c54b9cd 100644
> --- a/include/standard-headers/linux/virtio_ids.h
> +++ b/include/standard-headers/linux/virtio_ids.h
> @@ -54,5 +54,6 @@
> #define VIRTIO_ID_FS 26 /* virtio filesystem */
> #define VIRTIO_ID_PMEM 27 /* virtio pmem */
> #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
> +#define VIRTIO_ID_I2C 34 /* virtio i2c */
This should come in as a separate patch. Generally I run:
./scripts/update-linux-headers.sh ~/lsrc/linux.git
and tag the patch as (!MERGE) as a place holder until it can get picked
up with one of the regular UAPI updates.
>
> #endif /* _LINUX_VIRTIO_IDS_H */
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] hw/virtio: add vhost-user-i2c-pci boilerplate
2021-03-24 7:33 ` [PATCH 2/5] hw/virtio: add vhost-user-i2c-pci boilerplate Viresh Kumar
@ 2021-03-29 15:19 ` Alex Bennée
0 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-03-29 15:19 UTC (permalink / raw)
To: Viresh Kumar
Cc: Vincent Guittot, Jie Deng, Bill Mills, qemu-devel, Arnd Bergmann,
Mike Holmes, stratos-dev
Viresh Kumar <viresh.kumar@linaro.org> writes:
> This allows is to instantiate a vhost-user-i2c device as part of a PCI
> bus. It is mostly boilerplate which looks pretty similar to the
> vhost-user-fs-pci device.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> hw/virtio/meson.build | 1 +
> hw/virtio/vhost-user-i2c-pci.c | 79 ++++++++++++++++++++++++++++++++++
> 2 files changed, 80 insertions(+)
> create mode 100644 hw/virtio/vhost-user-i2c-pci.c
>
> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> index 1a0d736a0db5..bc352a600911 100644
> --- a/hw/virtio/meson.build
> +++ b/hw/virtio/meson.build
> @@ -26,6 +26,7 @@ virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
> virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
> virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
> virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
> +virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_I2C'], if_true: files('vhost-user-i2c-pci.c'))
>
> virtio_pci_ss = ss.source_set()
> virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
> diff --git a/hw/virtio/vhost-user-i2c-pci.c b/hw/virtio/vhost-user-i2c-pci.c
> new file mode 100644
> index 000000000000..4bcfeafcb632
> --- /dev/null
> +++ b/hw/virtio/vhost-user-i2c-pci.c
> @@ -0,0 +1,79 @@
> +/*
> + * Vhost-user i2c virtio device PCI glue
> + *
> + * Copyright (c) 2021 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/virtio/vhost-user-i2c.h"
> +#include "virtio-pci.h"
> +
> +struct VHostUserI2CPCI {
> + VirtIOPCIProxy parent_obj;
> + VHostUserI2C vdev;
> +};
> +
> +typedef struct VHostUserI2CPCI VHostUserI2CPCI;
> +
> +#define TYPE_VHOST_USER_I2C_PCI "vhost-user-i2c-pci-base"
> +
> +DECLARE_INSTANCE_CHECKER(VHostUserI2CPCI, VHOST_USER_I2C_PCI,
> + TYPE_VHOST_USER_I2C_PCI)
> +
> +static Property vhost_user_i2c_pci_properties[] = {
> + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> + DEV_NVECTORS_UNSPECIFIED),
I suspect we can drop the property if there is nothing useful the user
can specify here. We can just default to 1 on device realization.
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] tools/vhost-user-i2c: Add backend driver
2021-03-25 5:22 ` Viresh Kumar
2021-03-25 12:22 ` Alex Bennée
@ 2021-03-30 12:49 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-03-30 12:49 UTC (permalink / raw)
To: Viresh Kumar
Cc: Vincent Guittot, Jie Deng, Bill Mills, qemu-devel, Arnd Bergmann,
Mike Holmes, stratos-dev
Viresh Kumar <viresh.kumar@linaro.org> writes:
> On 25-03-21, 13:09, Jie Deng wrote:
>>
>> On 2021/3/24 15:33, Viresh Kumar wrote:
<snip>
>
>> > +/* vhost-user-i2c definitions */
>> > +
>> > +#ifndef container_of
>> > +#define container_of(ptr, type, member) ({ \
>> > + const typeof(((type *) 0)->member) *__mptr = (ptr); \
>> > + (type *) ((char *) __mptr - offsetof(type, member));})
>> > +#endif
>>
>>
>> This seems to be a general interface. I see there is a definition in
>> qemu/compiler.h.
>>
>> Can we reuse it ?
>
> Damn. My bad (maybe not). I picked this part from the RPMB patchset
> that Alex sent and didn't bother looking for it.
>
> Though on the other hand, we are looking to make this file independent
> of qemu so it can be used by other hypervisors without any (or much)
> modifications, and maybe so it was done so.
>
> Alex ?
Yes when doing virtio-rpmb I ended up copying a bunch of helper
functions from the core QEMU to avoid a dependency on the core code.
These included the vrpmb_iov_* functions as well as the HMAC-SHA-256
implementation (which QEMU has as well).
That said maybe this is futile because I ended up having to qemuutil to
the dependencies:
dependencies: [qemuutil, glib, gio, vhost_user],
because it ends up failing to build due to the trace_ points that have
been added to the libvhost-user library. I'm sure this wouldn't be too
difficult to overcome if needed.
That said this is all build time dependencies - the final binary does
not rely on a QEMU library as everything it needs from qemu is
statically linked in. That doesn't stop it being portable to other
hypervisors or running independently of QEMU but it does tie it to being
built as part of the source tree.
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-03-30 13:46 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 7:33 [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend Viresh Kumar
2021-03-24 7:33 ` [PATCH 1/5] hw/virtio: add boilerplate for vhost-user-i2c device Viresh Kumar
2021-03-29 15:13 ` Alex Bennée
2021-03-24 7:33 ` [PATCH 2/5] hw/virtio: add vhost-user-i2c-pci boilerplate Viresh Kumar
2021-03-29 15:19 ` Alex Bennée
2021-03-24 7:33 ` [PATCH 3/5] tools/vhost-user-i2c: Add backend driver Viresh Kumar
2021-03-25 5:09 ` Jie Deng
2021-03-25 5:22 ` Viresh Kumar
2021-03-25 12:22 ` Alex Bennée
2021-03-30 12:49 ` Alex Bennée
2021-03-25 6:17 ` Jie Deng
2021-03-25 6:36 ` Viresh Kumar
2021-03-25 16:16 ` Arnd Bergmann
2021-03-26 6:01 ` Viresh Kumar
2021-03-26 8:33 ` Arnd Bergmann
2021-03-26 7:14 ` Viresh Kumar
2021-03-26 8:24 ` Arnd Bergmann
2021-03-24 7:33 ` [PATCH 4/5] docs: add a man page for vhost-user-i2c Viresh Kumar
2021-03-24 7:33 ` [PATCH 5/5] MAINTAINERS: Add entry for virtio-i2c Viresh Kumar
2021-03-24 7:42 ` [PATCH 0/5] virtio: Implement generic vhost-user-i2c backend no-reply
2021-03-24 11:05 ` Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).