PCIe endpoint framework provides APIs to implement PCIe endpoint function.
This framework allows defining various PCIe endpoint function behaviors in
software. This patch extend the framework for virtio pci device. The
virtio is defined to communicate guest on virtual machine and host side.
Advantage of the virtio is the efficiency of data transfer and the conciseness
of implementation device using software. It also be applied to PCIe
endpoint function.
We designed and implemented a PCIe EP virtio console function driver using
the extended PCIe endpoint framework for virtio. It can be communicate
host and endpoint over virtio as console.
An architecture of the function driver is following:
┌────────────┐ ┌──────────────────────┬────────────┐
│virtio │ │ │virtio │
│console drv │ ├───────────────┐ │console drv │
├────────────┤ │(virtio console│ ├────────────┤
│ virtio bus │ │ device) │◄────►│ virtio bus │
├────────────┤ ├---------------┤ └────────────┤
│ │ │ pci ep virtio │ │
│ pci bus │ │ console drv │ │
│ │ pcie ├───────────────┤ │
│ │ ◄─────► │ pci ep Bus │ │
└────────────┘ └───────────────┴───────────────────┘
PCIe Root PCIe Endpoint
Introduced driver is `pci ep virtio console drv` in the figure. It works
as ep function for PCIe root and virtual virtio console device for PCIe
endpoint. Each side of virtio console driver has virtqueue, and
introduced driver transfers data on the virtqueue to each other. A data
on root tx queue is transfered to endpoint rx queue and vice versa.
This patchset is depend follwing patches which are under discussion.
- [RFC PATCH 0/3] Deal with alignment restriction on EP side
link: https://lore.kernel.org/linux-pci/[email protected]/
- [PATCH v3] vringh: IOMEM support
link: https://lore.kernel.org/virtualization/CACGkMEumt4p7jU+H+T-b9My0buhdS8a-1GCSnWjnCwMAM=wo1Q@mail.gmail.com/T/#t
First of this patchset is introduce a helper function to realize pci
virtio function using PCIe endpoint framework. The second one is adding
a missing definition for virtio pci header. The last one is for PCIe
endpoint virtio console driver.
This is tested on next-20230416 and RCar S4 board as PCIe endpoint.
Shunsuke Mie (3):
PCI: endpoint: introduce a helper to implement pci ep virtio function
virtio_pci: add a definition of queue flag in ISR
PCI: endpoint: Add EP function driver to provide virtio-console
functionality
drivers/pci/endpoint/functions/Kconfig | 19 +
drivers/pci/endpoint/functions/Makefile | 2 +
drivers/pci/endpoint/functions/pci-epf-vcon.c | 596 ++++++++++++++++++
.../pci/endpoint/functions/pci-epf-virtio.c | 458 ++++++++++++++
.../pci/endpoint/functions/pci-epf-virtio.h | 126 ++++
include/uapi/linux/virtio_pci.h | 3 +
6 files changed, 1204 insertions(+)
create mode 100644 drivers/pci/endpoint/functions/pci-epf-vcon.c
create mode 100644 drivers/pci/endpoint/functions/pci-epf-virtio.c
create mode 100644 drivers/pci/endpoint/functions/pci-epf-virtio.h
--
2.25.1
The Linux PCIe Endpoint framework supports to implement PCIe endpoint
functions using a PCIe controller operating in endpoint mode.
It is possble to realize the behavior of PCIe device, such as virtio PCI
device. This patch introduces a setof helper functions and data structures
to implement a PCIe endpoint function that behaves as a virtio device.
Those functions enable the implementation PCIe endpoint function that
comply with the virtio lecacy specification. Because modern virtio
specifications require devices to implement custom PCIe capabilities, which
are not currently supported by either PCIe controllers/drivers or the PCIe
endpoint framework.
The helper functions work with the new struct epf_virtio, which is
initialized and finalized using the following functions:
- int epf_virtio_init();
- void epf_virtio_final()
Once initialized, the PCIe configuration space can be read and written
using the following functions:
- epf_virtio_cfg_{read,write}#size()
- epf_virtio_cfg_{set,clear}#size()
- epf_virtio_cfg_memcpy_toio()
The size is supported 8, 16 and 32bits. The content of configuration space
varies depending on the type of virtio device.
After the setup, launch the kernel thread for negotiation with host virtio
drivers and detection virtqueue notifications with the function:
- epf_virtio_launch_bgtask()
Also there are functions to shutdown and reset the kthread.
- epf_virtio_terminate_bgtask()
- epf_virtio_terminate_reset()
Data transfer function is also provide.
- epf_virtio_memcpy_kiov2kiov()
While this patch provides functions for negotiating with host drivers and
copying data, each PCIe function driver must impl ement operations that
depend on each specific device, such as network, block, etc.
Signed-off-by: Shunsuke Mie <[email protected]>
---
Changes from v2:
- Improve the memcpy function between kiov and kiov on local ram and
remote ram via pcie bus.
drivers/pci/endpoint/functions/Kconfig | 7 +
drivers/pci/endpoint/functions/Makefile | 1 +
.../pci/endpoint/functions/pci-epf-virtio.c | 458 ++++++++++++++++++
.../pci/endpoint/functions/pci-epf-virtio.h | 126 +++++
4 files changed, 592 insertions(+)
create mode 100644 drivers/pci/endpoint/functions/pci-epf-virtio.c
create mode 100644 drivers/pci/endpoint/functions/pci-epf-virtio.h
diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
index 9fd560886871..fa1a6a569a8f 100644
--- a/drivers/pci/endpoint/functions/Kconfig
+++ b/drivers/pci/endpoint/functions/Kconfig
@@ -37,3 +37,10 @@ config PCI_EPF_VNTB
between PCI Root Port and PCIe Endpoint.
If in doubt, say "N" to disable Endpoint NTB driver.
+
+config PCI_EPF_VIRTIO
+ tristate
+ depends on PCI_ENDPOINT
+ select VHOST_RING_IOMEM
+ help
+ Helpers to implement PCI virtio Endpoint function
diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
index 5c13001deaba..a96f127ce900 100644
--- a/drivers/pci/endpoint/functions/Makefile
+++ b/drivers/pci/endpoint/functions/Makefile
@@ -6,3 +6,4 @@
obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
+obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
diff --git a/drivers/pci/endpoint/functions/pci-epf-virtio.c b/drivers/pci/endpoint/functions/pci-epf-virtio.c
new file mode 100644
index 000000000000..f67610dd247d
--- /dev/null
+++ b/drivers/pci/endpoint/functions/pci-epf-virtio.c
@@ -0,0 +1,458 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Helpers to implement PCIe virtio EP function.
+ */
+#include <linux/virtio_pci.h>
+#include <linux/virtio_config.h>
+#include <linux/kthread.h>
+
+#include "pci-epf-virtio.h"
+
+static void epf_virtio_unmap_vq(struct pci_epf *epf, void __iomem *vq_virt,
+ phys_addr_t vq_phys, unsigned int num)
+{
+ size_t vq_size = vring_size(num, VIRTIO_PCI_VRING_ALIGN);
+
+ pci_epc_unmap_addr(epf->epc, epf->func_no, epf->vfunc_no, vq_phys,
+ vq_virt, vq_size);
+ pci_epc_mem_free_addr(epf->epc, vq_phys, vq_virt, vq_size);
+}
+
+static void __iomem *epf_virtio_map_vq(struct pci_epf *epf,
+ phys_addr_t vq_pci_addr,
+ unsigned int num, phys_addr_t *vq_phys)
+{
+ int err;
+ size_t vq_size;
+ void __iomem *vq_virt;
+
+ vq_size = vring_size(num, VIRTIO_PCI_VRING_ALIGN);
+
+ vq_virt = pci_epc_map_addr(epf->epc, epf->func_no, epf->vfunc_no,
+ vq_pci_addr, vq_phys, vq_size);
+ if (IS_ERR(vq_virt)) {
+ pr_err("Failed to map virtuqueue to local");
+ goto err_free;
+ }
+
+ return vq_virt;
+
+err_free:
+ pci_epc_mem_free_addr(epf->epc, *vq_phys, vq_virt, vq_size);
+
+ return ERR_PTR(err);
+}
+
+static void epf_virtio_free_vringh(struct pci_epf *epf, struct epf_vringh *evrh)
+{
+ epf_virtio_unmap_vq(epf, evrh->virt, evrh->phys, evrh->num);
+ kfree(evrh);
+}
+
+static struct epf_vringh *epf_virtio_alloc_vringh(struct pci_epf *epf,
+ u64 features,
+ phys_addr_t pci_addr,
+ unsigned int num)
+{
+ int err;
+ struct vring vring;
+ struct epf_vringh *evrh;
+
+ evrh = kmalloc(sizeof(*evrh), GFP_KERNEL);
+ if (!evrh) {
+ err = -ENOMEM;
+ goto err_unmap_vq;
+ }
+
+ evrh->num = num;
+
+ evrh->virt = epf_virtio_map_vq(epf, pci_addr, num, &evrh->phys);
+ if (IS_ERR(evrh->virt))
+ return evrh->virt;
+
+ vring_init(&vring, num, evrh->virt, VIRTIO_PCI_VRING_ALIGN);
+
+ err = vringh_init_iomem(&evrh->vrh, features, num, false, vring.desc,
+ vring.avail, vring.used);
+ if (err)
+ goto err_free_epf_vq;
+
+ return evrh;
+
+err_free_epf_vq:
+ kfree(evrh);
+
+err_unmap_vq:
+ epf_virtio_unmap_vq(epf, evrh->virt, evrh->phys, evrh->num);
+
+ return ERR_PTR(err);
+}
+
+#define VIRTIO_PCI_LEGACY_CFG_BAR 0
+
+static void __iomem *epf_virtio_alloc_bar(struct pci_epf *epf, size_t size)
+{
+ struct pci_epf_bar *config_bar = &epf->bar[VIRTIO_PCI_LEGACY_CFG_BAR];
+ const struct pci_epc_features *features;
+ void __iomem *bar;
+ int err;
+
+ features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
+ if (!features) {
+ pr_debug("Failed to get PCI EPC features\n");
+ return ERR_PTR(-EOPNOTSUPP);
+ }
+
+ if (features->reserved_bar & BIT(VIRTIO_PCI_LEGACY_CFG_BAR)) {
+ pr_debug("Cannot use the PCI BAR for legacy virtio pci\n");
+ return ERR_PTR(-EOPNOTSUPP);
+ }
+
+ if (features->bar_fixed_size[VIRTIO_PCI_LEGACY_CFG_BAR]) {
+ if (size >
+ features->bar_fixed_size[VIRTIO_PCI_LEGACY_CFG_BAR]) {
+ pr_debug("PCI BAR size is not enough\n");
+ return ERR_PTR(-ENOMEM);
+ }
+ }
+
+ bar = pci_epf_alloc_space(epf, size, VIRTIO_PCI_LEGACY_CFG_BAR,
+ features->align, PRIMARY_INTERFACE);
+ if (!bar) {
+ pr_debug("Failed to allocate virtio-net config memory\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ config_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+ err = pci_epc_set_bar(epf->epc, epf->func_no, epf->vfunc_no,
+ config_bar);
+ if (err) {
+ pr_debug("Failed to set PCI BAR");
+ goto err_free_space;
+ }
+
+ return bar;
+
+err_free_space:
+
+ pci_epf_free_space(epf, bar, VIRTIO_PCI_LEGACY_CFG_BAR,
+ PRIMARY_INTERFACE);
+
+ return ERR_PTR(err);
+}
+
+static void epf_virtio_free_bar(struct pci_epf *epf, void __iomem *bar)
+{
+ struct pci_epf_bar *config_bar = &epf->bar[VIRTIO_PCI_LEGACY_CFG_BAR];
+
+ pci_epc_clear_bar(epf->epc, epf->func_no, epf->vfunc_no, config_bar);
+ pci_epf_free_space(epf, bar, VIRTIO_PCI_LEGACY_CFG_BAR,
+ PRIMARY_INTERFACE);
+}
+
+static void epf_virtio_init_bar(struct epf_virtio *evio, void __iomem *bar)
+{
+ evio->bar = bar;
+
+ epf_virtio_cfg_write32(evio, VIRTIO_PCI_HOST_FEATURES, evio->features);
+ epf_virtio_cfg_write16(evio, VIRTIO_PCI_ISR, VIRTIO_PCI_ISR_QUEUE);
+ epf_virtio_cfg_write16(evio, VIRTIO_PCI_QUEUE_NUM, evio->vqlen);
+ epf_virtio_cfg_write16(evio, VIRTIO_PCI_QUEUE_NOTIFY, evio->nvq);
+ epf_virtio_cfg_write8(evio, VIRTIO_PCI_STATUS, 0);
+}
+
+/**
+ * epf_virtio_init - initialize struct epf_virtio and setup BAR for virtio
+ * @evio: struct epf_virtio to initialize.
+ * @hdr: pci configuration space to show remote host.
+ * @bar_size: pci BAR size it depends on the virtio device type.
+ *
+ * Returns zero or a negative error.
+ */
+int epf_virtio_init(struct epf_virtio *evio, struct pci_epf_header *hdr,
+ size_t bar_size)
+{
+ struct pci_epf *epf = evio->epf;
+ void __iomem *bar;
+ int err;
+
+ err = pci_epc_write_header(epf->epc, epf->func_no, epf->vfunc_no, hdr);
+ if (err)
+ return err;
+
+ bar = epf_virtio_alloc_bar(epf, bar_size);
+ if (IS_ERR(bar))
+ return PTR_ERR(bar);
+
+ epf_virtio_init_bar(evio, bar);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(epf_virtio_init);
+
+/**
+ * epf_virtio_final - finalize struct epf_virtio. it frees bar and memories
+ * @evio: struct epf_virtio to finalize.
+ */
+void epf_virtio_final(struct epf_virtio *evio)
+{
+ epf_virtio_free_bar(evio->epf, evio->bar);
+
+ for (int i = 0; i < evio->nvq; i++)
+ epf_virtio_free_vringh(evio->epf, evio->vrhs[i]);
+
+ kfree(evio->vrhs);
+}
+EXPORT_SYMBOL_GPL(epf_virtio_final);
+
+static int epf_virtio_negotiate_vq(struct epf_virtio *evio)
+{
+ u32 pfn;
+ u16 sel;
+ int i = 0;
+ struct _pair {
+ u32 pfn;
+ u16 sel;
+ } *tmp;
+ int err = 0;
+ size_t nvq = evio->nvq;
+
+ tmp = kmalloc_array(nvq, sizeof(tmp[0]), GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
+
+ /*
+ * PCIe EP framework has no capability to hook access PCI BAR space from
+ * remote host driver, so poll the specific register, queue pfn to detect
+ * the writing from the driver.
+ *
+ * This implementation assumes that the address of each virtqueue is
+ * written only once.
+ */
+ for (i = 0; i < nvq; i++) {
+ while (!(pfn = epf_virtio_cfg_read32(evio,
+ VIRTIO_PCI_QUEUE_PFN)) &&
+ evio->running)
+ ;
+
+ sel = epf_virtio_cfg_read16(evio, VIRTIO_PCI_QUEUE_SEL);
+
+ epf_virtio_cfg_write32(evio, VIRTIO_PCI_QUEUE_PFN, 0);
+
+ tmp[i].pfn = pfn;
+ tmp[i].sel = sel;
+ }
+
+ if (!evio->running)
+ goto err_out;
+
+ evio->vrhs = kmalloc_array(nvq, sizeof(evio->vrhs[0]), GFP_KERNEL);
+ if (!evio->vrhs) {
+ err = -ENOMEM;
+ goto err_out;
+ }
+
+ for (i = 0; i < nvq; i++) {
+ phys_addr_t pci = tmp[i].pfn << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
+
+ evio->vrhs[i] = epf_virtio_alloc_vringh(
+ evio->epf, evio->features, pci, evio->vqlen);
+ if (IS_ERR(evio->vrhs[i])) {
+ err = PTR_ERR(evio->vrhs[i]);
+ goto err_free_evrhs;
+ }
+ }
+
+ kfree(tmp);
+
+ return 0;
+
+err_free_evrhs:
+ for (i -= 1; i > 0; i--)
+ epf_virtio_free_vringh(evio->epf, evio->vrhs[i]);
+
+ kfree(evio->vrhs);
+
+err_out:
+ kfree(tmp);
+
+ return err;
+}
+
+static void epf_virtio_monitor_qnotify(struct epf_virtio *evio)
+{
+ const u16 qn_default = evio->nvq;
+ u16 tmp;
+
+ /* Since there is no way to synchronize between the host and EP functions,
+ * it is possible to miss multiple notifications.
+ */
+ while (evio->running) {
+ tmp = epf_virtio_cfg_read16(evio, VIRTIO_PCI_QUEUE_NOTIFY);
+ if (tmp == qn_default)
+ continue;
+
+ epf_virtio_cfg_write16(evio, VIRTIO_PCI_QUEUE_NOTIFY,
+ qn_default);
+
+ evio->qn_callback(evio->qn_param);
+ }
+}
+
+static int epf_virtio_bgtask(void *param)
+{
+ struct epf_virtio *evio = param;
+ int err;
+
+ err = epf_virtio_negotiate_vq(evio);
+ if (err < 0) {
+ pr_err("failed to negoticate configs with driver\n");
+ return err;
+ }
+
+ while (!(epf_virtio_cfg_read8(evio, VIRTIO_PCI_STATUS) &
+ VIRTIO_CONFIG_S_DRIVER_OK) &&
+ evio->running)
+ ;
+
+ if (evio->ic_callback && evio->running)
+ evio->ic_callback(evio->ic_param);
+
+ epf_virtio_monitor_qnotify(evio);
+
+ return 0;
+}
+
+/**
+ * epf_virtio_launch_bgtask - spawn a kthread that emulates virtio device
+ * operations.
+ * @evio: It should be initialized prior with epf_virtio_init().
+ *
+ * Returns zero or a negative error.
+ */
+int epf_virtio_launch_bgtask(struct epf_virtio *evio)
+{
+ evio->bgtask = kthread_create(epf_virtio_bgtask, evio,
+ "pci-epf-virtio/bgtask");
+ if (IS_ERR(evio->bgtask))
+ return PTR_ERR(evio->bgtask);
+
+ evio->running = true;
+
+ sched_set_fifo(evio->bgtask);
+ wake_up_process(evio->bgtask);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(epf_virtio_launch_bgtask);
+
+/**
+ * epf_virtio_terminate_bgtask - shutdown a device emulation kthread.
+ * @evio: struct epf_virtio it already launched bgtask.
+ */
+void epf_virtio_terminate_bgtask(struct epf_virtio *evio)
+{
+ evio->running = false;
+
+ kthread_stop(evio->bgtask);
+}
+EXPORT_SYMBOL_GPL(epf_virtio_terminate_bgtask);
+
+/**
+ * epf_virtio_reset - reset virtio status
+ * @evio: struct epf_virtio to reset
+ *
+ * Returns zero or a negative error.
+ */
+int epf_virtio_reset(struct epf_virtio *evio)
+{
+ epf_virtio_terminate_bgtask(evio);
+ epf_virtio_init_bar(evio, evio->bar);
+
+ return epf_virtio_launch_bgtask(evio);
+}
+EXPORT_SYMBOL_GPL(epf_virtio_reset);
+
+int epf_virtio_getdesc(struct epf_virtio *evio, int index,
+ struct vringh_kiov *riov, struct vringh_kiov *wiov,
+ u16 *head)
+{
+ struct vringh *vrh = &evio->vrhs[index]->vrh;
+
+ return vringh_getdesc_iomem(vrh, riov, wiov, head, GFP_KERNEL);
+}
+
+void epf_virtio_abandon(struct epf_virtio *evio, int index, int num)
+{
+ struct vringh *vrh = &evio->vrhs[index]->vrh;
+
+ vringh_abandon_iomem(vrh, num);
+}
+
+void epf_virtio_iov_complete(struct epf_virtio *evio, int index, u16 head,
+ size_t total_len)
+{
+ struct vringh *vrh = &evio->vrhs[index]->vrh;
+
+ vringh_complete_iomem(vrh, head, total_len);
+}
+
+int epf_virtio_memcpy_kiov2kiov(struct epf_virtio *evio,
+ struct vringh_kiov *siov,
+ struct vringh_kiov *diov,
+ enum dma_transfer_direction dir)
+{
+ struct pci_epf *epf = evio->epf;
+ size_t slen, dlen;
+ u64 sbase, dbase;
+ phys_addr_t phys;
+ void *dst, *src;
+
+ for (; siov->i < siov->used; siov->i++, diov->i++) {
+ slen = siov->iov[siov->i].iov_len;
+ sbase = (u64)siov->iov[siov->i].iov_base;
+ dlen = diov->iov[diov->i].iov_len;
+ dbase = (u64)diov->iov[diov->i].iov_base;
+
+ if (dlen < slen) {
+ pr_info("not enough buffer\n");
+ return -EINVAL;
+ }
+
+ if (dir == DMA_MEM_TO_DEV) {
+ src = phys_to_virt(sbase);
+
+ dst = pci_epc_map_addr(epf->epc, epf->func_no,
+ epf->vfunc_no, dbase, &phys,
+ slen);
+ if (IS_ERR(dst)) {
+ pr_err("failed to map pci mmoery spact to local\n");
+ return PTR_ERR(dst);
+ }
+ } else {
+ src = pci_epc_map_addr(epf->epc, epf->func_no,
+ epf->vfunc_no, sbase, &phys,
+ slen);
+ if (IS_ERR(src)) {
+ pr_err("failed to map pci mmoery spact to local\n");
+ return PTR_ERR(src);
+ }
+
+ dst = phys_to_virt(dbase);
+ }
+
+ memcpy_fromio(dst, src, slen);
+
+ if (dir == DMA_MEM_TO_DEV) {
+ pci_epc_unmap_addr(epf->epc, epf->func_no,
+ epf->vfunc_no, phys, dst, slen);
+ } else {
+ pci_epc_unmap_addr(epf->epc, epf->func_no,
+ epf->vfunc_no, phys, src, slen);
+ }
+ }
+
+ return 1;
+}
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/pci/endpoint/functions/pci-epf-virtio.h b/drivers/pci/endpoint/functions/pci-epf-virtio.h
new file mode 100644
index 000000000000..80e18ff70c1c
--- /dev/null
+++ b/drivers/pci/endpoint/functions/pci-epf-virtio.h
@@ -0,0 +1,126 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PCI_EPF_VIRTIO_H__
+#define __PCI_EPF_VIRTIO_H__
+
+#include <linux/pci-epf.h>
+#include <linux/pci-epc.h>
+#include <linux/vringh.h>
+#include <linux/dmaengine.h>
+
+struct epf_vringh {
+ struct vringh vrh;
+ void __iomem *virt;
+ phys_addr_t phys;
+ unsigned int num;
+};
+
+struct epf_virtio {
+ /* Base PCI endpoint function */
+ struct pci_epf *epf;
+
+ /* Virtio parameters */
+ u64 features;
+ size_t bar_size;
+ size_t nvq;
+ size_t vqlen;
+
+ /* struct to access virtqueue on remote host */
+ struct epf_vringh **vrhs;
+
+ /* struct for thread to emulate virtio device */
+ struct task_struct *bgtask;
+
+ /* Virtual address of pci configuration space */
+ void __iomem *bar;
+
+ /* Callback function and parameter for queue notifcation
+ * Note: PCI EP function cannot detect qnotify accurately, therefore this
+ * callback function should check all of virtqueue's changes.
+ */
+ void (*qn_callback)(void *param);
+ void *qn_param;
+
+ /* Callback function and parameter for initialize complete */
+ void (*ic_callback)(void *param);
+ void *ic_param;
+
+ bool running;
+};
+
+#define DEFINE_EPF_VIRTIO_CFG_READ(size) \
+ static inline u##size epf_virtio_cfg_read##size( \
+ struct epf_virtio *evio, size_t offset) \
+ { \
+ void __iomem *base = evio->bar + offset; \
+ return ioread##size(base); \
+ }
+
+DEFINE_EPF_VIRTIO_CFG_READ(8)
+DEFINE_EPF_VIRTIO_CFG_READ(16)
+DEFINE_EPF_VIRTIO_CFG_READ(32)
+
+#define DEFINE_EPF_VIRTIO_CFG_WRITE(size) \
+ static inline void epf_virtio_cfg_write##size( \
+ struct epf_virtio *evio, size_t offset, u##size value) \
+ { \
+ void __iomem *base = evio->bar + offset; \
+ iowrite##size(value, base); \
+ }
+
+DEFINE_EPF_VIRTIO_CFG_WRITE(8);
+DEFINE_EPF_VIRTIO_CFG_WRITE(16);
+DEFINE_EPF_VIRTIO_CFG_WRITE(32);
+
+#define DEFINE_EPF_VIRTIO_CFG_SET(size) \
+ static inline void epf_virtio_cfg_set##size( \
+ struct epf_virtio *evio, size_t offset, u##size value) \
+ { \
+ void __iomem *base = evio->bar + offset; \
+ iowrite##size(ioread##size(base) | value, base); \
+ }
+
+DEFINE_EPF_VIRTIO_CFG_SET(8)
+DEFINE_EPF_VIRTIO_CFG_SET(16)
+DEFINE_EPF_VIRTIO_CFG_SET(32)
+
+#define DEFINE_EPF_VIRTIO_CFG_CLEAR(size) \
+ static inline void epf_virtio_cfg_clear##size( \
+ struct epf_virtio *evio, size_t offset, u##size value) \
+ { \
+ void __iomem *base = evio->bar + offset; \
+ iowrite##size(ioread##size(base) & ~value, base); \
+ }
+
+DEFINE_EPF_VIRTIO_CFG_CLEAR(8)
+DEFINE_EPF_VIRTIO_CFG_CLEAR(16)
+DEFINE_EPF_VIRTIO_CFG_CLEAR(32)
+
+static inline void epf_virtio_cfg_memcpy_toio(struct epf_virtio *evio,
+ size_t offset, void *buf,
+ size_t len)
+{
+ void __iomem *base = evio->bar + offset;
+
+ memcpy_toio(base, buf, len);
+}
+
+int epf_virtio_init(struct epf_virtio *evio, struct pci_epf_header *hdr,
+ size_t bar_size);
+void epf_virtio_final(struct epf_virtio *evio);
+int epf_virtio_launch_bgtask(struct epf_virtio *evio);
+void epf_virtio_terminate_bgtask(struct epf_virtio *evio);
+int epf_virtio_reset(struct epf_virtio *evio);
+
+int epf_virtio_getdesc(struct epf_virtio *evio, int index,
+ struct vringh_kiov *riov, struct vringh_kiov *wiov,
+ u16 *head);
+void epf_virtio_abandon(struct epf_virtio *evio, int index, int num);
+void epf_virtio_iov_complete(struct epf_virtio *evio, int index, u16 head,
+ size_t total_len);
+
+int epf_virtio_memcpy_kiov2kiov(struct epf_virtio *evio,
+ struct vringh_kiov *siov,
+ struct vringh_kiov *diov,
+ enum dma_transfer_direction dir);
+
+#endif /* __PCI_EPF_VIRTIO_H__ */
--
2.25.1
Add a new PCIe endpoint function driver that works as a pci virtio-console
device. The console connect to endpoint side console. It enables to
communicate PCIe host and endpoint.
Architecture is following:
┌────────────┐ ┌──────────────────────┬────────────┐
│virtioe │ │ │virtio │
│console drv │ ├───────────────┐ │console drv │
├────────────┤ │(virtio console│ ├────────────┤
│ virtio bus │ │ device) │◄────►│ virtio bus │
├────────────┤ ├---------------┤ └────────────┤
│ │ │ pci ep virtio │ │
│ pci bus │ │ console drv │ │
│ │ pcie ├───────────────┤ │
│ │ ◄─────► │ pci ep Bus │ │
└────────────┘ └───────────────┴───────────────────┘
PCIe Root PCIe Endpoint
This driver has two roles. The first is as a PCIe endpoint virtio console
function, which is implemented using the PCIe endpoint framework and PCIe
EP virtio helpers. The second is as a virtual virtio console device
connected to the virtio bus on PCIe endpoint Linux.
Communication between the two is achieved by copying the virtqueue data
between PCIe root and endpoint, respectively.
This is a simple implementation and does not include features of
virtio-console such as MULTIPORT, EMERG_WRITE, etc. As a result, each
virtio console driver only displays /dev/hvc0.
As an example of usage, by setting getty to /dev/hvc0, it is possible to
login to another host.
Signed-off-by: Shunsuke Mie <[email protected]>
---
Changes from v2:
- Change to use copy functions between kiovs of pci-epf-virtio.
drivers/pci/endpoint/functions/Kconfig | 12 +
drivers/pci/endpoint/functions/Makefile | 1 +
drivers/pci/endpoint/functions/pci-epf-vcon.c | 596 ++++++++++++++++++
3 files changed, 609 insertions(+)
create mode 100644 drivers/pci/endpoint/functions/pci-epf-vcon.c
diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
index fa1a6a569a8f..9ce2698b67e1 100644
--- a/drivers/pci/endpoint/functions/Kconfig
+++ b/drivers/pci/endpoint/functions/Kconfig
@@ -44,3 +44,15 @@ config PCI_EPF_VIRTIO
select VHOST_RING_IOMEM
help
Helpers to implement PCI virtio Endpoint function
+
+config PCI_EPF_VCON
+ tristate "PCI Endpoint virito-console driver"
+ depends on PCI_ENDPOINT
+ select VHOST_RING
+ select PCI_EPF_VIRTIO
+ help
+ PCIe Endpoint virtio-console function implementatino. This module
+ enables to show the virtio-console as pci device to PCIe host side, and
+ another virtual virtio-console device registers to endpoint system.
+ Those devices are connected virtually and can communicate each other.
+
diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
index a96f127ce900..b4056689ce33 100644
--- a/drivers/pci/endpoint/functions/Makefile
+++ b/drivers/pci/endpoint/functions/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
+obj-$(CONFIG_PCI_EPF_VCON) += pci-epf-vcon.o
diff --git a/drivers/pci/endpoint/functions/pci-epf-vcon.c b/drivers/pci/endpoint/functions/pci-epf-vcon.c
new file mode 100644
index 000000000000..31f4247cd10f
--- /dev/null
+++ b/drivers/pci/endpoint/functions/pci-epf-vcon.c
@@ -0,0 +1,596 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCI Endpoint function driver to impliment virtio-console device
+ * functionality.
+ */
+#include <linux/pci-epf.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_pci.h>
+#include <linux/virtio_console.h>
+#include <linux/virtio_ring.h>
+
+#include "pci-epf-virtio.h"
+
+static int virtio_queue_size = 0x100;
+module_param(virtio_queue_size, int, 0444);
+MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
+
+struct epf_vcon {
+ /* To access virtqueues on remote host */
+ struct epf_virtio evio;
+ struct vringh_kiov *rdev_iovs;
+
+ /* To register a local virtio bus */
+ struct virtio_device vdev;
+
+ /* To access virtqueus of local host driver */
+ struct vringh *vdev_vrhs;
+ struct vringh_kiov *vdev_iovs;
+ struct virtqueue **vdev_vqs;
+
+ /* For transportation and notification */
+ struct workqueue_struct *task_wq;
+ struct work_struct raise_irq_work, rx_work, tx_work;
+
+ /* To retain virtio features. It is commonly used local and remote. */
+ u64 features;
+
+ /* To show a status whether this driver is ready and the remote is connected */
+ bool connected;
+};
+
+enum {
+ VCON_VIRTQUEUE_RX,
+ VCON_VIRTQUEUE_TX,
+ // Should be end of enum
+ VCON_VIRTQUEUE_NUM
+};
+
+static struct epf_vcon *vdev_to_vcon(struct virtio_device *vdev)
+{
+ return container_of(vdev, struct epf_vcon, vdev);
+}
+
+static unsigned int epf_vcon_get_nvq(struct epf_vcon *vcon)
+{
+ /* This is a minimum implementation. Doesn't support any features of
+ * virtio console. It means driver and device use just 2 virtuque for tx
+ * and rx.
+ */
+ return VCON_VIRTQUEUE_NUM;
+}
+
+static void epf_vcon_rx_handler(struct work_struct *work)
+{
+ struct epf_vcon *vcon = container_of(work, struct epf_vcon, rx_work);
+ struct epf_virtio *evio = &vcon->evio;
+ struct vringh *dvrh;
+ struct vringh_kiov *siov, *diov;
+ int ret;
+
+ if (unlikely(!vcon->connected))
+ return;
+
+ dvrh = &vcon->vdev_vrhs[VCON_VIRTQUEUE_RX];
+ siov = &vcon->rdev_iovs[VCON_VIRTQUEUE_TX];
+ diov = &vcon->vdev_iovs[VCON_VIRTQUEUE_RX];
+
+ do {
+ u16 shead, dhead;
+ size_t total_len;
+
+ ret = epf_virtio_getdesc(evio, VCON_VIRTQUEUE_TX, siov, NULL,
+ &shead);
+ if (ret <= 0)
+ continue;
+
+ ret = vringh_getdesc_kern(dvrh, NULL, diov, &dhead, GFP_KERNEL);
+ if (ret <= 0) {
+ epf_virtio_abandon(evio, VCON_VIRTQUEUE_TX, 1);
+ continue;
+ }
+
+ total_len = vringh_kiov_length(siov);
+
+ ret = epf_virtio_memcpy_kiov2kiov(evio, siov, diov,
+ DMA_DEV_TO_MEM);
+ if (ret < 0)
+ continue;
+
+ epf_virtio_iov_complete(evio, VCON_VIRTQUEUE_TX, shead,
+ total_len);
+ vringh_complete_kern(dvrh, dhead, total_len);
+
+ } while (ret > 0);
+
+ vring_interrupt(0, vcon->vdev_vqs[VCON_VIRTQUEUE_RX]);
+}
+
+static void epf_vcon_tx_handler(struct work_struct *work)
+{
+ struct epf_vcon *vcon = container_of(work, struct epf_vcon, tx_work);
+ struct epf_virtio *evio = &vcon->evio;
+ struct vringh *svrh;
+ struct vringh_kiov *siov, *diov;
+ int ret;
+
+ if (unlikely(!vcon->connected))
+ return;
+
+ svrh = &vcon->vdev_vrhs[VCON_VIRTQUEUE_TX];
+ siov = &vcon->vdev_iovs[VCON_VIRTQUEUE_TX];
+ diov = &vcon->rdev_iovs[VCON_VIRTQUEUE_RX];
+
+ do {
+ u16 shead, dhead;
+ size_t total_len;
+
+ ret = vringh_getdesc_kern(svrh, siov, NULL, &shead, GFP_KERNEL);
+ if (ret <= 0)
+ continue;
+
+ ret = epf_virtio_getdesc(evio, VCON_VIRTQUEUE_RX, NULL, diov,
+ &dhead);
+ if (ret <= 0) {
+ vringh_abandon_kern(svrh, 1);
+ continue;
+ }
+
+ total_len = vringh_kiov_length(siov);
+
+ ret = epf_virtio_memcpy_kiov2kiov(evio, siov, diov,
+ DMA_MEM_TO_DEV);
+ if (ret < 0)
+ continue;
+
+ vringh_complete_kern(svrh, shead, total_len);
+ epf_virtio_iov_complete(evio, VCON_VIRTQUEUE_RX, dhead,
+ total_len);
+
+ } while (ret > 0);
+
+ queue_work(vcon->task_wq, &vcon->raise_irq_work);
+}
+
+static void epf_vcon_raise_irq_handler(struct work_struct *work)
+{
+ struct epf_vcon *vcon =
+ container_of(work, struct epf_vcon, raise_irq_work);
+ struct pci_epf *epf = vcon->evio.epf;
+
+ pci_epc_raise_irq(epf->epc, epf->func_no, epf->vfunc_no,
+ PCI_EPC_IRQ_INTX, 0);
+}
+
+static int epf_vcon_setup_common(struct epf_vcon *vcon)
+{
+ vcon->features = 0;
+ vcon->connected = false;
+
+ vcon->task_wq =
+ alloc_workqueue("pci-epf-vcon/task-wq",
+ WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND, 0);
+ if (!vcon->task_wq)
+ return -ENOMEM;
+
+ INIT_WORK(&vcon->rx_work, epf_vcon_rx_handler);
+ INIT_WORK(&vcon->tx_work, epf_vcon_tx_handler);
+ INIT_WORK(&vcon->raise_irq_work, epf_vcon_raise_irq_handler);
+
+ return 0;
+}
+
+static void epf_vcon_cleanup_common(struct epf_vcon *vcon)
+{
+ /* Should do first to stop polling in other kernel thread */
+ vcon->connected = false;
+
+ flush_work(&vcon->raise_irq_work);
+ flush_work(&vcon->tx_work);
+ flush_work(&vcon->rx_work);
+
+ destroy_workqueue(vcon->task_wq);
+}
+
+static void epf_vcon_qnotify_callback(void *param)
+{
+ struct epf_vcon *vcon = param;
+
+ queue_work(vcon->task_wq, &vcon->rx_work);
+}
+
+static void epf_vcon_initialize_complete(void *param)
+{
+ struct epf_vcon *vcon = param;
+
+ pr_debug("Remote host has connected\n");
+
+ vcon->connected = true;
+
+ /* send filled buffer */
+ queue_work(vcon->task_wq, &vcon->tx_work);
+}
+
+static struct pci_epf_header epf_vcon_pci_header = {
+ .vendorid = PCI_VENDOR_ID_REDHAT_QUMRANET,
+ .deviceid = VIRTIO_TRANS_ID_CONSOLE,
+ .subsys_vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET,
+ .subsys_id = VIRTIO_ID_CONSOLE,
+ .revid = 0,
+ .baseclass_code = PCI_BASE_CLASS_COMMUNICATION,
+ .interrupt_pin = PCI_INTERRUPT_PIN,
+};
+
+static int epf_vcon_setup_ep_func(struct epf_vcon *vcon, struct pci_epf *epf)
+{
+ int err;
+ struct epf_virtio *evio = &vcon->evio;
+ unsigned int nvq = epf_vcon_get_nvq(vcon);
+
+ vcon->rdev_iovs =
+ kmalloc_array(nvq, sizeof(vcon->rdev_iovs[0]), GFP_KERNEL);
+ if (!vcon->rdev_iovs)
+ return -ENOMEM;
+
+ for (int i = 0; i < nvq; i++)
+ vringh_kiov_init(&vcon->rdev_iovs[i], NULL, 0);
+
+ evio->epf = epf;
+ evio->features = vcon->features;
+ evio->nvq = nvq;
+ evio->vqlen = virtio_queue_size;
+
+ evio->qn_callback = epf_vcon_qnotify_callback;
+ evio->qn_param = vcon;
+
+ evio->ic_callback = epf_vcon_initialize_complete;
+ evio->ic_param = vcon;
+
+ err = epf_virtio_init(evio, &epf_vcon_pci_header, 0);
+ if (err)
+ goto err_cleanup_kiov;
+
+ err = epf_virtio_launch_bgtask(evio);
+ if (err)
+ goto err_virtio_final;
+
+ return 0;
+
+err_virtio_final:
+ epf_virtio_final(evio);
+
+err_cleanup_kiov:
+ for (int i = 0; i < nvq; i++)
+ vringh_kiov_cleanup(&vcon->rdev_iovs[i]);
+
+ kfree(vcon->rdev_iovs);
+
+ return err;
+}
+
+static void epf_vcon_cleanup_ep_func(struct epf_vcon *vcon)
+{
+ epf_virtio_terminate_bgtask(&vcon->evio);
+
+ epf_virtio_final(&vcon->evio);
+
+ kfree(vcon->rdev_iovs);
+}
+
+/*
+ * Functions for local virtio device operation
+ */
+static u64 epf_vcon_vdev_get_features(struct virtio_device *vdev)
+{
+ struct epf_vcon *vcon = vdev_to_vcon(vdev);
+
+ return vcon->features;
+}
+
+static int epf_vcon_vdev_finalize_features(struct virtio_device *vdev)
+{
+ struct epf_vcon *vcon = vdev_to_vcon(vdev);
+
+ return vdev->features != vcon->features;
+}
+
+static void epf_vcon_vdev_get_config(struct virtio_device *vdev,
+ unsigned int offset, void *buf,
+ unsigned int len)
+{
+ /* There is no config for virtio console because this console device
+ * doesn't any support features
+ */
+ memset(buf, 0x00, len);
+}
+
+static void epf_vcon_vdev_set_config(struct virtio_device *vdev,
+ unsigned int offset, const void *buf,
+ unsigned int len)
+{
+ /* Do nothing because this console device doesn't any support features */
+}
+
+static u8 epf_vcon_vdev_get_status(struct virtio_device *vdev)
+{
+ return 0;
+}
+
+static void epf_vcon_vdev_set_status(struct virtio_device *vdev, u8 status)
+{
+ if (status & VIRTIO_CONFIG_S_FAILED)
+ pr_debug("driver failed to setup this device\n");
+}
+
+static void epf_vcon_vdev_reset(struct virtio_device *vdev)
+{
+ struct epf_vcon *vcon = vdev_to_vcon(vdev);
+
+ epf_virtio_reset(&vcon->evio);
+}
+
+static bool epf_vcon_vdev_vq_notify(struct virtqueue *vq)
+{
+ struct epf_vcon *vcon = vdev_to_vcon(vq->vdev);
+
+ switch (vq->index) {
+ case VCON_VIRTQUEUE_RX:
+ case VCON_VIRTQUEUE_TX:
+ queue_work(vcon->task_wq, &vcon->tx_work);
+ break;
+ default:
+ return false;
+ }
+
+ return true;
+}
+
+static int epf_vcon_vdev_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
+ struct virtqueue *vqs[],
+ vq_callback_t *callback[],
+ const char *const names[], const bool *ctx,
+ struct irq_affinity *desc)
+{
+ struct epf_vcon *vcon = vdev_to_vcon(vdev);
+ int err;
+ int qidx, i;
+
+ if (nvqs > epf_vcon_get_nvq(vcon))
+ return -EINVAL;
+
+ for (qidx = 0, i = 0; i < nvqs; i++) {
+ struct virtqueue *vq;
+ const struct vring *vring;
+
+ if (!names[i]) {
+ vqs[i] = NULL;
+ continue;
+ }
+
+ vq = vring_create_virtqueue(qidx++, virtio_queue_size,
+ VIRTIO_PCI_VRING_ALIGN, vdev, true,
+ false, ctx ? ctx[i] : false,
+ epf_vcon_vdev_vq_notify,
+ callback[i], names[i]);
+ if (!vq) {
+ err = -ENOMEM;
+ goto err_del_vqs;
+ }
+
+ vqs[i] = vq;
+ vcon->vdev_vqs[i] = vq;
+
+ vring = virtqueue_get_vring(vq);
+ err = vringh_init_kern(&vcon->vdev_vrhs[i], vcon->features,
+ virtio_queue_size, false, vring->desc,
+ vring->avail, vring->used);
+ if (err) {
+ pr_err("failed to init vringh for vring %d\n", i);
+ goto err_del_vqs;
+ }
+ }
+
+ return 0;
+
+err_del_vqs:
+ for (; i >= 0; i--) {
+ if (!names[i])
+ continue;
+
+ if (!vqs[i])
+ continue;
+
+ vring_del_virtqueue(vqs[i]);
+ }
+ return err;
+}
+
+static void epf_vcon_vdev_del_vqs(struct virtio_device *vdev)
+{
+ struct epf_vcon *vcon = vdev_to_vcon(vdev);
+
+ for (int i = 0; i < epf_vcon_get_nvq(vcon); i++) {
+ if (!vcon->vdev_vqs[i])
+ continue;
+
+ vring_del_virtqueue(vcon->vdev_vqs[i]);
+ }
+}
+
+static const struct virtio_config_ops epf_vcon_vdev_config_ops = {
+ .get_features = epf_vcon_vdev_get_features,
+ .finalize_features = epf_vcon_vdev_finalize_features,
+ .get = epf_vcon_vdev_get_config,
+ .set = epf_vcon_vdev_set_config,
+ .get_status = epf_vcon_vdev_get_status,
+ .set_status = epf_vcon_vdev_set_status,
+ .reset = epf_vcon_vdev_reset,
+ .find_vqs = epf_vcon_vdev_find_vqs,
+ .del_vqs = epf_vcon_vdev_del_vqs,
+};
+
+static void epf_vcon_vdev_release(struct device *dev)
+{
+ /* Do nothing, because the struct virtio_device will be reused. */
+}
+
+static int epf_vcon_setup_vdev(struct epf_vcon *vcon, struct device *parent)
+{
+ int err;
+ struct virtio_device *vdev = &vcon->vdev;
+ const unsigned int nvq = epf_vcon_get_nvq(vcon);
+
+ vcon->vdev_vrhs =
+ kmalloc_array(nvq, sizeof(vcon->vdev_vrhs[0]), GFP_KERNEL);
+ if (!vcon->vdev_vrhs)
+ return -ENOMEM;
+
+ vcon->vdev_iovs =
+ kmalloc_array(nvq, sizeof(vcon->vdev_iovs[0]), GFP_KERNEL);
+ if (!vcon->vdev_iovs) {
+ err = -ENOMEM;
+ goto err_free_vrhs;
+ }
+
+ for (int i = 0; i < nvq; i++)
+ vringh_kiov_init(&vcon->vdev_iovs[i], NULL, 0);
+
+ vcon->vdev_vqs =
+ kmalloc_array(nvq, sizeof(vcon->vdev_vrhs[0]), GFP_KERNEL);
+ if (!vcon->vdev_vqs) {
+ err = -ENOMEM;
+ goto err_cleanup_kiov;
+ }
+
+ vdev->dev.parent = parent;
+ vdev->dev.release = epf_vcon_vdev_release;
+ vdev->config = &epf_vcon_vdev_config_ops;
+ vdev->id.vendor = PCI_VENDOR_ID_REDHAT_QUMRANET;
+ vdev->id.device = VIRTIO_ID_CONSOLE;
+
+ err = register_virtio_device(vdev);
+ if (err)
+ goto err_free_vdev_vqs;
+
+ return 0;
+
+err_free_vdev_vqs:
+ kfree(vcon->vdev_vqs);
+
+err_cleanup_kiov:
+ for (int i = 0; i < nvq; i++)
+ vringh_kiov_cleanup(&vcon->vdev_iovs[i]);
+
+ kfree(vcon->vdev_iovs);
+
+err_free_vrhs:
+ kfree(vcon->vdev_vrhs);
+
+ return err;
+}
+
+static void epf_vcon_cleanup_vdev(struct epf_vcon *vcon)
+{
+ unregister_virtio_device(&vcon->vdev);
+ /* Cleanup struct virtio_device that has kobject, otherwise error occures when
+ * reregister the virtio device.
+ */
+ memset(&vcon->vdev, 0x00, sizeof(vcon->vdev));
+
+ kfree(vcon->vdev_vqs);
+
+ for (int i = 0; i < epf_vcon_get_nvq(vcon); i++)
+ vringh_kiov_cleanup(&vcon->vdev_iovs[i]);
+
+ kfree(vcon->vdev_iovs);
+ kfree(vcon->vdev_vrhs);
+}
+
+static int epf_vcon_bind(struct pci_epf *epf)
+{
+ struct epf_vcon *vcon = epf_get_drvdata(epf);
+ int err;
+
+ err = epf_vcon_setup_common(vcon);
+ if (err)
+ return err;
+
+ err = epf_vcon_setup_ep_func(vcon, epf);
+ if (err)
+ goto err_cleanup_common;
+
+ err = epf_vcon_setup_vdev(vcon, epf->epc->dev.parent);
+ if (err)
+ goto err_cleanup_ep_func;
+
+ return 0;
+
+err_cleanup_common:
+ epf_vcon_cleanup_common(vcon);
+
+err_cleanup_ep_func:
+ epf_vcon_cleanup_ep_func(vcon);
+
+ return err;
+}
+
+static void epf_vcon_unbind(struct pci_epf *epf)
+{
+ struct epf_vcon *vcon = epf_get_drvdata(epf);
+
+ epf_vcon_cleanup_common(vcon);
+ epf_vcon_cleanup_ep_func(vcon);
+ epf_vcon_cleanup_vdev(vcon);
+}
+
+static struct pci_epf_ops epf_vcon_ops = {
+ .bind = epf_vcon_bind,
+ .unbind = epf_vcon_unbind,
+};
+
+static const struct pci_epf_device_id epf_vcon_ids[] = {
+ { .name = "pci_epf_vcon" },
+ {}
+};
+
+static int epf_vcon_probe(struct pci_epf *epf)
+{
+ struct epf_vcon *vcon;
+
+ vcon = devm_kzalloc(&epf->dev, sizeof(*vcon), GFP_KERNEL);
+ if (!vcon)
+ return -ENOMEM;
+
+ epf_set_drvdata(epf, vcon);
+
+ return 0;
+}
+
+static struct pci_epf_driver epf_vcon_drv = {
+ .driver.name = "pci_epf_vcon",
+ .ops = &epf_vcon_ops,
+ .id_table = epf_vcon_ids,
+ .probe = epf_vcon_probe,
+ .owner = THIS_MODULE,
+};
+
+static int __init epf_vcon_init(void)
+{
+ int err;
+
+ err = pci_epf_register_driver(&epf_vcon_drv);
+ if (err)
+ pr_err("Failed to register PCI EP virtio-console function\n");
+
+ return 0;
+}
+module_init(epf_vcon_init);
+
+static void epf_vcon_exit(void)
+{
+ pci_epf_unregister_driver(&epf_vcon_drv);
+}
+module_exit(epf_vcon_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Shunsuke Mie <[email protected]>");
--
2.25.1
Already it has beed defined a config changed flag of ISR, but not the queue
flag. Add a macro for it.
Signed-off-by: Shunsuke Mie <[email protected]>
---
include/uapi/linux/virtio_pci.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index f703afc7ad31..d405bea27240 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -94,6 +94,9 @@
#endif /* VIRTIO_PCI_NO_LEGACY */
+/* Bits for ISR status field:only when if MSI-X is disabled */
+/* The bit of the ISR which indicates a queue entry update. */
+#define VIRTIO_PCI_ISR_QUEUE 0x1
/* The bit of the ISR which indicates a device configuration change. */
#define VIRTIO_PCI_ISR_CONFIG 0x2
/* Vector value used to disable MSI for queue */
--
2.25.1
On Thu, Apr 27, 2023 at 07:44:25PM +0900, Shunsuke Mie wrote:
> ...
> PCI: endpoint: introduce a helper to implement pci ep virtio function
> virtio_pci: add a definition of queue flag in ISR
> PCI: endpoint: Add EP function driver to provide virtio-console
> functionality
Capitalize the first word consistently to match history ("Introduce",
"Add").
Capitalize "PCI" in English text.
Capitalize "EP" since it's sort of an acronym (you did once, but do it
both places :))
Bjorn
Random typos and trivial things. No need to repost until somebody
does a more substantive review.
On Thu, Apr 27, 2023 at 07:44:28PM +0900, Shunsuke Mie wrote:
> Add a new PCIe endpoint function driver that works as a pci virtio-console
> device. The console connect to endpoint side console. It enables to
> communicate PCIe host and endpoint.
s/pci/PCI/
> Architecture is following:
>
> ┌────────────┐ ┌──────────────────────┬────────────┐
> │virtioe │ │ │virtio │
> │console drv │ ├───────────────┐ │console drv │
> ├────────────┤ │(virtio console│ ├────────────┤
> │ virtio bus │ │ device) │◄────►│ virtio bus │
> ├────────────┤ ├---------------┤ └────────────┤
> │ │ │ pci ep virtio │ │
> │ pci bus │ │ console drv │ │
> │ │ pcie ├───────────────┤ │
> │ │ ◄─────► │ pci ep Bus │ │
> └────────────┘ └───────────────┴───────────────────┘
> PCIe Root PCIe Endpoint
s/virtioe/virtio/
s/pci/PCI/
s/pcie/PCIe/
s/ep/EP/
> +config PCI_EPF_VCON
> + tristate "PCI Endpoint virito-console driver"
s/virito/virtio/
> + depends on PCI_ENDPOINT
> + select VHOST_RING
> + select PCI_EPF_VIRTIO
> + help
> + PCIe Endpoint virtio-console function implementatino. This module
> + enables to show the virtio-console as pci device to PCIe host side, and
> + another virtual virtio-console device registers to endpoint system.
> + Those devices are connected virtually and can communicate each other.
s/implementatino/implementation/
s/pci/PCI/
s/can communicate/can communicate with/
> + * PCI Endpoint function driver to impliment virtio-console device
> + * functionality.
s/impliment/implement/
> +static int virtio_queue_size = 0x100;
> +module_param(virtio_queue_size, int, 0444);
> +MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
When and why would users need this parameter? Where is it documented?
> + /* To access virtqueus of local host driver */
s/virtqueus/virtqueues/
> + /* To show a status whether this driver is ready and the remote is connected */
Make this fit in 80 columns.
> + /* This is a minimum implementation. Doesn't support any features of
> + * virtio console. It means driver and device use just 2 virtuque for tx
> + * and rx.
> + */
Use common multi-line comment style:
/*
* This is ...
*/
s/virtuque/virtqueues/
> +static void epf_vcon_raise_irq_handler(struct work_struct *work)
> +{
> + struct epf_vcon *vcon =
> + container_of(work, struct epf_vcon, raise_irq_work);
Rewrap.
> +static int epf_vcon_setup_common(struct epf_vcon *vcon)
> +{
> + vcon->features = 0;
> + vcon->connected = false;
> +
> + vcon->task_wq =
> + alloc_workqueue("pci-epf-vcon/task-wq",
Looks like this would fit on the previous line?
> + WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND, 0);
> +static void epf_vcon_initialize_complete(void *param)
> +{
> + struct epf_vcon *vcon = param;
> +
> + pr_debug("Remote host has connected\n");
Is there any device info you could include here, e.g., with dev_dbg()?
It's nice if users have a little context.
> +static int epf_vcon_setup_ep_func(struct epf_vcon *vcon, struct pci_epf *epf)
> +{
> + int err;
> + struct epf_virtio *evio = &vcon->evio;
> + unsigned int nvq = epf_vcon_get_nvq(vcon);
> +
> + vcon->rdev_iovs =
> + kmalloc_array(nvq, sizeof(vcon->rdev_iovs[0]), GFP_KERNEL);
Move the function name and as many parameters as fit in 80 columns to
the previous line to match prevailing style.
> + /* There is no config for virtio console because this console device
> + * doesn't any support features
> + */
Multi-line comment style.
s/doesn't any support/doesn't support any/? Is that what you mean?
> + /* Do nothing because this console device doesn't any support features */
Same.
> +static void epf_vcon_vdev_set_status(struct virtio_device *vdev, u8 status)
> +{
> + if (status & VIRTIO_CONFIG_S_FAILED)
> + pr_debug("driver failed to setup this device\n");
dev_dbg() if possible.
> + err = vringh_init_kern(&vcon->vdev_vrhs[i], vcon->features,
> + virtio_queue_size, false, vring->desc,
> + vring->avail, vring->used);
> + if (err) {
> + pr_err("failed to init vringh for vring %d\n", i);
dev_err() if possible.
> +static int epf_vcon_setup_vdev(struct epf_vcon *vcon, struct device *parent)
> +{
> + int err;
> + struct virtio_device *vdev = &vcon->vdev;
> + const unsigned int nvq = epf_vcon_get_nvq(vcon);
> +
> + vcon->vdev_vrhs =
> + kmalloc_array(nvq, sizeof(vcon->vdev_vrhs[0]), GFP_KERNEL);
Rewrap.
> + vcon->vdev_iovs =
> + kmalloc_array(nvq, sizeof(vcon->vdev_iovs[0]), GFP_KERNEL);
Rewrap.
> + vcon->vdev_vqs =
> + kmalloc_array(nvq, sizeof(vcon->vdev_vrhs[0]), GFP_KERNEL);
Rewrap.
> +static void epf_vcon_cleanup_vdev(struct epf_vcon *vcon)
> +{
> + unregister_virtio_device(&vcon->vdev);
> + /* Cleanup struct virtio_device that has kobject, otherwise error occures when
> + * reregister the virtio device.
> + */
Multi-line style and rewrap to fit in 80 columns.
> +static int __init epf_vcon_init(void)
> +{
> + int err;
> +
> + err = pci_epf_register_driver(&epf_vcon_drv);
> + if (err)
> + pr_err("Failed to register PCI EP virtio-console function\n");
dev_err() if possible (doesn't look like it *is* possible).
Looks like this registers a *driver*, so maybe change the message from
"function" to "driver"?
Bjorn
Simple typos, don't repost until there's more substantive feedback.
On Thu, Apr 27, 2023 at 07:44:26PM +0900, Shunsuke Mie wrote:
> The Linux PCIe Endpoint framework supports to implement PCIe endpoint
> functions using a PCIe controller operating in endpoint mode.
> It is possble to realize the behavior of PCIe device, such as virtio PCI
> device. This patch introduces a setof helper functions and data structures
> to implement a PCIe endpoint function that behaves as a virtio device.
s/possble/possible/
s/setof/set of/
> Those functions enable the implementation PCIe endpoint function that
> comply with the virtio lecacy specification. Because modern virtio
> specifications require devices to implement custom PCIe capabilities, which
> are not currently supported by either PCIe controllers/drivers or the PCIe
> endpoint framework.
s/implementation PCIe endpoint function/implementation of PCIe endpoint functions/
s/lecacy/legacy/ (What does "legacy" mean? Is there a spec for this?)
I guess "legacy virtio" devices need not implement custom PCIe
capabilities, but "modern virtio" devices must implement them?
Capitalize "Endpoint framework" consistently; sometimes it's
"Endpoint", other times it's "endpoint".
> While this patch provides functions for negotiating with host drivers and
> copying data, each PCIe function driver must impl ement operations that
> depend on each specific device, such as network, block, etc.
s/impl ement/implement/
> +#include <linux/virtio_pci.h>
> +#include <linux/virtio_config.h>
> +#include <linux/kthread.h>
Typically the header includes would be alphabetized if possible.
> + vq_virt = pci_epc_map_addr(epf->epc, epf->func_no, epf->vfunc_no,
> + vq_pci_addr, vq_phys, vq_size);
> + if (IS_ERR(vq_virt)) {
> + pr_err("Failed to map virtuqueue to local");
s/virtuqueue/virtqueue/
I know you probably don't have any way to use dev_err(), but this
message really needs some context, like a driver name and instance or
something.
> +#define VIRTIO_PCI_LEGACY_CFG_BAR 0
What makes this a "legacy cfg BAR"? Is there some spec that covers
virtio BAR usage?
> + * epf_virtio_init - initialize struct epf_virtio and setup BAR for virtio
> + * @evio: struct epf_virtio to initialize.
> + * @hdr: pci configuration space to show remote host.
> + * @bar_size: pci BAR size it depends on the virtio device type.
s/pci/PCI/ (there were also a few more instances above in messages or
comments)
> + * epf_virtio_final - finalize struct epf_virtio. it frees bar and memories
> + * @evio: struct epf_virtio to finalize.
s/bar/BAR/
> +static void epf_virtio_monitor_qnotify(struct epf_virtio *evio)
> +{
> + const u16 qn_default = evio->nvq;
> + u16 tmp;
> +
> + /* Since there is no way to synchronize between the host and EP functions,
> + * it is possible to miss multiple notifications.
Multi-line comment style.
> + err = epf_virtio_negotiate_vq(evio);
> + if (err < 0) {
> + pr_err("failed to negoticate configs with driver\n");
s/negoticate/negotiate/
> + * epf_virtio_reset - reset virtio status
Some of the function descriptions end with a period (".") and others
don't. Please figure out what the most common style is and use that
consistently.
> + dst = pci_epc_map_addr(epf->epc, epf->func_no,
> + epf->vfunc_no, dbase, &phys,
> + slen);
> + if (IS_ERR(dst)) {
> + pr_err("failed to map pci mmoery spact to local\n");
s/pci/PCI/
s/mmoery/memory/
s/spact/space/ ?
Also below.
IIRC some previous messages started with a capital letter. Please
make them all consistent.
> + if (dir == DMA_MEM_TO_DEV) {
> + pci_epc_unmap_addr(epf->epc, epf->func_no,
> + epf->vfunc_no, phys, dst, slen);
> + } else {
> + pci_epc_unmap_addr(epf->epc, epf->func_no,
> + epf->vfunc_no, phys, src, slen);
> + }
> + }
> +
> + return 1;
I guess this function returns either a negative error code or the
value 1? That seems sort of weird (I think "negative error code or
*zero* is more typical), but maybe you're following some convention?
> +#include <linux/pci-epf.h>
> +#include <linux/pci-epc.h>
> +#include <linux/vringh.h>
> +#include <linux/dmaengine.h>
Alpha order if possible
> + /* Virtual address of pci configuration space */
s/pci/PCI/
> + /* Callback function and parameter for queue notifcation
> + * Note: PCI EP function cannot detect qnotify accurately, therefore this
> + * callback function should check all of virtqueue's changes.
> + */
Multi-line comment style.
Bjorn
On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
>
> The Linux PCIe Endpoint framework supports to implement PCIe endpoint
> functions using a PCIe controller operating in endpoint mode.
> It is possble to realize the behavior of PCIe device, such as virtio PCI
> device. This patch introduces a setof helper functions and data structures
> to implement a PCIe endpoint function that behaves as a virtio device.
>
> Those functions enable the implementation PCIe endpoint function that
> comply with the virtio lecacy specification. Because modern virtio
> specifications require devices to implement custom PCIe capabilities, which
> are not currently supported by either PCIe controllers/drivers or the PCIe
> endpoint framework.
>
> The helper functions work with the new struct epf_virtio, which is
> initialized and finalized using the following functions:
>
> - int epf_virtio_init();
> - void epf_virtio_final()
>
> Once initialized, the PCIe configuration space can be read and written
> using the following functions:
>
> - epf_virtio_cfg_{read,write}#size()
> - epf_virtio_cfg_{set,clear}#size()
> - epf_virtio_cfg_memcpy_toio()
>
> The size is supported 8, 16 and 32bits. The content of configuration space
> varies depending on the type of virtio device.
>
> After the setup, launch the kernel thread for negotiation with host virtio
> drivers and detection virtqueue notifications with the function:
>
> - epf_virtio_launch_bgtask()
>
> Also there are functions to shutdown and reset the kthread.
>
> - epf_virtio_terminate_bgtask()
> - epf_virtio_terminate_reset()
>
> Data transfer function is also provide.
>
> - epf_virtio_memcpy_kiov2kiov()
>
> While this patch provides functions for negotiating with host drivers and
> copying data, each PCIe function driver must impl ement operations that
> depend on each specific device, such as network, block, etc.
>
> Signed-off-by: Shunsuke Mie <[email protected]>
> ---
>
> Changes from v2:
> - Improve the memcpy function between kiov and kiov on local ram and
> remote ram via pcie bus.
>
> drivers/pci/endpoint/functions/Kconfig | 7 +
> drivers/pci/endpoint/functions/Makefile | 1 +
> .../pci/endpoint/functions/pci-epf-virtio.c | 458 ++++++++++++++++++
> .../pci/endpoint/functions/pci-epf-virtio.h | 126 +++++
> 4 files changed, 592 insertions(+)
> create mode 100644 drivers/pci/endpoint/functions/pci-epf-virtio.c
> create mode 100644 drivers/pci/endpoint/functions/pci-epf-virtio.h
>
> diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
> index 9fd560886871..fa1a6a569a8f 100644
> --- a/drivers/pci/endpoint/functions/Kconfig
> +++ b/drivers/pci/endpoint/functions/Kconfig
> @@ -37,3 +37,10 @@ config PCI_EPF_VNTB
> between PCI Root Port and PCIe Endpoint.
>
> If in doubt, say "N" to disable Endpoint NTB driver.
> +
> +config PCI_EPF_VIRTIO
> + tristate
> + depends on PCI_ENDPOINT
> + select VHOST_RING_IOMEM
> + help
> + Helpers to implement PCI virtio Endpoint function
> diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
> index 5c13001deaba..a96f127ce900 100644
> --- a/drivers/pci/endpoint/functions/Makefile
> +++ b/drivers/pci/endpoint/functions/Makefile
> @@ -6,3 +6,4 @@
> obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
> obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
> obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
> +obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
> diff --git a/drivers/pci/endpoint/functions/pci-epf-virtio.c b/drivers/pci/endpoint/functions/pci-epf-virtio.c
> new file mode 100644
> index 000000000000..f67610dd247d
> --- /dev/null
> +++ b/drivers/pci/endpoint/functions/pci-epf-virtio.c
> @@ -0,0 +1,458 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Helpers to implement PCIe virtio EP function.
> + */
> +#include <linux/virtio_pci.h>
> +#include <linux/virtio_config.h>
> +#include <linux/kthread.h>
> +
> +#include "pci-epf-virtio.h"
> +
> +static void epf_virtio_unmap_vq(struct pci_epf *epf, void __iomem *vq_virt,
> + phys_addr_t vq_phys, unsigned int num)
> +{
> + size_t vq_size = vring_size(num, VIRTIO_PCI_VRING_ALIGN);
> +
> + pci_epc_unmap_addr(epf->epc, epf->func_no, epf->vfunc_no, vq_phys,
> + vq_virt, vq_size);
> + pci_epc_mem_free_addr(epf->epc, vq_phys, vq_virt, vq_size);
> +}
> +
> +static void __iomem *epf_virtio_map_vq(struct pci_epf *epf,
> + phys_addr_t vq_pci_addr,
> + unsigned int num, phys_addr_t *vq_phys)
> +{
> + int err;
> + size_t vq_size;
> + void __iomem *vq_virt;
> +
> + vq_size = vring_size(num, VIRTIO_PCI_VRING_ALIGN);
> +
> + vq_virt = pci_epc_map_addr(epf->epc, epf->func_no, epf->vfunc_no,
> + vq_pci_addr, vq_phys, vq_size);
> + if (IS_ERR(vq_virt)) {
> + pr_err("Failed to map virtuqueue to local");
> + goto err_free;
> + }
> +
> + return vq_virt;
> +
> +err_free:
> + pci_epc_mem_free_addr(epf->epc, *vq_phys, vq_virt, vq_size);
> +
> + return ERR_PTR(err);
> +}
> +
> +static void epf_virtio_free_vringh(struct pci_epf *epf, struct epf_vringh *evrh)
> +{
> + epf_virtio_unmap_vq(epf, evrh->virt, evrh->phys, evrh->num);
> + kfree(evrh);
> +}
> +
> +static struct epf_vringh *epf_virtio_alloc_vringh(struct pci_epf *epf,
> + u64 features,
> + phys_addr_t pci_addr,
> + unsigned int num)
> +{
> + int err;
> + struct vring vring;
> + struct epf_vringh *evrh;
> +
> + evrh = kmalloc(sizeof(*evrh), GFP_KERNEL);
> + if (!evrh) {
> + err = -ENOMEM;
> + goto err_unmap_vq;
> + }
> +
> + evrh->num = num;
> +
> + evrh->virt = epf_virtio_map_vq(epf, pci_addr, num, &evrh->phys);
> + if (IS_ERR(evrh->virt))
> + return evrh->virt;
> +
> + vring_init(&vring, num, evrh->virt, VIRTIO_PCI_VRING_ALIGN);
> +
> + err = vringh_init_iomem(&evrh->vrh, features, num, false, vring.desc,
> + vring.avail, vring.used);
> + if (err)
> + goto err_free_epf_vq;
> +
> + return evrh;
> +
> +err_free_epf_vq:
> + kfree(evrh);
> +
> +err_unmap_vq:
> + epf_virtio_unmap_vq(epf, evrh->virt, evrh->phys, evrh->num);
> +
> + return ERR_PTR(err);
> +}
> +
> +#define VIRTIO_PCI_LEGACY_CFG_BAR 0
> +
> +static void __iomem *epf_virtio_alloc_bar(struct pci_epf *epf, size_t size)
> +{
> + struct pci_epf_bar *config_bar = &epf->bar[VIRTIO_PCI_LEGACY_CFG_BAR];
> + const struct pci_epc_features *features;
> + void __iomem *bar;
> + int err;
> +
> + features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
> + if (!features) {
> + pr_debug("Failed to get PCI EPC features\n");
> + return ERR_PTR(-EOPNOTSUPP);
> + }
> +
> + if (features->reserved_bar & BIT(VIRTIO_PCI_LEGACY_CFG_BAR)) {
> + pr_debug("Cannot use the PCI BAR for legacy virtio pci\n");
> + return ERR_PTR(-EOPNOTSUPP);
Since 1.0 has been used for years, I would suggest starting from a
modern device other than a legacy one. Otherwise you may end up with
some hacky stuffs more below.
> + }
> +
> + if (features->bar_fixed_size[VIRTIO_PCI_LEGACY_CFG_BAR]) {
> + if (size >
> + features->bar_fixed_size[VIRTIO_PCI_LEGACY_CFG_BAR]) {
> + pr_debug("PCI BAR size is not enough\n");
> + return ERR_PTR(-ENOMEM);
> + }
> + }
> +
> + bar = pci_epf_alloc_space(epf, size, VIRTIO_PCI_LEGACY_CFG_BAR,
> + features->align, PRIMARY_INTERFACE);
> + if (!bar) {
> + pr_debug("Failed to allocate virtio-net config memory\n");
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + config_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
This is the tricky part:
1) spec said legacy bar is I/O not memory
2) this code may still work if the host is running with Linux since
the virtio driver work for memory bar
3) but it may not work if the host is not running with Linux
> + err = pci_epc_set_bar(epf->epc, epf->func_no, epf->vfunc_no,
> + config_bar);
> + if (err) {
> + pr_debug("Failed to set PCI BAR");
> + goto err_free_space;
> + }
> +
> + return bar;
> +
> +err_free_space:
> +
> + pci_epf_free_space(epf, bar, VIRTIO_PCI_LEGACY_CFG_BAR,
> + PRIMARY_INTERFACE);
> +
> + return ERR_PTR(err);
> +}
> +
> +static void epf_virtio_free_bar(struct pci_epf *epf, void __iomem *bar)
> +{
> + struct pci_epf_bar *config_bar = &epf->bar[VIRTIO_PCI_LEGACY_CFG_BAR];
> +
> + pci_epc_clear_bar(epf->epc, epf->func_no, epf->vfunc_no, config_bar);
> + pci_epf_free_space(epf, bar, VIRTIO_PCI_LEGACY_CFG_BAR,
> + PRIMARY_INTERFACE);
> +}
> +
> +static void epf_virtio_init_bar(struct epf_virtio *evio, void __iomem *bar)
> +{
> + evio->bar = bar;
> +
> + epf_virtio_cfg_write32(evio, VIRTIO_PCI_HOST_FEATURES, evio->features);
> + epf_virtio_cfg_write16(evio, VIRTIO_PCI_ISR, VIRTIO_PCI_ISR_QUEUE);
> + epf_virtio_cfg_write16(evio, VIRTIO_PCI_QUEUE_NUM, evio->vqlen);
> + epf_virtio_cfg_write16(evio, VIRTIO_PCI_QUEUE_NOTIFY, evio->nvq);
> + epf_virtio_cfg_write8(evio, VIRTIO_PCI_STATUS, 0);
> +}
> +
> +/**
> + * epf_virtio_init - initialize struct epf_virtio and setup BAR for virtio
> + * @evio: struct epf_virtio to initialize.
> + * @hdr: pci configuration space to show remote host.
> + * @bar_size: pci BAR size it depends on the virtio device type.
> + *
> + * Returns zero or a negative error.
> + */
> +int epf_virtio_init(struct epf_virtio *evio, struct pci_epf_header *hdr,
> + size_t bar_size)
> +{
> + struct pci_epf *epf = evio->epf;
> + void __iomem *bar;
> + int err;
> +
> + err = pci_epc_write_header(epf->epc, epf->func_no, epf->vfunc_no, hdr);
> + if (err)
> + return err;
> +
> + bar = epf_virtio_alloc_bar(epf, bar_size);
> + if (IS_ERR(bar))
> + return PTR_ERR(bar);
> +
> + epf_virtio_init_bar(evio, bar);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(epf_virtio_init);
> +
> +/**
> + * epf_virtio_final - finalize struct epf_virtio. it frees bar and memories
> + * @evio: struct epf_virtio to finalize.
> + */
> +void epf_virtio_final(struct epf_virtio *evio)
> +{
> + epf_virtio_free_bar(evio->epf, evio->bar);
> +
> + for (int i = 0; i < evio->nvq; i++)
> + epf_virtio_free_vringh(evio->epf, evio->vrhs[i]);
> +
> + kfree(evio->vrhs);
> +}
> +EXPORT_SYMBOL_GPL(epf_virtio_final);
> +
> +static int epf_virtio_negotiate_vq(struct epf_virtio *evio)
> +{
> + u32 pfn;
> + u16 sel;
> + int i = 0;
> + struct _pair {
> + u32 pfn;
> + u16 sel;
> + } *tmp;
> + int err = 0;
> + size_t nvq = evio->nvq;
> +
> + tmp = kmalloc_array(nvq, sizeof(tmp[0]), GFP_KERNEL);
> + if (!tmp)
> + return -ENOMEM;
> +
> + /*
> + * PCIe EP framework has no capability to hook access PCI BAR space from
> + * remote host driver, so poll the specific register, queue pfn to detect
> + * the writing from the driver.
There were discussions in the past which tried to have a new transport
that works for the endpoint instead of doing tricks like this. Have
you ever considered this?
> + *
> + * This implementation assumes that the address of each virtqueue is
> + * written only once.
> + */
> + for (i = 0; i < nvq; i++) {
> + while (!(pfn = epf_virtio_cfg_read32(evio,
> + VIRTIO_PCI_QUEUE_PFN)) &&
> + evio->running)
> + ;
Should we do cond_resched() here?
> +
> + sel = epf_virtio_cfg_read16(evio, VIRTIO_PCI_QUEUE_SEL);
> +
> + epf_virtio_cfg_write32(evio, VIRTIO_PCI_QUEUE_PFN, 0);
> +
> + tmp[i].pfn = pfn;
> + tmp[i].sel = sel;
> + }
> +
> + if (!evio->running)
> + goto err_out;
> +
> + evio->vrhs = kmalloc_array(nvq, sizeof(evio->vrhs[0]), GFP_KERNEL);
> + if (!evio->vrhs) {
> + err = -ENOMEM;
> + goto err_out;
> + }
> +
> + for (i = 0; i < nvq; i++) {
> + phys_addr_t pci = tmp[i].pfn << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
> +
> + evio->vrhs[i] = epf_virtio_alloc_vringh(
> + evio->epf, evio->features, pci, evio->vqlen);
> + if (IS_ERR(evio->vrhs[i])) {
> + err = PTR_ERR(evio->vrhs[i]);
> + goto err_free_evrhs;
> + }
> + }
> +
> + kfree(tmp);
> +
> + return 0;
> +
> +err_free_evrhs:
> + for (i -= 1; i > 0; i--)
> + epf_virtio_free_vringh(evio->epf, evio->vrhs[i]);
> +
> + kfree(evio->vrhs);
> +
> +err_out:
> + kfree(tmp);
> +
> + return err;
> +}
> +
> +static void epf_virtio_monitor_qnotify(struct epf_virtio *evio)
> +{
> + const u16 qn_default = evio->nvq;
> + u16 tmp;
> +
> + /* Since there is no way to synchronize between the host and EP functions,
> + * it is possible to miss multiple notifications.
> + */
> + while (evio->running) {
> + tmp = epf_virtio_cfg_read16(evio, VIRTIO_PCI_QUEUE_NOTIFY);
> + if (tmp == qn_default)
> + continue;
cond_resched()?
> +
> + epf_virtio_cfg_write16(evio, VIRTIO_PCI_QUEUE_NOTIFY,
> + qn_default);
> +
> + evio->qn_callback(evio->qn_param);
> + }
> +}
> +
> +static int epf_virtio_bgtask(void *param)
> +{
> + struct epf_virtio *evio = param;
> + int err;
> +
> + err = epf_virtio_negotiate_vq(evio);
> + if (err < 0) {
> + pr_err("failed to negoticate configs with driver\n");
> + return err;
> + }
> +
> + while (!(epf_virtio_cfg_read8(evio, VIRTIO_PCI_STATUS) &
> + VIRTIO_CONFIG_S_DRIVER_OK) &&
> + evio->running)
> + ;
> +
> + if (evio->ic_callback && evio->running)
> + evio->ic_callback(evio->ic_param);
> +
> + epf_virtio_monitor_qnotify(evio);
> +
> + return 0;
> +}
> +
> +/**
> + * epf_virtio_launch_bgtask - spawn a kthread that emulates virtio device
> + * operations.
> + * @evio: It should be initialized prior with epf_virtio_init().
> + *
> + * Returns zero or a negative error.
> + */
> +int epf_virtio_launch_bgtask(struct epf_virtio *evio)
> +{
> + evio->bgtask = kthread_create(epf_virtio_bgtask, evio,
> + "pci-epf-virtio/bgtask");
> + if (IS_ERR(evio->bgtask))
> + return PTR_ERR(evio->bgtask);
> +
> + evio->running = true;
> +
> + sched_set_fifo(evio->bgtask);
> + wake_up_process(evio->bgtask);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(epf_virtio_launch_bgtask);
> +
> +/**
> + * epf_virtio_terminate_bgtask - shutdown a device emulation kthread.
> + * @evio: struct epf_virtio it already launched bgtask.
> + */
> +void epf_virtio_terminate_bgtask(struct epf_virtio *evio)
> +{
> + evio->running = false;
> +
> + kthread_stop(evio->bgtask);
> +}
> +EXPORT_SYMBOL_GPL(epf_virtio_terminate_bgtask);
> +
> +/**
> + * epf_virtio_reset - reset virtio status
> + * @evio: struct epf_virtio to reset
> + *
> + * Returns zero or a negative error.
> + */
> +int epf_virtio_reset(struct epf_virtio *evio)
> +{
> + epf_virtio_terminate_bgtask(evio);
> + epf_virtio_init_bar(evio, evio->bar);
> +
> + return epf_virtio_launch_bgtask(evio);
> +}
> +EXPORT_SYMBOL_GPL(epf_virtio_reset);
> +
> +int epf_virtio_getdesc(struct epf_virtio *evio, int index,
> + struct vringh_kiov *riov, struct vringh_kiov *wiov,
> + u16 *head)
> +{
> + struct vringh *vrh = &evio->vrhs[index]->vrh;
> +
> + return vringh_getdesc_iomem(vrh, riov, wiov, head, GFP_KERNEL);
> +}
> +
> +void epf_virtio_abandon(struct epf_virtio *evio, int index, int num)
> +{
> + struct vringh *vrh = &evio->vrhs[index]->vrh;
> +
> + vringh_abandon_iomem(vrh, num);
> +}
> +
> +void epf_virtio_iov_complete(struct epf_virtio *evio, int index, u16 head,
> + size_t total_len)
> +{
> + struct vringh *vrh = &evio->vrhs[index]->vrh;
> +
> + vringh_complete_iomem(vrh, head, total_len);
> +}
> +
> +int epf_virtio_memcpy_kiov2kiov(struct epf_virtio *evio,
> + struct vringh_kiov *siov,
> + struct vringh_kiov *diov,
> + enum dma_transfer_direction dir)
> +{
> + struct pci_epf *epf = evio->epf;
> + size_t slen, dlen;
> + u64 sbase, dbase;
> + phys_addr_t phys;
> + void *dst, *src;
> +
> + for (; siov->i < siov->used; siov->i++, diov->i++) {
> + slen = siov->iov[siov->i].iov_len;
> + sbase = (u64)siov->iov[siov->i].iov_base;
> + dlen = diov->iov[diov->i].iov_len;
> + dbase = (u64)diov->iov[diov->i].iov_base;
> +
> + if (dlen < slen) {
> + pr_info("not enough buffer\n");
> + return -EINVAL;
> + }
> +
> + if (dir == DMA_MEM_TO_DEV) {
> + src = phys_to_virt(sbase);
> +
> + dst = pci_epc_map_addr(epf->epc, epf->func_no,
> + epf->vfunc_no, dbase, &phys,
> + slen);
> + if (IS_ERR(dst)) {
> + pr_err("failed to map pci mmoery spact to local\n");
Typos.
> + return PTR_ERR(dst);
> + }
> + } else {
> + src = pci_epc_map_addr(epf->epc, epf->func_no,
> + epf->vfunc_no, sbase, &phys,
> + slen);
> + if (IS_ERR(src)) {
> + pr_err("failed to map pci mmoery spact to local\n");
Typos.
Thanks
On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
>
> Already it has beed defined a config changed flag of ISR, but not the queue
Typo should be "been".
> flag. Add a macro for it.
>
> Signed-off-by: Shunsuke Mie <[email protected]>
Other than this,
Acked-by: Jason Wang <[email protected]>
> ---
> include/uapi/linux/virtio_pci.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> index f703afc7ad31..d405bea27240 100644
> --- a/include/uapi/linux/virtio_pci.h
> +++ b/include/uapi/linux/virtio_pci.h
> @@ -94,6 +94,9 @@
>
> #endif /* VIRTIO_PCI_NO_LEGACY */
>
> +/* Bits for ISR status field:only when if MSI-X is disabled */
> +/* The bit of the ISR which indicates a queue entry update. */
> +#define VIRTIO_PCI_ISR_QUEUE 0x1
> /* The bit of the ISR which indicates a device configuration change. */
> #define VIRTIO_PCI_ISR_CONFIG 0x2
> /* Vector value used to disable MSI for queue */
> --
> 2.25.1
>
On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
>
> Add a new PCIe endpoint function driver that works as a pci virtio-console
> device. The console connect to endpoint side console. It enables to
> communicate PCIe host and endpoint.
>
> Architecture is following:
>
> ┌────────────┐ ┌──────────────────────┬────────────┐
> │virtioe │ │ │virtio │
> │console drv │ ├───────────────┐ │console drv │
> ├────────────┤ │(virtio console│ ├────────────┤
> │ virtio bus │ │ device) │◄────►│ virtio bus │
> ├────────────┤ ├---------------┤ └────────────┤
> │ │ │ pci ep virtio │ │
> │ pci bus │ │ console drv │ │
> │ │ pcie ├───────────────┤ │
> │ │ ◄─────► │ pci ep Bus │ │
> └────────────┘ └───────────────┴───────────────────┘
> PCIe Root PCIe Endpoint
>
I think it might only works for peer devices like:
net, console or vsock.
So there're many choices here, I'd like to know what's the reason for
you to implement a mediation.
An alternative is to implement a dedicated net, console and vsock
driver for vringh (CAIF somehow works like this). This would have
better performance.
> This driver has two roles. The first is as a PCIe endpoint virtio console
> function, which is implemented using the PCIe endpoint framework and PCIe
> EP virtio helpers. The second is as a virtual virtio console device
> connected to the virtio bus on PCIe endpoint Linux.
>
> Communication between the two is achieved by copying the virtqueue data
> between PCIe root and endpoint, respectively.
>
> This is a simple implementation and does not include features of
> virtio-console such as MULTIPORT, EMERG_WRITE, etc. As a result, each
> virtio console driver only displays /dev/hvc0.
>
> As an example of usage, by setting getty to /dev/hvc0, it is possible to
> login to another host.
>
> Signed-off-by: Shunsuke Mie <[email protected]>
> ---
> Changes from v2:
> - Change to use copy functions between kiovs of pci-epf-virtio.
>
> drivers/pci/endpoint/functions/Kconfig | 12 +
> drivers/pci/endpoint/functions/Makefile | 1 +
> drivers/pci/endpoint/functions/pci-epf-vcon.c | 596 ++++++++++++++++++
> 3 files changed, 609 insertions(+)
> create mode 100644 drivers/pci/endpoint/functions/pci-epf-vcon.c
>
> diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
> index fa1a6a569a8f..9ce2698b67e1 100644
> --- a/drivers/pci/endpoint/functions/Kconfig
> +++ b/drivers/pci/endpoint/functions/Kconfig
> @@ -44,3 +44,15 @@ config PCI_EPF_VIRTIO
> select VHOST_RING_IOMEM
> help
> Helpers to implement PCI virtio Endpoint function
> +
> +config PCI_EPF_VCON
> + tristate "PCI Endpoint virito-console driver"
> + depends on PCI_ENDPOINT
> + select VHOST_RING
> + select PCI_EPF_VIRTIO
> + help
> + PCIe Endpoint virtio-console function implementatino. This module
> + enables to show the virtio-console as pci device to PCIe host side, and
> + another virtual virtio-console device registers to endpoint system.
> + Those devices are connected virtually and can communicate each other.
> +
> diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
> index a96f127ce900..b4056689ce33 100644
> --- a/drivers/pci/endpoint/functions/Makefile
> +++ b/drivers/pci/endpoint/functions/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
> obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
> obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
> obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
> +obj-$(CONFIG_PCI_EPF_VCON) += pci-epf-vcon.o
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vcon.c b/drivers/pci/endpoint/functions/pci-epf-vcon.c
> new file mode 100644
> index 000000000000..31f4247cd10f
> --- /dev/null
> +++ b/drivers/pci/endpoint/functions/pci-epf-vcon.c
> @@ -0,0 +1,596 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCI Endpoint function driver to impliment virtio-console device
> + * functionality.
> + */
> +#include <linux/pci-epf.h>
> +#include <linux/virtio_ids.h>
> +#include <linux/virtio_pci.h>
> +#include <linux/virtio_console.h>
> +#include <linux/virtio_ring.h>
> +
> +#include "pci-epf-virtio.h"
> +
> +static int virtio_queue_size = 0x100;
> +module_param(virtio_queue_size, int, 0444);
> +MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
> +
> +struct epf_vcon {
> + /* To access virtqueues on remote host */
> + struct epf_virtio evio;
> + struct vringh_kiov *rdev_iovs;
> +
> + /* To register a local virtio bus */
> + struct virtio_device vdev;
> +
> + /* To access virtqueus of local host driver */
> + struct vringh *vdev_vrhs;
> + struct vringh_kiov *vdev_iovs;
> + struct virtqueue **vdev_vqs;
> +
> + /* For transportation and notification */
> + struct workqueue_struct *task_wq;
> + struct work_struct raise_irq_work, rx_work, tx_work;
> +
> + /* To retain virtio features. It is commonly used local and remote. */
> + u64 features;
> +
> + /* To show a status whether this driver is ready and the remote is connected */
> + bool connected;
> +};
> +
> +enum {
> + VCON_VIRTQUEUE_RX,
> + VCON_VIRTQUEUE_TX,
> + // Should be end of enum
> + VCON_VIRTQUEUE_NUM
> +};
It would be better if we can split the console specific thing out,
then it allows us to do ethernet and vsock in the future.
Thanks
Hi Bjorn,
Thanks for the many comments. I will fix the mannerisms and typos as noted.
2023年4月28日(金) 3:09 Bjorn Helgaas <[email protected]>:
>
> Random typos and trivial things. No need to repost until somebody
> does a more substantive review.
>
> On Thu, Apr 27, 2023 at 07:44:28PM +0900, Shunsuke Mie wrote:
> > Add a new PCIe endpoint function driver that works as a pci virtio-console
> > device. The console connect to endpoint side console. It enables to
> > communicate PCIe host and endpoint.
>
> s/pci/PCI/
>
> > Architecture is following:
> >
> > ┌────────────┐ ┌──────────────────────┬────────────┐
> > │virtioe │ │ │virtio │
> > │console drv │ ├───────────────┐ │console drv │
> > ├────────────┤ │(virtio console│ ├────────────┤
> > │ virtio bus │ │ device) │◄────►│ virtio bus │
> > ├────────────┤ ├---------------┤ └────────────┤
> > │ │ │ pci ep virtio │ │
> > │ pci bus │ │ console drv │ │
> > │ │ pcie ├───────────────┤ │
> > │ │ ◄─────► │ pci ep Bus │ │
> > └────────────┘ └───────────────┴───────────────────┘
> > PCIe Root PCIe Endpoint
>
> s/virtioe/virtio/
> s/pci/PCI/
> s/pcie/PCIe/
> s/ep/EP/
>
> > +config PCI_EPF_VCON
> > + tristate "PCI Endpoint virito-console driver"
>
> s/virito/virtio/
>
> > + depends on PCI_ENDPOINT
> > + select VHOST_RING
> > + select PCI_EPF_VIRTIO
> > + help
> > + PCIe Endpoint virtio-console function implementatino. This module
> > + enables to show the virtio-console as pci device to PCIe host side, and
> > + another virtual virtio-console device registers to endpoint system.
> > + Those devices are connected virtually and can communicate each other.
>
> s/implementatino/implementation/
> s/pci/PCI/
> s/can communicate/can communicate with/
>
> > + * PCI Endpoint function driver to impliment virtio-console device
> > + * functionality.
>
> s/impliment/implement/
>
> > +static int virtio_queue_size = 0x100;
> > +module_param(virtio_queue_size, int, 0444);
> > +MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
>
> When and why would users need this parameter? Where is it documented?
>
> > + /* To access virtqueus of local host driver */
>
> s/virtqueus/virtqueues/
>
> > + /* To show a status whether this driver is ready and the remote is connected */
>
> Make this fit in 80 columns.
>
> > + /* This is a minimum implementation. Doesn't support any features of
> > + * virtio console. It means driver and device use just 2 virtuque for tx
> > + * and rx.
> > + */
>
> Use common multi-line comment style:
>
> /*
> * This is ...
> */
I'll follow the style.
> s/virtuque/virtqueues/
>
> > +static void epf_vcon_raise_irq_handler(struct work_struct *work)
> > +{
> > + struct epf_vcon *vcon =
> > + container_of(work, struct epf_vcon, raise_irq_work);
>
> Rewrap.
>
> > +static int epf_vcon_setup_common(struct epf_vcon *vcon)
> > +{
> > + vcon->features = 0;
> > + vcon->connected = false;
> > +
> > + vcon->task_wq =
> > + alloc_workqueue("pci-epf-vcon/task-wq",
>
> Looks like this would fit on the previous line?
>
> > + WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND, 0);
>
> > +static void epf_vcon_initialize_complete(void *param)
> > +{
> > + struct epf_vcon *vcon = param;
> > +
> > + pr_debug("Remote host has connected\n");
>
> Is there any device info you could include here, e.g., with dev_dbg()?
> It's nice if users have a little context.
I see. I'll use it.
> > +static int epf_vcon_setup_ep_func(struct epf_vcon *vcon, struct pci_epf *epf)
> > +{
> > + int err;
> > + struct epf_virtio *evio = &vcon->evio;
> > + unsigned int nvq = epf_vcon_get_nvq(vcon);
> > +
> > + vcon->rdev_iovs =
> > + kmalloc_array(nvq, sizeof(vcon->rdev_iovs[0]), GFP_KERNEL);
>
> Move the function name and as many parameters as fit in 80 columns to
> the previous line to match prevailing style.
I've just applied clang-format... Ok, I'll fix it manually.
> > + /* There is no config for virtio console because this console device
> > + * doesn't any support features
> > + */
>
> Multi-line comment style.
>
> s/doesn't any support/doesn't support any/? Is that what you mean?
>
> > + /* Do nothing because this console device doesn't any support features */
>
> Same.
>
> > +static void epf_vcon_vdev_set_status(struct virtio_device *vdev, u8 status)
> > +{
> > + if (status & VIRTIO_CONFIG_S_FAILED)
> > + pr_debug("driver failed to setup this device\n");
>
> dev_dbg() if possible.
>
> > + err = vringh_init_kern(&vcon->vdev_vrhs[i], vcon->features,
> > + virtio_queue_size, false, vring->desc,
> > + vring->avail, vring->used);
> > + if (err) {
> > + pr_err("failed to init vringh for vring %d\n", i);
>
> dev_err() if possible.
>
> > +static int epf_vcon_setup_vdev(struct epf_vcon *vcon, struct device *parent)
> > +{
> > + int err;
> > + struct virtio_device *vdev = &vcon->vdev;
> > + const unsigned int nvq = epf_vcon_get_nvq(vcon);
> > +
> > + vcon->vdev_vrhs =
> > + kmalloc_array(nvq, sizeof(vcon->vdev_vrhs[0]), GFP_KERNEL);
>
> Rewrap.
>
> > + vcon->vdev_iovs =
> > + kmalloc_array(nvq, sizeof(vcon->vdev_iovs[0]), GFP_KERNEL);
>
> Rewrap.
>
> > + vcon->vdev_vqs =
> > + kmalloc_array(nvq, sizeof(vcon->vdev_vrhs[0]), GFP_KERNEL);
>
> Rewrap.
>
> > +static void epf_vcon_cleanup_vdev(struct epf_vcon *vcon)
> > +{
> > + unregister_virtio_device(&vcon->vdev);
> > + /* Cleanup struct virtio_device that has kobject, otherwise error occures when
> > + * reregister the virtio device.
> > + */
>
> Multi-line style and rewrap to fit in 80 columns.
>
> > +static int __init epf_vcon_init(void)
> > +{
> > + int err;
> > +
> > + err = pci_epf_register_driver(&epf_vcon_drv);
> > + if (err)
> > + pr_err("Failed to register PCI EP virtio-console function\n");
>
> dev_err() if possible (doesn't look like it *is* possible).
>
> Looks like this registers a *driver*, so maybe change the message from
> "function" to "driver"?
It should be "driver".
https://docs.kernel.org/PCI/endpoint/pci-endpoint.html#epc-apis-for-the-pci-endpoint-function-driver
> Bjorn
Best regards,
Shunsuke
Hi Json,
2023年5月8日(月) 13:03 Jason Wang <[email protected]>:
>
> On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
> >
> > Add a new PCIe endpoint function driver that works as a pci virtio-console
> > device. The console connect to endpoint side console. It enables to
> > communicate PCIe host and endpoint.
> >
> > Architecture is following:
> >
> > ┌────────────┐ ┌──────────────────────┬────────────┐
> > │virtioe │ │ │virtio │
> > │console drv │ ├───────────────┐ │console drv │
> > ├────────────┤ │(virtio console│ ├────────────┤
> > │ virtio bus │ │ device) │◄────►│ virtio bus │
> > ├────────────┤ ├---------------┤ └────────────┤
> > │ │ │ pci ep virtio │ │
> > │ pci bus │ │ console drv │ │
> > │ │ pcie ├───────────────┤ │
> > │ │ ◄─────► │ pci ep Bus │ │
> > └────────────┘ └───────────────┴───────────────────┘
> > PCIe Root PCIe Endpoint
> >
>
> I think it might only works for peer devices like:
>
> net, console or vsock.
Could you tell me what "peer devices" means?
> So there're many choices here, I'd like to know what's the reason for
> you to implement a mediation.
>
> An alternative is to implement a dedicated net, console and vsock
> driver for vringh (CAIF somehow works like this). This would have
> better performance.
Does it mean that the driver also functions as a network driver directly?
>
>
> > This driver has two roles. The first is as a PCIe endpoint virtio console
> > function, which is implemented using the PCIe endpoint framework and PCIe
> > EP virtio helpers. The second is as a virtual virtio console device
> > connected to the virtio bus on PCIe endpoint Linux.
> >
> > Communication between the two is achieved by copying the virtqueue data
> > between PCIe root and endpoint, respectively.
> >
> > This is a simple implementation and does not include features of
> > virtio-console such as MULTIPORT, EMERG_WRITE, etc. As a result, each
> > virtio console driver only displays /dev/hvc0.
> >
> > As an example of usage, by setting getty to /dev/hvc0, it is possible to
> > login to another host.
> >
> > Signed-off-by: Shunsuke Mie <[email protected]>
> > ---
> > Changes from v2:
> > - Change to use copy functions between kiovs of pci-epf-virtio.
> >
> > drivers/pci/endpoint/functions/Kconfig | 12 +
> > drivers/pci/endpoint/functions/Makefile | 1 +
> > drivers/pci/endpoint/functions/pci-epf-vcon.c | 596 ++++++++++++++++++
> > 3 files changed, 609 insertions(+)
> > create mode 100644 drivers/pci/endpoint/functions/pci-epf-vcon.c
> >
> > diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
> > index fa1a6a569a8f..9ce2698b67e1 100644
> > --- a/drivers/pci/endpoint/functions/Kconfig
> > +++ b/drivers/pci/endpoint/functions/Kconfig
> > @@ -44,3 +44,15 @@ config PCI_EPF_VIRTIO
> > select VHOST_RING_IOMEM
> > help
> > Helpers to implement PCI virtio Endpoint function
> > +
> > +config PCI_EPF_VCON
> > + tristate "PCI Endpoint virito-console driver"
> > + depends on PCI_ENDPOINT
> > + select VHOST_RING
> > + select PCI_EPF_VIRTIO
> > + help
> > + PCIe Endpoint virtio-console function implementatino. This module
> > + enables to show the virtio-console as pci device to PCIe host side, and
> > + another virtual virtio-console device registers to endpoint system.
> > + Those devices are connected virtually and can communicate each other.
> > +
> > diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
> > index a96f127ce900..b4056689ce33 100644
> > --- a/drivers/pci/endpoint/functions/Makefile
> > +++ b/drivers/pci/endpoint/functions/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
> > obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
> > obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
> > obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
> > +obj-$(CONFIG_PCI_EPF_VCON) += pci-epf-vcon.o
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vcon.c b/drivers/pci/endpoint/functions/pci-epf-vcon.c
> > new file mode 100644
> > index 000000000000..31f4247cd10f
> > --- /dev/null
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vcon.c
> > @@ -0,0 +1,596 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PCI Endpoint function driver to impliment virtio-console device
> > + * functionality.
> > + */
> > +#include <linux/pci-epf.h>
> > +#include <linux/virtio_ids.h>
> > +#include <linux/virtio_pci.h>
> > +#include <linux/virtio_console.h>
> > +#include <linux/virtio_ring.h>
> > +
> > +#include "pci-epf-virtio.h"
> > +
> > +static int virtio_queue_size = 0x100;
> > +module_param(virtio_queue_size, int, 0444);
> > +MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
> > +
> > +struct epf_vcon {
> > + /* To access virtqueues on remote host */
> > + struct epf_virtio evio;
> > + struct vringh_kiov *rdev_iovs;
> > +
> > + /* To register a local virtio bus */
> > + struct virtio_device vdev;
> > +
> > + /* To access virtqueus of local host driver */
> > + struct vringh *vdev_vrhs;
> > + struct vringh_kiov *vdev_iovs;
> > + struct virtqueue **vdev_vqs;
> > +
> > + /* For transportation and notification */
> > + struct workqueue_struct *task_wq;
> > + struct work_struct raise_irq_work, rx_work, tx_work;
> > +
> > + /* To retain virtio features. It is commonly used local and remote. */
> > + u64 features;
> > +
> > + /* To show a status whether this driver is ready and the remote is connected */
> > + bool connected;
> > +};
> > +
> > +enum {
> > + VCON_VIRTQUEUE_RX,
> > + VCON_VIRTQUEUE_TX,
> > + // Should be end of enum
> > + VCON_VIRTQUEUE_NUM
> > +};
>
> It would be better if we can split the console specific thing out,
> then it allows us to do ethernet and vsock in the future.
I'm planning to implement each virtio device in a separate file.
https://lwn.net/Articles/922124/
> Thanks
>
Best regards,
Shunsuke
2023年5月8日(月) 12:59 Jason Wang <[email protected]>:
>
> On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
> >
> > Already it has beed defined a config changed flag of ISR, but not the queue
>
> Typo should be "been".
Thanks for pointing that out.
> > flag. Add a macro for it.
> >
> > Signed-off-by: Shunsuke Mie <[email protected]>
>
> Other than this,
>
> Acked-by: Jason Wang <[email protected]>
>
> > ---
> > include/uapi/linux/virtio_pci.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> > index f703afc7ad31..d405bea27240 100644
> > --- a/include/uapi/linux/virtio_pci.h
> > +++ b/include/uapi/linux/virtio_pci.h
> > @@ -94,6 +94,9 @@
> >
> > #endif /* VIRTIO_PCI_NO_LEGACY */
> >
> > +/* Bits for ISR status field:only when if MSI-X is disabled */
> > +/* The bit of the ISR which indicates a queue entry update. */
> > +#define VIRTIO_PCI_ISR_QUEUE 0x1
> > /* The bit of the ISR which indicates a device configuration change. */
> > #define VIRTIO_PCI_ISR_CONFIG 0x2
> > /* Vector value used to disable MSI for queue */
> > --
> > 2.25.1
> >
>
I'll fix the typo and some styles you mentioned.
In this e-mail, I reply to the other comments.
2023年4月28日(金) 3:28 Bjorn Helgaas <[email protected]>:
>
> Simple typos, don't repost until there's more substantive feedback.
>
> On Thu, Apr 27, 2023 at 07:44:26PM +0900, Shunsuke Mie wrote:
> > The Linux PCIe Endpoint framework supports to implement PCIe endpoint
> > functions using a PCIe controller operating in endpoint mode.
> > It is possble to realize the behavior of PCIe device, such as virtio PCI
> > device. This patch introduces a setof helper functions and data structures
> > to implement a PCIe endpoint function that behaves as a virtio device.
>
> s/possble/possible/
> s/setof/set of/
>
> > Those functions enable the implementation PCIe endpoint function that
> > comply with the virtio lecacy specification. Because modern virtio
> > specifications require devices to implement custom PCIe capabilities, which
> > are not currently supported by either PCIe controllers/drivers or the PCIe
> > endpoint framework.
>
> s/implementation PCIe endpoint function/implementation of PCIe endpoint functions/
> s/lecacy/legacy/ (What does "legacy" mean? Is there a spec for this?)
Yes, it is defined in spec.
> I guess "legacy virtio" devices need not implement custom PCIe
> capabilities, but "modern virtio" devices must implement them?
That I wanted to explain here.
> Capitalize "Endpoint framework" consistently; sometimes it's
> "Endpoint", other times it's "endpoint".
I'll take care to be consistent.
> > While this patch provides functions for negotiating with host drivers and
> > copying data, each PCIe function driver must impl ement operations that
> > depend on each specific device, such as network, block, etc.
>
> s/impl ement/implement/
>
> > +#include <linux/virtio_pci.h>
> > +#include <linux/virtio_config.h>
> > +#include <linux/kthread.h>
>
> Typically the header includes would be alphabetized if possible.
I'll try to reorder them.
> > + vq_virt = pci_epc_map_addr(epf->epc, epf->func_no, epf->vfunc_no,
> > + vq_pci_addr, vq_phys, vq_size);
> > + if (IS_ERR(vq_virt)) {
> > + pr_err("Failed to map virtuqueue to local");
>
> s/virtuqueue/virtqueue/
>
> I know you probably don't have any way to use dev_err(), but this
> message really needs some context, like a driver name and instance or
> something.
I'll try to use the dev_* function appropriately If possible.
> > +#define VIRTIO_PCI_LEGACY_CFG_BAR 0
>
> What makes this a "legacy cfg BAR"? Is there some spec that covers
> virtio BAR usage?
Yes. Virtio Legacy interface defines the PCI BAR number to use.
https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-14500010
> > + * epf_virtio_init - initialize struct epf_virtio and setup BAR for virtio
> > + * @evio: struct epf_virtio to initialize.
> > + * @hdr: pci configuration space to show remote host.
> > + * @bar_size: pci BAR size it depends on the virtio device type.
>
> s/pci/PCI/ (there were also a few more instances above in messages or
> comments)
>
> > + * epf_virtio_final - finalize struct epf_virtio. it frees bar and memories
> > + * @evio: struct epf_virtio to finalize.
>
> s/bar/BAR/
>
> > +static void epf_virtio_monitor_qnotify(struct epf_virtio *evio)
> > +{
> > + const u16 qn_default = evio->nvq;
> > + u16 tmp;
> > +
> > + /* Since there is no way to synchronize between the host and EP functions,
> > + * it is possible to miss multiple notifications.
>
> Multi-line comment style.
>
> > + err = epf_virtio_negotiate_vq(evio);
> > + if (err < 0) {
> > + pr_err("failed to negoticate configs with driver\n");
>
> s/negoticate/negotiate/
>
> > + * epf_virtio_reset - reset virtio status
>
> Some of the function descriptions end with a period (".") and others
> don't. Please figure out what the most common style is and use that
> consistently.
I agree. I'll fix to be consistent.
> > + dst = pci_epc_map_addr(epf->epc, epf->func_no,
> > + epf->vfunc_no, dbase, &phys,
> > + slen);
> > + if (IS_ERR(dst)) {
> > + pr_err("failed to map pci mmoery spact to local\n");
>
> s/pci/PCI/
> s/mmoery/memory/
> s/spact/space/ ?
>
> Also below.
>
> IIRC some previous messages started with a capital letter. Please
> make them all consistent.
Sure.
> > + if (dir == DMA_MEM_TO_DEV) {
> > + pci_epc_unmap_addr(epf->epc, epf->func_no,
> > + epf->vfunc_no, phys, dst, slen);
> > + } else {
> > + pci_epc_unmap_addr(epf->epc, epf->func_no,
> > + epf->vfunc_no, phys, src, slen);
> > + }
> > + }
> > +
> > + return 1;
>
> I guess this function returns either a negative error code or the
> value 1? That seems sort of weird (I think "negative error code or
> *zero* is more typical), but maybe you're following some convention?
It has to be 0. It is my mistake.
Some vringh functions return 0 (data does not exist), 1 (it exists),
or error. But this functions is not related.
> > +#include <linux/pci-epf.h>
> > +#include <linux/pci-epc.h>
> > +#include <linux/vringh.h>
> > +#include <linux/dmaengine.h>
>
> Alpha order if possible
Yes.
> > + /* Virtual address of pci configuration space */
>
> s/pci/PCI/
>
> > + /* Callback function and parameter for queue notifcation
> > + * Note: PCI EP function cannot detect qnotify accurately, therefore this
> > + * callback function should check all of virtqueue's changes.
> > + */
>
> Multi-line comment style.
>
> Bjorn
Best regards,
Shunsuke
Gentle ping ...
Thanks,
Shunsuke.
On 2023/05/10 12:17, Shunsuke Mie wrote:
> Hi Json,
> 2023年5月8日(月) 13:03 Jason Wang <[email protected]>:
>> On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
>>> Add a new PCIe endpoint function driver that works as a pci virtio-console
>>> device. The console connect to endpoint side console. It enables to
>>> communicate PCIe host and endpoint.
>>>
>>> Architecture is following:
>>>
>>> ┌────────────┐ ┌──────────────────────┬────────────┐
>>> │virtioe │ │ │virtio │
>>> │console drv │ ├───────────────┐ │console drv │
>>> ├────────────┤ │(virtio console│ ├────────────┤
>>> │ virtio bus │ │ device) │◄────►│ virtio bus │
>>> ├────────────┤ ├---------------┤ └────────────┤
>>> │ │ │ pci ep virtio │ │
>>> │ pci bus │ │ console drv │ │
>>> │ │ pcie ├───────────────┤ │
>>> │ │ ◄─────► │ pci ep Bus │ │
>>> └────────────┘ └───────────────┴───────────────────┘
>>> PCIe Root PCIe Endpoint
>>>
>> I think it might only works for peer devices like:
>>
>> net, console or vsock.
> Could you tell me what "peer devices" means?
>
>> So there're many choices here, I'd like to know what's the reason for
>> you to implement a mediation.
>>
>> An alternative is to implement a dedicated net, console and vsock
>> driver for vringh (CAIF somehow works like this). This would have
>> better performance.
> Does it mean that the driver also functions as a network driver directly?
>>
>>> This driver has two roles. The first is as a PCIe endpoint virtio console
>>> function, which is implemented using the PCIe endpoint framework and PCIe
>>> EP virtio helpers. The second is as a virtual virtio console device
>>> connected to the virtio bus on PCIe endpoint Linux.
>>>
>>> Communication between the two is achieved by copying the virtqueue data
>>> between PCIe root and endpoint, respectively.
>>>
>>> This is a simple implementation and does not include features of
>>> virtio-console such as MULTIPORT, EMERG_WRITE, etc. As a result, each
>>> virtio console driver only displays /dev/hvc0.
>>>
>>> As an example of usage, by setting getty to /dev/hvc0, it is possible to
>>> login to another host.
>>>
>>> Signed-off-by: Shunsuke Mie <[email protected]>
>>> ---
>>> Changes from v2:
>>> - Change to use copy functions between kiovs of pci-epf-virtio.
>>>
>>> drivers/pci/endpoint/functions/Kconfig | 12 +
>>> drivers/pci/endpoint/functions/Makefile | 1 +
>>> drivers/pci/endpoint/functions/pci-epf-vcon.c | 596 ++++++++++++++++++
>>> 3 files changed, 609 insertions(+)
>>> create mode 100644 drivers/pci/endpoint/functions/pci-epf-vcon.c
>>>
>>> diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
>>> index fa1a6a569a8f..9ce2698b67e1 100644
>>> --- a/drivers/pci/endpoint/functions/Kconfig
>>> +++ b/drivers/pci/endpoint/functions/Kconfig
>>> @@ -44,3 +44,15 @@ config PCI_EPF_VIRTIO
>>> select VHOST_RING_IOMEM
>>> help
>>> Helpers to implement PCI virtio Endpoint function
>>> +
>>> +config PCI_EPF_VCON
>>> + tristate "PCI Endpoint virito-console driver"
>>> + depends on PCI_ENDPOINT
>>> + select VHOST_RING
>>> + select PCI_EPF_VIRTIO
>>> + help
>>> + PCIe Endpoint virtio-console function implementatino. This module
>>> + enables to show the virtio-console as pci device to PCIe host side, and
>>> + another virtual virtio-console device registers to endpoint system.
>>> + Those devices are connected virtually and can communicate each other.
>>> +
>>> diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
>>> index a96f127ce900..b4056689ce33 100644
>>> --- a/drivers/pci/endpoint/functions/Makefile
>>> +++ b/drivers/pci/endpoint/functions/Makefile
>>> @@ -7,3 +7,4 @@ obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
>>> obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
>>> obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
>>> obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
>>> +obj-$(CONFIG_PCI_EPF_VCON) += pci-epf-vcon.o
>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-vcon.c b/drivers/pci/endpoint/functions/pci-epf-vcon.c
>>> new file mode 100644
>>> index 000000000000..31f4247cd10f
>>> --- /dev/null
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-vcon.c
>>> @@ -0,0 +1,596 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * PCI Endpoint function driver to impliment virtio-console device
>>> + * functionality.
>>> + */
>>> +#include <linux/pci-epf.h>
>>> +#include <linux/virtio_ids.h>
>>> +#include <linux/virtio_pci.h>
>>> +#include <linux/virtio_console.h>
>>> +#include <linux/virtio_ring.h>
>>> +
>>> +#include "pci-epf-virtio.h"
>>> +
>>> +static int virtio_queue_size = 0x100;
>>> +module_param(virtio_queue_size, int, 0444);
>>> +MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
>>> +
>>> +struct epf_vcon {
>>> + /* To access virtqueues on remote host */
>>> + struct epf_virtio evio;
>>> + struct vringh_kiov *rdev_iovs;
>>> +
>>> + /* To register a local virtio bus */
>>> + struct virtio_device vdev;
>>> +
>>> + /* To access virtqueus of local host driver */
>>> + struct vringh *vdev_vrhs;
>>> + struct vringh_kiov *vdev_iovs;
>>> + struct virtqueue **vdev_vqs;
>>> +
>>> + /* For transportation and notification */
>>> + struct workqueue_struct *task_wq;
>>> + struct work_struct raise_irq_work, rx_work, tx_work;
>>> +
>>> + /* To retain virtio features. It is commonly used local and remote. */
>>> + u64 features;
>>> +
>>> + /* To show a status whether this driver is ready and the remote is connected */
>>> + bool connected;
>>> +};
>>> +
>>> +enum {
>>> + VCON_VIRTQUEUE_RX,
>>> + VCON_VIRTQUEUE_TX,
>>> + // Should be end of enum
>>> + VCON_VIRTQUEUE_NUM
>>> +};
>> It would be better if we can split the console specific thing out,
>> then it allows us to do ethernet and vsock in the future.
> I'm planning to implement each virtio device in a separate file.
> https://lwn.net/Articles/922124/
>
>
>
>> Thanks
>>
> Best regards,
> Shunsuke
On Thu, May 18, 2023 at 5:54 PM Shunsuke Mie <[email protected]> wrote:
>
> Gentle ping ...
>
>
> Thanks,
>
> Shunsuke.
>
> On 2023/05/10 12:17, Shunsuke Mie wrote:
> > Hi Json,
> > 2023年5月8日(月) 13:03 Jason Wang <[email protected]>:
> >> On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
> >>> Add a new PCIe endpoint function driver that works as a pci virtio-console
> >>> device. The console connect to endpoint side console. It enables to
> >>> communicate PCIe host and endpoint.
> >>>
> >>> Architecture is following:
> >>>
> >>> ┌────────────┐ ┌──────────────────────┬────────────┐
> >>> │virtioe │ │ │virtio │
> >>> │console drv │ ├───────────────┐ │console drv │
> >>> ├────────────┤ │(virtio console│ ├────────────┤
> >>> │ virtio bus │ │ device) │◄────►│ virtio bus │
> >>> ├────────────┤ ├---------------┤ └────────────┤
> >>> │ │ │ pci ep virtio │ │
> >>> │ pci bus │ │ console drv │ │
> >>> │ │ pcie ├───────────────┤ │
> >>> │ │ ◄─────► │ pci ep Bus │ │
> >>> └────────────┘ └───────────────┴───────────────────┘
> >>> PCIe Root PCIe Endpoint
> >>>
> >> I think it might only works for peer devices like:
> >>
> >> net, console or vsock.
> > Could you tell me what "peer devices" means?
I meant, for example we know in the case of virtio-net, TX can talk
with RX belonging to another device directly.
But this is not the case for other devices like virito-blk.
> >
> >> So there're many choices here, I'd like to know what's the reason for
> >> you to implement a mediation.
> >>
> >> An alternative is to implement a dedicated net, console and vsock
> >> driver for vringh (CAIF somehow works like this). This would have
> >> better performance.
> > Does it mean that the driver also functions as a network driver directly?
I meant, e.g in the case of networking, you can have a dedicated
driver with two vringh in the endpoint side.
The benefit is the performance, no need for the (datapath) mediation.
But if we don't care about the performance, this proposal seems to be fine.
Thanks
> >>
> >>> This driver has two roles. The first is as a PCIe endpoint virtio console
> >>> function, which is implemented using the PCIe endpoint framework and PCIe
> >>> EP virtio helpers. The second is as a virtual virtio console device
> >>> connected to the virtio bus on PCIe endpoint Linux.
> >>>
> >>> Communication between the two is achieved by copying the virtqueue data
> >>> between PCIe root and endpoint, respectively.
> >>>
> >>> This is a simple implementation and does not include features of
> >>> virtio-console such as MULTIPORT, EMERG_WRITE, etc. As a result, each
> >>> virtio console driver only displays /dev/hvc0.
> >>>
> >>> As an example of usage, by setting getty to /dev/hvc0, it is possible to
> >>> login to another host.
> >>>
> >>> Signed-off-by: Shunsuke Mie <[email protected]>
> >>> ---
> >>> Changes from v2:
> >>> - Change to use copy functions between kiovs of pci-epf-virtio.
> >>>
> >>> drivers/pci/endpoint/functions/Kconfig | 12 +
> >>> drivers/pci/endpoint/functions/Makefile | 1 +
> >>> drivers/pci/endpoint/functions/pci-epf-vcon.c | 596 ++++++++++++++++++
> >>> 3 files changed, 609 insertions(+)
> >>> create mode 100644 drivers/pci/endpoint/functions/pci-epf-vcon.c
> >>>
> >>> diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
> >>> index fa1a6a569a8f..9ce2698b67e1 100644
> >>> --- a/drivers/pci/endpoint/functions/Kconfig
> >>> +++ b/drivers/pci/endpoint/functions/Kconfig
> >>> @@ -44,3 +44,15 @@ config PCI_EPF_VIRTIO
> >>> select VHOST_RING_IOMEM
> >>> help
> >>> Helpers to implement PCI virtio Endpoint function
> >>> +
> >>> +config PCI_EPF_VCON
> >>> + tristate "PCI Endpoint virito-console driver"
> >>> + depends on PCI_ENDPOINT
> >>> + select VHOST_RING
> >>> + select PCI_EPF_VIRTIO
> >>> + help
> >>> + PCIe Endpoint virtio-console function implementatino. This module
> >>> + enables to show the virtio-console as pci device to PCIe host side, and
> >>> + another virtual virtio-console device registers to endpoint system.
> >>> + Those devices are connected virtually and can communicate each other.
> >>> +
> >>> diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
> >>> index a96f127ce900..b4056689ce33 100644
> >>> --- a/drivers/pci/endpoint/functions/Makefile
> >>> +++ b/drivers/pci/endpoint/functions/Makefile
> >>> @@ -7,3 +7,4 @@ obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
> >>> obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
> >>> obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
> >>> obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
> >>> +obj-$(CONFIG_PCI_EPF_VCON) += pci-epf-vcon.o
> >>> diff --git a/drivers/pci/endpoint/functions/pci-epf-vcon.c b/drivers/pci/endpoint/functions/pci-epf-vcon.c
> >>> new file mode 100644
> >>> index 000000000000..31f4247cd10f
> >>> --- /dev/null
> >>> +++ b/drivers/pci/endpoint/functions/pci-epf-vcon.c
> >>> @@ -0,0 +1,596 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * PCI Endpoint function driver to impliment virtio-console device
> >>> + * functionality.
> >>> + */
> >>> +#include <linux/pci-epf.h>
> >>> +#include <linux/virtio_ids.h>
> >>> +#include <linux/virtio_pci.h>
> >>> +#include <linux/virtio_console.h>
> >>> +#include <linux/virtio_ring.h>
> >>> +
> >>> +#include "pci-epf-virtio.h"
> >>> +
> >>> +static int virtio_queue_size = 0x100;
> >>> +module_param(virtio_queue_size, int, 0444);
> >>> +MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
> >>> +
> >>> +struct epf_vcon {
> >>> + /* To access virtqueues on remote host */
> >>> + struct epf_virtio evio;
> >>> + struct vringh_kiov *rdev_iovs;
> >>> +
> >>> + /* To register a local virtio bus */
> >>> + struct virtio_device vdev;
> >>> +
> >>> + /* To access virtqueus of local host driver */
> >>> + struct vringh *vdev_vrhs;
> >>> + struct vringh_kiov *vdev_iovs;
> >>> + struct virtqueue **vdev_vqs;
> >>> +
> >>> + /* For transportation and notification */
> >>> + struct workqueue_struct *task_wq;
> >>> + struct work_struct raise_irq_work, rx_work, tx_work;
> >>> +
> >>> + /* To retain virtio features. It is commonly used local and remote. */
> >>> + u64 features;
> >>> +
> >>> + /* To show a status whether this driver is ready and the remote is connected */
> >>> + bool connected;
> >>> +};
> >>> +
> >>> +enum {
> >>> + VCON_VIRTQUEUE_RX,
> >>> + VCON_VIRTQUEUE_TX,
> >>> + // Should be end of enum
> >>> + VCON_VIRTQUEUE_NUM
> >>> +};
> >> It would be better if we can split the console specific thing out,
> >> then it allows us to do ethernet and vsock in the future.
> > I'm planning to implement each virtio device in a separate file.
> > https://lwn.net/Articles/922124/
> >
> >
> >
> >> Thanks
> >>
> > Best regards,
> > Shunsuke
>
I'm sorry for late response.
On 2023/05/19 11:01, Jason Wang wrote:
> On Thu, May 18, 2023 at 5:54 PM Shunsuke Mie <[email protected]> wrote:
>> Gentle ping ...
>>
>>
>> Thanks,
>>
>> Shunsuke.
>>
>> On 2023/05/10 12:17, Shunsuke Mie wrote:
>>> Hi Json,
>>> 2023年5月8日(月) 13:03 Jason Wang <[email protected]>:
>>>> On Thu, Apr 27, 2023 at 6:44 PM Shunsuke Mie <[email protected]> wrote:
>>>>> Add a new PCIe endpoint function driver that works as a pci virtio-console
>>>>> device. The console connect to endpoint side console. It enables to
>>>>> communicate PCIe host and endpoint.
>>>>>
>>>>> Architecture is following:
>>>>>
>>>>> ┌────────────┐ ┌──────────────────────┬────────────┐
>>>>> │virtioe │ │ │virtio │
>>>>> │console drv │ ├───────────────┐ │console drv │
>>>>> ├────────────┤ │(virtio console│ ├────────────┤
>>>>> │ virtio bus │ │ device) │◄────►│ virtio bus │
>>>>> ├────────────┤ ├---------------┤ └────────────┤
>>>>> │ │ │ pci ep virtio │ │
>>>>> │ pci bus │ │ console drv │ │
>>>>> │ │ pcie ├───────────────┤ │
>>>>> │ │ ◄─────► │ pci ep Bus │ │
>>>>> └────────────┘ └───────────────┴───────────────────┘
>>>>> PCIe Root PCIe Endpoint
>>>>>
>>>> I think it might only works for peer devices like:
>>>>
>>>> net, console or vsock.
>>> Could you tell me what "peer devices" means?
> I meant, for example we know in the case of virtio-net, TX can talk
> with RX belonging to another device directly.
>
> But this is not the case for other devices like virito-blk.
Thank you. I comprehended it.
>>>> So there're many choices here, I'd like to know what's the reason for
>>>> you to implement a mediation.
>>>>
>>>> An alternative is to implement a dedicated net, console and vsock
>>>> driver for vringh (CAIF somehow works like this). This would have
>>>> better performance.
>>> Does it mean that the driver also functions as a network driver directly?
> I meant, e.g in the case of networking, you can have a dedicated
> driver with two vringh in the endpoint side.
>
> The benefit is the performance, no need for the (datapath) mediation.
>
> But if we don't care about the performance, this proposal seems to be fine.
>
> Thanks
I agree with you. The design you suggested is better in terms of
performance.
However, the proposed design is not bad for the following the reasons I
think.
The proposed design has one more operation in control plane because the data
steps over the virtio-net driver, but the number of copies at the data plane
remains the same. I think the operation added in control plane has small
effects
for performance.
Moreover, there are some advantages when the data step over the virtio-net
driver. We can make use of the optimizations and some functions without
modifications and implementations. e.g. ethtool and XDP(BPF) supports.
Any comments would be appreciated.
>>>>> This driver has two roles. The first is as a PCIe endpoint virtio console
>>>>> function, which is implemented using the PCIe endpoint framework and PCIe
>>>>> EP virtio helpers. The second is as a virtual virtio console device
>>>>> connected to the virtio bus on PCIe endpoint Linux.
>>>>>
>>>>> Communication between the two is achieved by copying the virtqueue data
>>>>> between PCIe root and endpoint, respectively.
>>>>>
>>>>> This is a simple implementation and does not include features of
>>>>> virtio-console such as MULTIPORT, EMERG_WRITE, etc. As a result, each
>>>>> virtio console driver only displays /dev/hvc0.
>>>>>
>>>>> As an example of usage, by setting getty to /dev/hvc0, it is possible to
>>>>> login to another host.
>>>>>
>>>>> Signed-off-by: Shunsuke Mie <[email protected]>
>>>>> ---
>>>>> Changes from v2:
>>>>> - Change to use copy functions between kiovs of pci-epf-virtio.
>>>>>
>>>>> drivers/pci/endpoint/functions/Kconfig | 12 +
>>>>> drivers/pci/endpoint/functions/Makefile | 1 +
>>>>> drivers/pci/endpoint/functions/pci-epf-vcon.c | 596 ++++++++++++++++++
>>>>> 3 files changed, 609 insertions(+)
>>>>> create mode 100644 drivers/pci/endpoint/functions/pci-epf-vcon.c
>>>>>
>>>>> diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
>>>>> index fa1a6a569a8f..9ce2698b67e1 100644
>>>>> --- a/drivers/pci/endpoint/functions/Kconfig
>>>>> +++ b/drivers/pci/endpoint/functions/Kconfig
>>>>> @@ -44,3 +44,15 @@ config PCI_EPF_VIRTIO
>>>>> select VHOST_RING_IOMEM
>>>>> help
>>>>> Helpers to implement PCI virtio Endpoint function
>>>>> +
>>>>> +config PCI_EPF_VCON
>>>>> + tristate "PCI Endpoint virito-console driver"
>>>>> + depends on PCI_ENDPOINT
>>>>> + select VHOST_RING
>>>>> + select PCI_EPF_VIRTIO
>>>>> + help
>>>>> + PCIe Endpoint virtio-console function implementatino. This module
>>>>> + enables to show the virtio-console as pci device to PCIe host side, and
>>>>> + another virtual virtio-console device registers to endpoint system.
>>>>> + Those devices are connected virtually and can communicate each other.
>>>>> +
>>>>> diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
>>>>> index a96f127ce900..b4056689ce33 100644
>>>>> --- a/drivers/pci/endpoint/functions/Makefile
>>>>> +++ b/drivers/pci/endpoint/functions/Makefile
>>>>> @@ -7,3 +7,4 @@ obj-$(CONFIG_PCI_EPF_TEST) += pci-epf-test.o
>>>>> obj-$(CONFIG_PCI_EPF_NTB) += pci-epf-ntb.o
>>>>> obj-$(CONFIG_PCI_EPF_VNTB) += pci-epf-vntb.o
>>>>> obj-$(CONFIG_PCI_EPF_VIRTIO) += pci-epf-virtio.o
>>>>> +obj-$(CONFIG_PCI_EPF_VCON) += pci-epf-vcon.o
>>>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-vcon.c b/drivers/pci/endpoint/functions/pci-epf-vcon.c
>>>>> new file mode 100644
>>>>> index 000000000000..31f4247cd10f
>>>>> --- /dev/null
>>>>> +++ b/drivers/pci/endpoint/functions/pci-epf-vcon.c
>>>>> @@ -0,0 +1,596 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * PCI Endpoint function driver to impliment virtio-console device
>>>>> + * functionality.
>>>>> + */
>>>>> +#include <linux/pci-epf.h>
>>>>> +#include <linux/virtio_ids.h>
>>>>> +#include <linux/virtio_pci.h>
>>>>> +#include <linux/virtio_console.h>
>>>>> +#include <linux/virtio_ring.h>
>>>>> +
>>>>> +#include "pci-epf-virtio.h"
>>>>> +
>>>>> +static int virtio_queue_size = 0x100;
>>>>> +module_param(virtio_queue_size, int, 0444);
>>>>> +MODULE_PARM_DESC(virtio_queue_size, "A length of virtqueue");
>>>>> +
>>>>> +struct epf_vcon {
>>>>> + /* To access virtqueues on remote host */
>>>>> + struct epf_virtio evio;
>>>>> + struct vringh_kiov *rdev_iovs;
>>>>> +
>>>>> + /* To register a local virtio bus */
>>>>> + struct virtio_device vdev;
>>>>> +
>>>>> + /* To access virtqueus of local host driver */
>>>>> + struct vringh *vdev_vrhs;
>>>>> + struct vringh_kiov *vdev_iovs;
>>>>> + struct virtqueue **vdev_vqs;
>>>>> +
>>>>> + /* For transportation and notification */
>>>>> + struct workqueue_struct *task_wq;
>>>>> + struct work_struct raise_irq_work, rx_work, tx_work;
>>>>> +
>>>>> + /* To retain virtio features. It is commonly used local and remote. */
>>>>> + u64 features;
>>>>> +
>>>>> + /* To show a status whether this driver is ready and the remote is connected */
>>>>> + bool connected;
>>>>> +};
>>>>> +
>>>>> +enum {
>>>>> + VCON_VIRTQUEUE_RX,
>>>>> + VCON_VIRTQUEUE_TX,
>>>>> + // Should be end of enum
>>>>> + VCON_VIRTQUEUE_NUM
>>>>> +};
>>>> It would be better if we can split the console specific thing out,
>>>> then it allows us to do ethernet and vsock in the future.
>>> I'm planning to implement each virtio device in a separate file.
>>> https://lwn.net/Articles/922124/
>>>
>>>
>>>
>>>> Thanks
>>>>
>>> Best regards,
>>> Shunsuke
Best regards,
Shunsuke