On 9/2/2024 6:15 PM, Alex Bennée wrote:
Haixu Cui <quic_haix...@quicinc.com> writes:

Hi Alex,
     Thanks a lot for your comments, please refer to my response below.

On 8/28/2024 1:14 AM, Alex Bennée wrote:
Haixu Cui <quic_haix...@quicinc.com> writes:
Apologies for the delay in getting to this.

This work is based on the virtio-spi spec, virtio-spi driver introduced by
the following patch series:
- https://github.com/oasis-tcs/virtio-spec/tree/virtio-1.4/device-types/spi
- https://lwn.net/Articles/966715/

To test with rust-vmm vhost-user-spi daemon, start the vhost-daemon firstly:
      vhost-device-spi --socket-path=vspi.sock --socket-count=1 --device 
"/dev/spidev0.0"
I'm struggling to test this on my main dev box. Are there any dummy
SPI
modules for the kernel for testing? Otherwise we could consider
implementing something similar to "mock_gpio" for the rust-vmm
vhost-user-spi backend?

I verified this on my board with physical SPI interface, and I don't
know if these is dummy SPI module available in kernel. I'll look into
this.

I'll see if I can boot full Linux into a QEMU machine with SPI devices
which would be another approach.


Great idea.


Then invoke qemu with the following parameters:
      qemu-system-aarch64 -m 1G \
          -chardev socket,path=/home/root/vspi.sock0,id=vspi \
          -device vhost-user-spi-pci,chardev=vspi,id=spi \
          -object memory-backend-file,id=mem,size=1G,mem-path=/dev/shm,share=on 
\
          -numa node,memdev=mem
          ...


Signed-off-by: Haixu Cui <quic_haix...@quicinc.com>
---
   hw/virtio/Kconfig                           |   5 +
   hw/virtio/meson.build                       |   3 +
   hw/virtio/vhost-user-spi-pci.c              |  69 ++++++++
   hw/virtio/vhost-user-spi.c                  |  66 +++++++
   hw/virtio/virtio.c                          |   4 +-
   include/hw/virtio/vhost-user-spi.h          |  25 +++
   include/standard-headers/linux/virtio_ids.h |   1 +
   include/standard-headers/linux/virtio_spi.h | 186 ++++++++++++++++++++
   8 files changed, 358 insertions(+), 1 deletion(-)
   create mode 100644 hw/virtio/vhost-user-spi-pci.c
   create mode 100644 hw/virtio/vhost-user-spi.c
   create mode 100644 include/hw/virtio/vhost-user-spi.h
   create mode 100644 include/standard-headers/linux/virtio_spi.h
Generally we want separate headers patches for the importing of
headers.
Doubly so in this case because I can't see the SPI definitions in the
current Linux master. So:
    - 1/2 - Import headers for SPI (!merge until upstream)
    - 2/2 - Implement vhost-user stub for virtio-spi


Should I move only virtio_spi.h to the first patch, or all of the
header files? I don't quite understand here.

Just the kernel headers (include/standard-headers). You should import
the kernel headers from a checked out kernel tree using:

   ./scripts/update-linux-headers.sh <path/to/linux.git>

and save them as a single commit for the 1st patch. As the SPI code is
not yet upstream just make it clear in the commit to avoid merging. e.g.

   linux-headers: update to 6.10-rc5 + SPI patches (!merge)

   This imports the headers from the current Linux HEAD + the VirtIO SPI
   patches which are not yet upstream. Once the SPI work is up-streamed in
   the kernel they will be imported from there.

Just make sure you kernel base is newer than the last import otherwise
you will get a lot of additional noise.


Thank you so much for your guidance, I'll generate header file based on 6.11-rc7, the latest version.

BR & Thanks


Best Regards
Thanks


diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd4..d5857651e5 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -110,3 +110,8 @@ config VHOST_USER_SCMI
       bool
       default y
       depends on VIRTIO && VHOST_USER
+
+config VHOST_USER_SPI
+    bool
+    default y
+    depends on VIRTIO && VHOST_USER
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 621fc65454..42296219e5 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -26,6 +26,7 @@ if have_vhost
       system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: 
files('vhost-user-rng.c'))
       system_virtio_ss.add(when: 'CONFIG_VHOST_USER_SND', if_true: 
files('vhost-user-snd.c'))
       system_virtio_ss.add(when: 'CONFIG_VHOST_USER_INPUT', if_true: 
files('vhost-user-input.c'))
+    system_virtio_ss.add(when: 'CONFIG_VHOST_USER_SPI', if_true: 
files('vhost-user-spi.c'))
         # PCI Stubs
       system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: 
files('vhost-user-device-pci.c'))
@@ -39,6 +40,8 @@ if have_vhost
                            if_true: files('vhost-user-snd-pci.c'))
       system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 
'CONFIG_VHOST_USER_INPUT'],
                            if_true: files('vhost-user-input-pci.c'))
+    system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_SPI'],
+                         if_true: files('vhost-user-spi-pci.c'))
     endif
     if have_vhost_vdpa
       system_virtio_ss.add(files('vhost-vdpa.c'))
