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.161649...
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...
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
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 */
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 */
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);
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
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')
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 ?
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 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 ?
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.
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;
+}
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;
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
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.
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
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.
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
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 \ + ...
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
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.... --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
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.
stratos-dev@op-lists.linaro.org