diff --git a/hw/virtio/vhost-user-spi-pci.c b/hw/virtio/vhost-user-spi-pci.c
new file mode 100644
index 0000000000..3565d526af
--- /dev/null
+++ b/hw/virtio/vhost-user-spi-pci.c
@@ -0,0 +1,69 @@
+/*
+ * Vhost-user spi virtio device PCI glue
+ *
+ * Copyright(c) 2024 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/vhost-user-spi.h"
+#include "hw/virtio/virtio-pci.h"
+
+struct VHostUserSPIPCI {
+    VirtIOPCIProxy parent_obj;
+    VHostUserSPI vdev;
+};
+
+typedef struct VHostUserSPIPCI VHostUserSPIPCI;
+
+#define TYPE_VHOST_USER_SPI_PCI "vhost-user-spi-pci-base"
+
+DECLARE_INSTANCE_CHECKER(VHostUserSPIPCI, VHOST_USER_SPI_PCI,
+                         TYPE_VHOST_USER_SPI_PCI)
+
+static void vhost_user_spi_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+    VHostUserSPIPCI *dev = VHOST_USER_SPI_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+
+    vpci_dev->nvectors = 1;
+    qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+}
+
+static void vhost_user_spi_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_spi_pci_realize;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+    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_spi_pci_instance_init(Object *obj)
+{
+    VHostUserSPIPCI *dev = VHOST_USER_SPI_PCI(obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VHOST_USER_SPI);
+}
+
+static const VirtioPCIDeviceTypeInfo vhost_user_spi_pci_info = {
+    .base_name = TYPE_VHOST_USER_SPI_PCI,
+    .non_transitional_name = "vhost-user-spi-pci",
+    .instance_size = sizeof(VHostUserSPIPCI),
+    .instance_init = vhost_user_spi_pci_instance_init,
+    .class_init = vhost_user_spi_pci_class_init,
+};
+
+static void vhost_user_spi_pci_register(void)
+{
+    virtio_pci_types_register(&vhost_user_spi_pci_info);
+}
+
+type_init(vhost_user_spi_pci_register);
diff --git a/hw/virtio/vhost-user-spi.c b/hw/virtio/vhost-user-spi.c
new file mode 100644
index 0000000000..e138b8b53b
--- /dev/null
+++ b/hw/virtio/vhost-user-spi.c
@@ -0,0 +1,66 @@
+/*
+ * Vhost-user spi virtio device
+ *
+ * Copyright(c) 2024 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ * 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-spi.h"
+#include "qemu/error-report.h"
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_spi.h"
+
+static Property vspi_properties[] = {
+    DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vspi_realize(DeviceState *dev, Error **errp)
+{
+    VHostUserBase *vub = VHOST_USER_BASE(dev);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_GET_CLASS(dev);
+
+    /* Fixed for SPI */
+    vub->virtio_id = VIRTIO_ID_SPI;
+    vub->num_vqs = 1;
+    vub->vq_size = 4;
+    vub->config_size = sizeof(struct virtio_spi_config);
+
+    vubc->parent_realize(dev, errp);
+}
+
+static const VMStateDescription vu_spi_vmstate = {
+    .name = "vhost-user-spi",
+    .unmigratable = 1,
+};
+
+static void vu_spi_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_CLASS(klass);
+
+    dc->vmsd = &vu_spi_vmstate;
+    device_class_set_props(dc, vspi_properties);
+    device_class_set_parent_realize(dc, vspi_realize,
+                                    &vubc->parent_realize);
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+}
+
+static const TypeInfo vu_spi_info = {
+    .name = TYPE_VHOST_USER_SPI,
+    .parent = TYPE_VHOST_USER_BASE,
+    .instance_size = sizeof(VHostUserSPI),
+    .class_init = vu_spi_class_init,
+};
+
+static void vu_spi_register_types(void)
+{
+    type_register_static(&vu_spi_info);
+}
+
+type_init(vu_spi_register_types)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 583a224163..689e2e21e7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -46,6 +46,7 @@
   #include "standard-headers/linux/virtio_iommu.h"
   #include "standard-headers/linux/virtio_mem.h"
   #include "standard-headers/linux/virtio_vsock.h"
+#include "standard-headers/linux/virtio_spi.h"
     /*
    * Maximum size of virtio device config space
@@ -194,7 +195,8 @@ const char *virtio_device_names[] = {
       [VIRTIO_ID_PARAM_SERV] = "virtio-param-serv",
       [VIRTIO_ID_AUDIO_POLICY] = "virtio-audio-pol",
       [VIRTIO_ID_BT] = "virtio-bluetooth",
-    [VIRTIO_ID_GPIO] = "virtio-gpio"
+    [VIRTIO_ID_GPIO] = "virtio-gpio",
+    [VIRTIO_ID_SPI] = "virtio-spi"
   };
For the vhost-user-stub bits when split from the headers:
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>



Reply via email to