[PATCH 0/6] Add virtio-mmio support

[Gerd Hoffmann]

qemu got a new "microvm" machine type which has no pci support.
virtio-mmio is used for devices instead.  This series adds
support for virtio-mmio devices to seabios to allow booting
from block and scsi devices.

Gerd Hoffmann (6):
  string: add strtol() implementation
  paravirt/qemu: virtio-mmio device discovery
  virtio-mmio: device probing and initialization.
  virtio-mmio: add support to vp_*() functions
  virtio-mmio: add support for scsi devices.
  virtio-mmio: add support for block devices.

Makefile             |  2 +-
 src/hw/virtio-blk.h  |  1 +
 src/hw/virtio-mmio.h | 77 +++++++++++++++++++++++++++++++++++++++++
 src/hw/virtio-pci.h  |  1 +
 src/hw/virtio-scsi.h |  1 +
 src/string.h         |  1 +
 src/util.h           |  2 ++
 src/block.c          |  2 ++
 src/boot.c           | 19 +++++++++++
 src/fw/paravirt.c    | 24 +++++++++++++
 src/hw/virtio-blk.c  | 71 ++++++++++++++++++++++++++++++++++++++
 src/hw/virtio-mmio.c | 81 ++++++++++++++++++++++++++++++++++++++++++++
 src/hw/virtio-pci.c  | 68 +++++++++++++++++++++++++++++++------
 src/hw/virtio-scsi.c | 73 +++++++++++++++++++++++++++++++++------
 src/string.c         | 25 ++++++++++++++
 15 files changed, 425 insertions(+), 23 deletions(-)
 create mode 100644 src/hw/virtio-mmio.h
 create mode 100644 src/hw/virtio-mmio.c

[PATCH 1/6] string: add strtol() implementation

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 src/string.h |  1 +
 src/string.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/src/string.h b/src/string.h
index d069989db4b4..17ef3c6a1e89 100644
--- a/src/string.h
+++ b/src/string.h
@@ -27,5 +27,6 @@ void *memmove(void *d, const void *s, size_t len);
 char *strtcpy(char *dest, const char *src, size_t len);
 char *strchr(const char *s, int c);
 char *nullTrailingSpace(char *buf);
+u64 strtol(const char *ptr, int base);

#endif // string.h
diff --git a/src/string.c b/src/string.c
index adb8198f84f4..ec8cf6108561 100644
--- a/src/string.c
+++ b/src/string.c
@@ -249,3 +249,28 @@ nullTrailingSpace(char *buf)
         buf++;
     return buf;
 }
+
+u64 strtol(const char *ptr, int base)
+{
+    u64 digit, val = 0;
+
+    for (;;) {
+        switch (*ptr) {
+        case '0' ... '9':
+            digit = *ptr - '0';
+            break;
+        case 'a' ... 'f':
+            digit = *ptr - 'a' + 10;
+            break;
+        case 'A' ... 'F':
+            digit = *ptr - 'A' + 10;
+            break;
+        default:
+            return val;
+        }
+        if (digit >= base)
+            return val;
+        val = val * base + digit;
+        ptr++;
+    }
+}

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Gerd Hoffmann]

Use bootorder fw_cfg file to find bootable virtio-mmio devices.
Also add a new virtio-mmio.c source file, providing a function
to register virtio-mmio devices.

Signed-off-by: Gerd Hoffmann 
---
 Makefile             |  2 +-
 src/hw/virtio-mmio.h |  6 ++++++
 src/fw/paravirt.c    | 24 ++++++++++++++++++++++++
 src/hw/virtio-mmio.c | 30 ++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 src/hw/virtio-mmio.h
 create mode 100644 src/hw/virtio-mmio.c

diff --git a/Makefile b/Makefile
index 5f7d5370198a..985ef591a13b 100644
--- a/Makefile
+++ b/Makefile
@@ -43,7 +43,7 @@ SRC32FLAT=$(SRCBOTH) post.c e820map.c malloc.c romfile.c x86.c optionroms.c \
     fw/coreboot.c fw/lzmadecode.c fw/multiboot.c fw/csm.c fw/biostables.c \
     fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/smp.c fw/mtrr.c fw/xen.c \
     fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c fw/romfile_loader.c \
-    hw/virtio-ring.c hw/virtio-pci.c hw/virtio-blk.c hw/virtio-scsi.c \
+    hw/virtio-ring.c hw/virtio-pci.c hw/virtio-mmio.c hw/virtio-blk.c hw/virtio-scsi.c \
     hw/tpm_drivers.c hw/nvme.c
 SRC32SEG=string.c output.c pcibios.c apm.c stacks.c hw/pci.c hw/serialio.c
 DIRS=src src/hw src/fw vgasrc
diff --git a/src/hw/virtio-mmio.h b/src/hw/virtio-mmio.h
new file mode 100644
index 000000000000..751984241f49
--- /dev/null
+++ b/src/hw/virtio-mmio.h
@@ -0,0 +1,6 @@
+#ifndef _VIRTIO_MMIO_H
+#define _VIRTIO_MMIO_H
+
+void virtio_mmio_register(u64 mmio);
+
+#endif /* _VIRTIO_MMIO_H */
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 119280c574fd..aaaf8b8dad31 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -15,6 +15,7 @@
 #include "hw/pcidevice.h" // pci_probe_devices
 #include "hw/pci_regs.h" // PCI_DEVICE_ID
 #include "hw/serialio.h" // PORT_SERIAL1
+#include "hw/virtio-mmio.h" // virtio_mmio_register
 #include "hw/rtc.h" // CMOS_*
 #include "malloc.h" // malloc_tmp
 #include "output.h" // dprintf
@@ -192,6 +193,27 @@ static void msr_feature_control_setup(void)
         wrmsr_smp(MSR_IA32_FEATURE_CONTROL, feature_control_bits);
 }

+static void qemu_probe_virtio_mmio(void)
+{
+    char *data, *next;
+    int size;
+    u64 mmio;
+
+    data = romfile_loadfile("bootorder", &size);
+    while (data) {
+        next = strchr(data, '\n');
+        if (next) {
+            *next = '\0';
+            next++;
+        }
+        if (memcmp("/virtio-mmio@", data, 13) == 0) {
+            mmio = strtol(data+13, 16);
+            virtio_mmio_register(mmio);
+        }
+        data = next;
+    }
+}
+
 void
 qemu_platform_setup(void)
 {
@@ -217,6 +239,8 @@ qemu_platform_setup(void)
     msr_feature_control_setup();
     smp_setup();

+    qemu_probe_virtio_mmio();
+
     // Create bios tables
     if (MaxCountCPUs <= 255) {
         pirtable_setup();
diff --git a/src/hw/virtio-mmio.c b/src/hw/virtio-mmio.c
new file mode 100644
index 000000000000..0f320921df30
--- /dev/null
+++ b/src/hw/virtio-mmio.c
@@ -0,0 +1,30 @@
+#include "config.h" // CONFIG_DEBUG_LEVEL
+#include "malloc.h" // free
+#include "output.h" // dprintf
+#include "virtio-pci.h"
+#include "virtio-ring.h"
+
+/* qemu microvm supports 8 virtio-mmio devices */
+static u64 devs[8];
+
+void virtio_mmio_register(u64 mmio)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(devs); i++) {
+        if (devs[i] == mmio) {
+            /*
+             * This can happen in case we have multiple scsi devices
+             * attached to a single virtio-scsi controller
+             */
+            dprintf(3, "virtio-mmio: duplicate device at 0x%llx, ignoring\n", mmio);
+            return;
+        }
+        if (devs[i] == 0) {
+            dprintf(1, "virtio-mmio: device at 0x%llx\n", mmio);
+            devs[i] = mmio;
+            return;
+        }
+    }
+    dprintf(1, "virtio-mmio: device list full\n");
+}

[PATCH 3/6] virtio-mmio: device probing and initialization.

[Gerd Hoffmann]

Add virtio_mmio_setup() to probe virtio mmio devices.  Add
vp_init_mmio() to initialize device struct.  Because virtio-pci and
virtio-mmio are quite simliar we reuse the infrastructure we already
have for virtio-pci and just setup struct vp_cap for virtio-mmio.

Signed-off-by: Gerd Hoffmann 
---
 src/hw/virtio-mmio.h | 71 ++++++++++++++++++++++++++++++++++++++++++++
 src/hw/virtio-pci.h  |  1 +
 src/block.c          |  2 ++
 src/hw/virtio-mmio.c | 49 ++++++++++++++++++++++++++++++
 4 files changed, 123 insertions(+)

diff --git a/src/hw/virtio-mmio.h b/src/hw/virtio-mmio.h
index 751984241f49..5e24f692c1ae 100644
--- a/src/hw/virtio-mmio.h
+++ b/src/hw/virtio-mmio.h
@@ -1,6 +1,77 @@
 #ifndef _VIRTIO_MMIO_H
 #define _VIRTIO_MMIO_H

+struct vp_device;
+
+typedef struct virtio_mmio_cfg {
+    u32 magic;
+    u32 version;
+    u32 device_id;
+    u32 vendor_id;
+
+    u32 device_feature;
+    u32 device_feature_select;
+    u32 res_18;
+    u32 res_1c;
+
+    u32 guest_feature;
+    u32 guest_feature_select;
+    u32 legacy_guest_page_size;
+    u32 res_2c;
+
+    u32 queue_select;
+    u32 queue_num_max;
+    u32 queue_num;
+    u32 legacy_queue_align;
+
+    u32 legacy_queue_pfn;
+    u32 queue_ready;
+    u32 res_48;
+    u32 res_4c;
+
+    u32 queue_notify;
+    u32 res_54;
+    u32 res_58;
+    u32 res_5c;
+
+    u32 irq_status;
+    u32 irq_ack;
+    u32 res_68;
+    u32 res_6c;
+
+    u32 device_status;
+    u32 res_74;
+    u32 res_78;
+    u32 res_7c;
+
+    u32 queue_desc_lo;
+    u32 queue_desc_hi;
+    u32 res_88;
+    u32 res_8c;
+
+    u32 queue_driver_lo;
+    u32 queue_driver_hi;
+    u32 res_98;
+    u32 res_9c;
+
+    u32 queue_device_lo;
+    u32 queue_device_hi;
+    u32 res_a8;
+    u32 shm_sel;
+
+    u32 shmem_len_lo;
+    u32 shmem_len_hi;
+    u32 shmem_base_lo;
+    u32 shmem_base_hi;
+
+    u32 res_c0_f7[14];
+
+    u32 res_f8;
+    u32 config_generation;
+} virtio_mmio_cfg;
+
 void virtio_mmio_register(u64 mmio);
+void virtio_mmio_setup(void);
+void vp_init_mmio(struct vp_device *vp, void *mmio);

#endif /* _VIRTIO_MMIO_H */
diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h
index 492e5c7c9166..269626448558 100644
--- a/src/hw/virtio-pci.h
+++ b/src/hw/virtio-pci.h
@@ -111,6 +111,7 @@ struct vp_device {
     struct vp_cap common, notify, isr, device, legacy;
     u32 notify_off_multiplier;
     u8 use_modern;
+    u8 use_mmio;
 };

u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size);
diff --git a/src/block.c b/src/block.c
index 1f600b85391b..9ef607610c40 100644
--- a/src/block.c
+++ b/src/block.c
@@ -20,6 +20,7 @@
 #include "hw/usb-uas.h" // uas_process_op
 #include "hw/virtio-blk.h" // process_virtio_blk_op
 #include "hw/virtio-scsi.h" // virtio_scsi_process_op
+#include "hw/virtio-mmio.h" // virtio_mmio_setup
 #include "hw/nvme.h" // nvme_process_op
 #include "malloc.h" // malloc_low
 #include "output.h" // dprintf
@@ -514,6 +515,7 @@ block_setup(void)
     ramdisk_setup();
     virtio_blk_setup();
     virtio_scsi_setup();
+    virtio_mmio_setup();
     lsi_scsi_setup();
     esp_scsi_setup();
     megasas_setup();
diff --git a/src/hw/virtio-mmio.c b/src/hw/virtio-mmio.c
index 0f320921df30..d9068c7c035b 100644
--- a/src/hw/virtio-mmio.c
+++ b/src/hw/virtio-mmio.c
@@ -1,8 +1,11 @@
 #include "config.h" // CONFIG_DEBUG_LEVEL
 #include "malloc.h" // free
 #include "output.h" // dprintf
+#include "stacks.h" // run_thread
+#include "string.h" // memset
 #include "virtio-pci.h"
 #include "virtio-ring.h"
+#include "virtio-mmio.h"

/* qemu microvm supports 8 virtio-mmio devices */
 static u64 devs[8];
@@ -28,3 +31,49 @@ void virtio_mmio_register(u64 mmio)
     }
     dprintf(1, "virtio-mmio: device list full\n");
 }
+
+void virtio_mmio_setup(void)
+{
+    u32 magic, version, devid;
+    void *mmio;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(devs); i++) {
+        if (devs[i] == 0)
+            return;
+        mmio = (void*)(u32)(devs[i]);
+        magic = readl(mmio);
+        if (magic != 0x74726976)
+            continue;
+        version = readl(mmio+4);
+        if (version != 1 /* legacy */ &&
+            version != 2 /* 1.0 */)
+            continue;
+        devid = readl(mmio+8);
+        dprintf(1, "virtio-mmio: %llx: device id %x%s\n",
+                devs[i], devid, version == 1 ? " (legacy)" : "");
+        switch (devid) {
+        case 2: /* blk */
+            /* TODO */
+            break;
+        case 8: /* scsi */
+            /* TODO */
+            break;
+        default:
+            break;
+        }
+    }
+}
+
+void vp_init_mmio(struct vp_device *vp, void *mmio)
+{
+    memset(vp, 0, sizeof(*vp));
+    vp->use_mmio = 1;
+    vp->common.mode = VP_ACCESS_MMIO;
+    vp->common.memaddr = mmio;
+    vp->device.mode = VP_ACCESS_MMIO;
+    vp->device.memaddr = mmio + 0x100;
+    vp_reset(vp);
+    vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE |
+                  VIRTIO_CONFIG_S_DRIVER);
+}

[PATCH 4/6] virtio-mmio: add support to vp_*() functions

[Gerd Hoffmann]

Add support for virtio-mmio to the vp_*() helper functions.
Both legacy and 1.0 virto-mmio versions are supported.  They
are very simliar anyway, only the virtqueue initialization
is slightly different.

Signed-off-by: Gerd Hoffmann 
---
 src/hw/virtio-pci.c | 68 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c
index d5435218fb0a..213c49777c40 100644
--- a/src/hw/virtio-pci.c
+++ b/src/hw/virtio-pci.c
@@ -23,6 +23,7 @@
 #include "pci_regs.h" // PCI_BASE_ADDRESS_0
 #include "string.h" // memset
 #include "virtio-pci.h"
+#include "virtio-mmio.h"
 #include "virtio-ring.h"

u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size)
@@ -189,7 +190,11 @@ u64 vp_get_features(struct vp_device *vp)
 {
     u32 f0, f1;

-    if (vp->use_modern) {
+    if (vp->use_mmio) {
+        vp_write(&vp->common, virtio_mmio_cfg, device_feature_select, 0);
+        f0 = vp_read(&vp->common, virtio_mmio_cfg, device_feature);
+        f1 = 0;
+    } else if (vp->use_modern) {
         vp_write(&vp->common, virtio_pci_common_cfg, device_feature_select, 0);
         f0 = vp_read(&vp->common, virtio_pci_common_cfg, device_feature);
         vp_write(&vp->common, virtio_pci_common_cfg, device_feature_select, 1);
@@ -208,7 +213,10 @@ void vp_set_features(struct vp_device *vp, u64 features)
     f0 = features;
     f1 = features >> 32;

-    if (vp->use_modern) {
+    if (vp->use_mmio) {
+        vp_write(&vp->common, virtio_mmio_cfg, guest_feature_select, f0);
+        vp_write(&vp->common, virtio_mmio_cfg, guest_feature, f0);
+    } else if (vp->use_modern) {
         vp_write(&vp->common, virtio_pci_common_cfg, guest_feature_select, 0);
         vp_write(&vp->common, virtio_pci_common_cfg, guest_feature, f0);
         vp_write(&vp->common, virtio_pci_common_cfg, guest_feature_select, 1);
@@ -220,7 +228,9 @@ void vp_set_features(struct vp_device *vp, u64 features)

u8 vp_get_status(struct vp_device *vp)
 {
-    if (vp->use_modern) {
+    if (vp->use_mmio) {
+        return vp_read(&vp->common, virtio_mmio_cfg, device_status);
+    } else if (vp->use_modern) {
         return vp_read(&vp->common, virtio_pci_common_cfg, device_status);
     } else {
         return vp_read(&vp->legacy, virtio_pci_legacy, status);
@@ -231,7 +241,9 @@ void vp_set_status(struct vp_device *vp, u8 status)
 {
     if (status == 0)        /* reset */
         return;
-    if (vp->use_modern) {
+    if (vp->use_mmio) {
+        vp_write(&vp->common, virtio_mmio_cfg, device_status, status);
+    } else if (vp->use_modern) {
         vp_write(&vp->common, virtio_pci_common_cfg, device_status, status);
     } else {
         vp_write(&vp->legacy, virtio_pci_legacy, status, status);
@@ -240,7 +252,9 @@ void vp_set_status(struct vp_device *vp, u8 status)

u8 vp_get_isr(struct vp_device *vp)
 {
-    if (vp->use_modern) {
+    if (vp->use_mmio) {
+        return vp_read(&vp->common, virtio_mmio_cfg, irq_status);
+    } else if (vp->use_modern) {
         return vp_read(&vp->isr, virtio_pci_isr, isr);
     } else {
         return vp_read(&vp->legacy, virtio_pci_legacy, isr);
@@ -249,7 +263,10 @@ u8 vp_get_isr(struct vp_device *vp)

void vp_reset(struct vp_device *vp)
 {
-    if (vp->use_modern) {
+    if (vp->use_mmio) {
+        vp_write(&vp->common, virtio_mmio_cfg, device_status, 0);
+        vp_read(&vp->common, virtio_mmio_cfg, irq_status);
+    } else if (vp->use_modern) {
         vp_write(&vp->common, virtio_pci_common_cfg, device_status, 0);
         vp_read(&vp->isr, virtio_pci_isr, isr);
     } else {
@@ -260,7 +277,9 @@ void vp_reset(struct vp_device *vp)

void vp_notify(struct vp_device *vp, struct vring_virtqueue *vq)
 {
-    if (vp->use_modern) {
+    if (vp->use_mmio) {
+        vp_write(&vp->common, virtio_mmio_cfg, queue_notify, vq->queue_index);
+    } else if (vp->use_modern) {
         u32 offset = vq->queue_notify_off * vp->notify_off_multiplier;
         switch (vp->notify.mode) {
         case VP_ACCESS_IO:
@@ -305,14 +324,21 @@ int vp_find_vq(struct vp_device *vp, int queue_index,

/* select the queue */
-   if (vp->use_modern) {
+   if (vp->use_mmio) {
+       vp_write(&vp->common, virtio_mmio_cfg, queue_select, queue_index);
+   } else if (vp->use_modern) {
        vp_write(&vp->common, virtio_pci_common_cfg, queue_select, queue_index);
    } else {
        vp_write(&vp->legacy, virtio_pci_legacy, queue_sel, queue_index);
    }

/* check if the queue is available */
-   if (vp->use_modern) {
+   if (vp->use_mmio) {
+       num = vp_read(&vp->common, virtio_mmio_cfg, queue_num_max);
+       if (num > MAX_QUEUE_NUM)
+           num = MAX_QUEUE_NUM;
+       vp_write(&vp->common, virtio_mmio_cfg, queue_num, num);
+   } else if (vp->use_modern) {
        num = vp_read(&vp->common, virtio_pci_common_cfg, queue_size);
        if (num > MAX_QUEUE_NUM) {
            vp_write(&vp->common, virtio_pci_common_cfg, queue_size,
@@ -332,7 +358,9 @@ int vp_find_vq(struct vp_device *vp, int queue_index,
    }

/* check if the queue is already active */
-   if (vp->use_modern) {
+   if (vp->use_mmio) {
+       /* TODO */;
+   } else if (vp->use_modern) {
        if (vp_read(&vp->common, virtio_pci_common_cfg, queue_enable)) {
            dprintf(1, "ERROR: queue already active\n");
            goto fail;
@@ -354,7 +382,25 @@ int vp_find_vq(struct vp_device *vp, int queue_index,
     * NOTE: vr->desc is initialized by vring_init()
     */

-   if (vp->use_modern) {
+   if (vp->use_mmio) {
+       if (vp_read(&vp->common, virtio_mmio_cfg, version) == 2) {
+           vp_write(&vp->common, virtio_mmio_cfg, queue_desc_lo,
+                    (unsigned long)virt_to_phys(vr->desc));
+           vp_write(&vp->common, virtio_mmio_cfg, queue_desc_hi, 0);
+           vp_write(&vp->common, virtio_mmio_cfg, queue_driver_lo,
+                    (unsigned long)virt_to_phys(vr->avail));
+           vp_write(&vp->common, virtio_mmio_cfg, queue_driver_hi, 0);
+           vp_write(&vp->common, virtio_mmio_cfg, queue_device_lo,
+                    (unsigned long)virt_to_phys(vr->used));
+           vp_write(&vp->common, virtio_mmio_cfg, queue_device_hi, 0);
+           vp_write(&vp->common, virtio_mmio_cfg, queue_ready, 1);
+       } else {
+           vp_write(&vp->common, virtio_mmio_cfg, legacy_guest_page_size,
+                    (unsigned long)1 << PAGE_SHIFT);
+           vp_write(&vp->common, virtio_mmio_cfg, legacy_queue_pfn,
+                    (unsigned long)virt_to_phys(vr->desc) >> PAGE_SHIFT);
+       }
+   } else if (vp->use_modern) {
        vp_write(&vp->common, virtio_pci_common_cfg, queue_desc_lo,
                 (unsigned long)virt_to_phys(vr->desc));
        vp_write(&vp->common, virtio_pci_common_cfg, queue_desc_hi, 0);

[PATCH 5/6] virtio-mmio: add support for scsi devices.

[Gerd Hoffmann]

Add new fields to struct virtio_lun_s for mmio support,
add mmio parameter to virtio_scsi_init_lun(), so both
pci and mmio devices can be handled.

Add and use bootprio_find_scsi_mmio_device() to figure
boot priority of devices connected to a virtio-mmio
scsi controller.

Finally add init_virtio_scsi_mmio() to initialize one
virtio-mmio scsi controller.

Signed-off-by: Gerd Hoffmann 
---
 src/hw/virtio-scsi.h |  1 +
 src/util.h           |  1 +
 src/boot.c           | 10 ++++++
 src/hw/virtio-mmio.c |  3 +-
 src/hw/virtio-scsi.c | 73 +++++++++++++++++++++++++++++++++++++-------
 5 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/src/hw/virtio-scsi.h b/src/hw/virtio-scsi.h
index 7532cc98e56e..8f01de4cba99 100644
--- a/src/hw/virtio-scsi.h
+++ b/src/hw/virtio-scsi.h
@@ -43,5 +43,6 @@ struct virtio_scsi_resp_cmd {
 struct disk_op_s;
 int virtio_scsi_process_op(struct disk_op_s *op);
 void virtio_scsi_setup(void);
+void init_virtio_scsi_mmio(void *data);

#endif /* _VIRTIO_SCSI_H */
diff --git a/src/util.h b/src/util.h
index 94592a2d2e8d..1c82e09ed87b 100644
--- a/src/util.h
+++ b/src/util.h
@@ -31,6 +31,7 @@ u8 is_bootprio_strict(void);
 struct pci_device;
 int bootprio_find_pci_device(struct pci_device *pci);
 int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun);
+int bootprio_find_scsi_mmio_device(void *mmio, int target, int lun);
 int bootprio_find_ata_device(struct pci_device *pci, int chanid, int slave);
 int bootprio_find_fdc_device(struct pci_device *pci, int port, int fdid);
 int bootprio_find_pci_rom(struct pci_device *pci, int instance);
diff --git a/src/boot.c b/src/boot.c
index 4f12988f687c..f2f084bea843 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -328,6 +328,16 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
     return find_prio(desc);
 }

+int bootprio_find_scsi_mmio_device(void *mmio, int target, int lun)
+{
+    if (!CONFIG_BOOTORDER)
+        return -1;
+    char desc[256];
+    snprintf(desc, sizeof(desc), "/virtio-mmio@%016x/*@0/*@%x,%x",
+             (u32)mmio, target, lun);
+    return find_prio(desc);
+}
+
 int bootprio_find_ata_device(struct pci_device *pci, int chanid, int slave)
 {
     if (CONFIG_CSM)
diff --git a/src/hw/virtio-mmio.c b/src/hw/virtio-mmio.c
index d9068c7c035b..3b514d944a9d 100644
--- a/src/hw/virtio-mmio.c
+++ b/src/hw/virtio-mmio.c
@@ -4,6 +4,7 @@
 #include "stacks.h" // run_thread
 #include "string.h" // memset
 #include "virtio-pci.h"
+#include "virtio-scsi.h"
 #include "virtio-ring.h"
 #include "virtio-mmio.h"

@@ -57,7 +58,7 @@ void virtio_mmio_setup(void)
             /* TODO */
             break;
         case 8: /* scsi */
-            /* TODO */
+            run_thread(init_virtio_scsi_mmio, mmio);
             break;
         default:
             break;
diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
index a5332848b8c8..59f1c654d76b 100644
--- a/src/hw/virtio-scsi.c
+++ b/src/hw/virtio-scsi.c
@@ -22,10 +22,13 @@
 #include "virtio-pci.h"
 #include "virtio-ring.h"
 #include "virtio-scsi.h"
+#include "virtio-mmio.h"

struct virtio_lun_s {
     struct drive_s drive;
     struct pci_device *pci;
+    void *mmio;
+    char name[16];
     struct vring_virtqueue *vq;
     struct vp_device *vp;
     u16 target;
@@ -94,7 +97,8 @@ virtio_scsi_process_op(struct disk_op_s *op)
 }

static void
-virtio_scsi_init_lun(struct virtio_lun_s *vlun, struct pci_device *pci,
+virtio_scsi_init_lun(struct virtio_lun_s *vlun,
+                     struct pci_device *pci, void *mmio,
                      struct vp_device *vp, struct vring_virtqueue *vq,
                      u16 target, u16 lun)
 {
@@ -102,10 +106,15 @@ virtio_scsi_init_lun(struct virtio_lun_s *vlun, struct pci_device *pci,
     vlun->drive.type = DTYPE_VIRTIO_SCSI;
     vlun->drive.cntl_id = pci->bdf;
     vlun->pci = pci;
+    vlun->mmio = mmio;
     vlun->vp = vp;
     vlun->vq = vq;
     vlun->target = target;
     vlun->lun = lun;
+    if (vlun->pci)
+        snprintf(vlun->name, sizeof(vlun->name), "pci:%pP", vlun->pci);
+    if (vlun->mmio)
+        snprintf(vlun->name, sizeof(vlun->name), "mmio:%08x", (u32)vlun->mmio);
 }

static int
@@ -114,12 +123,17 @@ virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
     u8 skip_nonbootable = is_bootprio_strict();
     struct virtio_lun_s *tmpl_vlun =
         container_of(tmpl_drv, struct virtio_lun_s, drive);
-    int prio = bootprio_find_scsi_device(tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
+    int prio = -1;
+
+    if (tmpl_vlun->pci)
+        prio = bootprio_find_scsi_device(tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
+    if (tmpl_vlun->mmio)
+        prio = bootprio_find_scsi_mmio_device(tmpl_vlun->mmio, tmpl_vlun->target, tmpl_vlun->lun);

if (skip_nonbootable && prio < 0) {
-        dprintf(1, "skipping init of a non-bootable virtio-scsi dev at %pP,"
+        dprintf(1, "skipping init of a non-bootable virtio-scsi dev at %s,"
                 " target %d, lun %d\n",
-                tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
+                tmpl_vlun->name, tmpl_vlun->target, tmpl_vlun->lun);
         return -1;
     }

@@ -128,11 +142,12 @@ virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
         warn_noalloc();
         return -1;
     }
-    virtio_scsi_init_lun(vlun, tmpl_vlun->pci, tmpl_vlun->vp, tmpl_vlun->vq,
-                         tmpl_vlun->target, lun);
+    virtio_scsi_init_lun(vlun, tmpl_vlun->pci, tmpl_vlun->mmio,tmpl_vlun->vp,
+                         tmpl_vlun->vq, tmpl_vlun->target, lun);

-    boot_lchs_find_scsi_device(vlun->pci, vlun->target, vlun->lun,
-                               &(vlun->drive.lchs));
+    if (vlun->pci)
+        boot_lchs_find_scsi_device(vlun->pci, vlun->target, vlun->lun,
+                                   &(vlun->drive.lchs));
     int ret = scsi_drive_setup(&vlun->drive, "virtio-scsi", prio);
     if (ret)
         goto fail;
@@ -144,13 +159,13 @@ fail:
 }

static int
-virtio_scsi_scan_target(struct pci_device *pci, struct vp_device *vp,
+virtio_scsi_scan_target(struct pci_device *pci, void *mmio, struct vp_device *vp,
                         struct vring_virtqueue *vq, u16 target)
 {

struct virtio_lun_s vlun0;

-    virtio_scsi_init_lun(&vlun0, pci, vp, vq, target, 0);
+    virtio_scsi_init_lun(&vlun0, pci, mmio, vp, vq, target, 0);

int ret = scsi_rep_luns_scan(&vlun0.drive, virtio_scsi_add_lun);
     return ret < 0 ? 0 : ret;
@@ -198,7 +213,43 @@ init_virtio_scsi(void *data)

int i, tot;
     for (tot = 0, i = 0; i < 256; i++)
-        tot += virtio_scsi_scan_target(pci, vp, vq, i);
+        tot += virtio_scsi_scan_target(pci, NULL, vp, vq, i);
+
+    if (!tot)
+        goto fail;
+
+    return;
+
+fail:
+    vp_reset(vp);
+    free(vp);
+    free(vq);
+}
+
+void
+init_virtio_scsi_mmio(void *mmio)
+{
+    dprintf(1, "found virtio-scsi-mmio at %p\n", mmio);
+    struct vring_virtqueue *vq = NULL;
+    struct vp_device *vp = malloc_high(sizeof(*vp));
+    if (!vp) {
+        warn_noalloc();
+        return;
+    }
+    vp_init_mmio(vp, mmio);
+    u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER;
+
+    if (vp_find_vq(vp, 2, &vq) < 0 ) {
+        dprintf(1, "fail to find vq for virtio-scsi-mmio %p\n", mmio);
+        goto fail;
+    }
+
+    status |= VIRTIO_CONFIG_S_DRIVER_OK;
+    vp_set_status(vp, status);
+
+    int i, tot;
+    for (tot = 0, i = 0; i < 256; i++)
+        tot += virtio_scsi_scan_target(NULL, mmio, vp, vq, i);

if (!tot)
         goto fail;

[PATCH 6/6] virtio-mmio: add support for block devices.

[Gerd Hoffmann]

Add and use bootprio_find_mmio_device() to figure
the boot priority of virtio-mmio block devices.

Add init_virtio_blk_mmio to initialize one
virtio-mmio block device.

Signed-off-by: Gerd Hoffmann 
---
 src/hw/virtio-blk.h  |  1 +
 src/util.h           |  1 +
 src/boot.c           |  9 ++++++
 src/hw/virtio-blk.c  | 71 ++++++++++++++++++++++++++++++++++++++++++++
 src/hw/virtio-mmio.c |  3 +-
 5 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/src/hw/virtio-blk.h b/src/hw/virtio-blk.h
index 157bed62744a..d20461a2a3b2 100644
--- a/src/hw/virtio-blk.h
+++ b/src/hw/virtio-blk.h
@@ -39,5 +39,6 @@ struct virtio_blk_outhdr {
 struct disk_op_s;
 int virtio_blk_process_op(struct disk_op_s *op);
 void virtio_blk_setup(void);
+void init_virtio_blk_mmio(void *mmio);

#endif /* _VIRTIO_BLK_H */
diff --git a/src/util.h b/src/util.h
index 1c82e09ed87b..4f27fc307439 100644
--- a/src/util.h
+++ b/src/util.h
@@ -30,6 +30,7 @@ void bcv_prepboot(void);
 u8 is_bootprio_strict(void);
 struct pci_device;
 int bootprio_find_pci_device(struct pci_device *pci);
+int bootprio_find_mmio_device(void *mmio);
 int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun);
 int bootprio_find_scsi_mmio_device(void *mmio, int target, int lun);
 int bootprio_find_ata_device(struct pci_device *pci, int chanid, int slave);
diff --git a/src/boot.c b/src/boot.c
index f2f084bea843..cc87b1d98476 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -316,6 +316,15 @@ int bootprio_find_pci_device(struct pci_device *pci)
     return find_prio(desc);
 }

+int bootprio_find_mmio_device(void *mmio)
+{
+    if (!CONFIG_BOOTORDER)
+        return -1;
+    char desc[256];
+    snprintf(desc, sizeof(desc), "/virtio-mmio@%016x/*", (u32)mmio);
+    return find_prio(desc);
+}
+
 int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
 {
     if (!CONFIG_BOOTORDER)
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c
index a5e28fc858b1..3b198965c8ba 100644
--- a/src/hw/virtio-blk.c
+++ b/src/hw/virtio-blk.c
@@ -20,6 +20,7 @@
 #include "string.h" // memset
 #include "util.h" // usleep, bootprio_find_pci_device, is_bootprio_strict
 #include "virtio-pci.h"
+#include "virtio-mmio.h"
 #include "virtio-ring.h"
 #include "virtio-blk.h"

@@ -193,6 +194,76 @@ fail:
     free(vdrive);
 }

+void
+init_virtio_blk_mmio(void *mmio)
+{
+    u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER;
+    dprintf(1, "found virtio-blk-mmio at %p\n", mmio);
+    struct virtiodrive_s *vdrive = malloc_low(sizeof(*vdrive));
+    if (!vdrive) {
+        warn_noalloc();
+        return;
+    }
+    memset(vdrive, 0, sizeof(*vdrive));
+    vdrive->drive.type = DTYPE_VIRTIO_BLK;
+    vdrive->drive.cntl_id = (u32)mmio;
+
+    vp_init_mmio(&vdrive->vp, mmio);
+    if (vp_find_vq(&vdrive->vp, 0, &vdrive->vq) < 0 ) {
+        dprintf(1, "fail to find vq for virtio-blk-mmio %p\n", mmio);
+        goto fail;
+    }
+
+    struct vp_device *vp = &vdrive->vp;
+    u64 features = vp_get_features(vp);
+    u64 version1 = 1ull << VIRTIO_F_VERSION_1;
+    u64 blk_size = 1ull << VIRTIO_BLK_F_BLK_SIZE;
+
+    features = features & (version1 | blk_size);
+    vp_set_features(vp, features);
+    status |= VIRTIO_CONFIG_S_FEATURES_OK;
+    vp_set_status(vp, status);
+    if (!(vp_get_status(vp) & VIRTIO_CONFIG_S_FEATURES_OK)) {
+        dprintf(1, "device didn't accept features: %p\n", mmio);
+        goto fail;
+    }
+
+    vdrive->drive.sectors =
+        vp_read(&vp->device, struct virtio_blk_config, capacity);
+    if (features & blk_size) {
+        vdrive->drive.blksize =
+            vp_read(&vp->device, struct virtio_blk_config, blk_size);
+    } else {
+        vdrive->drive.blksize = DISK_SECTOR_SIZE;
+    }
+    if (vdrive->drive.blksize != DISK_SECTOR_SIZE) {
+        dprintf(1, "virtio-blk-mmio %p block size %d is unsupported\n",
+                mmio, vdrive->drive.blksize);
+        goto fail;
+    }
+    dprintf(1, "virtio-blk-mmio %p blksize=%d sectors=%u\n",
+            mmio, vdrive->drive.blksize, (u32)vdrive->drive.sectors);
+
+    vdrive->drive.pchs.cylinder =
+        vp_read(&vp->device, struct virtio_blk_config, cylinders);
+    vdrive->drive.pchs.head =
+        vp_read(&vp->device, struct virtio_blk_config, heads);
+    vdrive->drive.pchs.sector =
+        vp_read(&vp->device, struct virtio_blk_config, sectors);
+
+    char *desc = znprintf(MAXDESCSIZE, "Virtio disk mmio:%p", mmio);
+    boot_add_hd(&vdrive->drive, desc, bootprio_find_mmio_device(mmio));
+
+    status |= VIRTIO_CONFIG_S_DRIVER_OK;
+    vp_set_status(&vdrive->vp, status);
+    return;
+
+fail:
+    vp_reset(&vdrive->vp);
+    free(vdrive->vq);
+    free(vdrive);
+}
+
 void
 virtio_blk_setup(void)
 {
diff --git a/src/hw/virtio-mmio.c b/src/hw/virtio-mmio.c
index 3b514d944a9d..30834ee372b1 100644
--- a/src/hw/virtio-mmio.c
+++ b/src/hw/virtio-mmio.c
@@ -4,6 +4,7 @@
 #include "stacks.h" // run_thread
 #include "string.h" // memset
 #include "virtio-pci.h"
+#include "virtio-blk.h"
 #include "virtio-scsi.h"
 #include "virtio-ring.h"
 #include "virtio-mmio.h"
@@ -55,7 +56,7 @@ void virtio_mmio_setup(void)
                 devs[i], devid, version == 1 ? " (legacy)" : "");
         switch (devid) {
         case 2: /* blk */
-            /* TODO */
+            run_thread(init_virtio_blk_mmio, mmio);
             break;
         case 8: /* scsi */
             run_thread(init_virtio_scsi_mmio, mmio);

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Kevin O'Connor]

On Thu, Mar 19, 2020 at 08:39:33AM +0100, Gerd Hoffmann wrote:
> Use bootorder fw_cfg file to find bootable virtio-mmio devices.
> Also add a new virtio-mmio.c source file, providing a function
> to register virtio-mmio devices.

Can you expand on this?  I'm not sure I understand what this patch
does.  It seems the bootorder file is used for device detection, but
that seems odd.

-Kevin
> Signed-off-by: Gerd Hoffmann
>
> Makefile             |  2 +-
>  src/hw/virtio-mmio.h |  6 ++++++
>  src/fw/paravirt.c    | 24 ++++++++++++++++++++++++
>  src/hw/virtio-mmio.c | 30 ++++++++++++++++++++++++++++++
>  4 files changed, 61 insertions(+), 1 deletion(-)
>  create mode 100644 src/hw/virtio-mmio.h
>  create mode 100644 src/hw/virtio-mmio.c
>
> diff --git a/Makefile b/Makefile
> index 5f7d5370198a..985ef591a13b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -43,7 +43,7 @@ SRC32FLAT=$(SRCBOTH) post.c e820map.c malloc.c romfile.c x86.c optionroms.c \
>      fw/coreboot.c fw/lzmadecode.c fw/multiboot.c fw/csm.c fw/biostables.c \
>      fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/smp.c fw/mtrr.c fw/xen.c \
>      fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c fw/romfile_loader.c \
> hw/virtio-ring.c hw/virtio-pci.c hw/virtio-blk.c hw/virtio-scsi.c \
> hw/virtio-ring.c hw/virtio-pci.c hw/virtio-mmio.c hw/virtio-blk.c hw/virtio-scsi.c \
> hw/tpm_drivers.c hw/nvme.c
>
> SRC32SEG=string.c output.c pcibios.c apm.c stacks.c hw/pci.c hw/serialio.c
>  DIRS=src src/hw src/fw vgasrc
> diff --git a/src/hw/virtio-mmio.h b/src/hw/virtio-mmio.h
> new file mode 100644
> index 000000000000..751984241f49
> --- /dev/null
> +++ b/src/hw/virtio-mmio.h
> @@ -0,0 +1,6 @@
> +#ifndef _VIRTIO_MMIO_H
> +#define _VIRTIO_MMIO_H
>
> +void virtio_mmio_register(u64 mmio);
>
> +#endif /* _VIRTIO_MMIO_H */
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 119280c574fd..aaaf8b8dad31 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -15,6 +15,7 @@
>  #include "hw/pcidevice.h" // pci_probe_devices
>  #include "hw/pci_regs.h" // PCI_DEVICE_ID
>  #include "hw/serialio.h" // PORT_SERIAL1
> +#include "hw/virtio-mmio.h" // virtio_mmio_register
>  #include "hw/rtc.h" // CMOS_*
>  #include "malloc.h" // malloc_tmp
>  #include "output.h" // dprintf
> @@ -192,6 +193,27 @@ static void msr_feature_control_setup(void)
>          wrmsr_smp(MSR_IA32_FEATURE_CONTROL, feature_control_bits);
>  }
>
> +static void qemu_probe_virtio_mmio(void)
> +{
> char *data, *next;
> int size;
> u64 mmio;
> data = romfile_loadfile("bootorder", &size);
> while (data) {
>    next = strchr(data, '\n');
>    if (next) {
>        *next = '\0';
>        next++;
>    }
>    if (memcmp("/virtio-mmio@", data, 13) == 0) {
>        mmio = strtol(data+13, 16);
>        virtio_mmio_register(mmio);
>    }
>    data = next;
> }
>
> +}
>
> void
>  qemu_platform_setup(void)
>  {
> @@ -217,6 +239,8 @@ qemu_platform_setup(void)
>      msr_feature_control_setup();
>      smp_setup();
> qemu_probe_virtio_mmio();
> // Create bios tables
>    if (MaxCountCPUs <= 255) {
>        pirtable_setup();
>
> diff --git a/src/hw/virtio-mmio.c b/src/hw/virtio-mmio.c
> new file mode 100644
> index 000000000000..0f320921df30
> --- /dev/null
> +++ b/src/hw/virtio-mmio.c
> @@ -0,0 +1,30 @@
> +#include "config.h" // CONFIG_DEBUG_LEVEL
> +#include "malloc.h" // free
> +#include "output.h" // dprintf
> +#include "virtio-pci.h"
> +#include "virtio-ring.h"
>
> +/* qemu microvm supports 8 virtio-mmio devices */
> +static u64 devs[8];
>
> +void virtio_mmio_register(u64 mmio)
> +{
> int i;
> for (i = 0; i < ARRAY_SIZE(devs); i++) {
>    if (devs[i] == mmio) {
>        /*
>         * This can happen in case we have multiple scsi devices
>         * attached to a single virtio-scsi controller
>         */
>        dprintf(3, "virtio-mmio: duplicate device at 0x%llx, ignoring\n", mmio);
>        return;
>    }
>    if (devs[i] == 0) {
>        dprintf(1, "virtio-mmio: device at 0x%llx\n", mmio);
>        devs[i] = mmio;
>        return;
>    }
> }
> dprintf(1, "virtio-mmio: device list full\n");
> +}
>
> 2.18.2
> _______________________________________________
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Gerd Hoffmann]

On Thu, Mar 19, 2020 at 07:48:19PM -0400, Kevin O'Connor wrote:
> On Thu, Mar 19, 2020 at 08:39:33AM +0100, Gerd Hoffmann wrote:
> > Use bootorder fw_cfg file to find bootable virtio-mmio devices.
> > Also add a new virtio-mmio.c source file, providing a function
> > to register virtio-mmio devices.
>
> Can you expand on this?  I'm not sure I understand what this patch
> does.  It seems the bootorder file is used for device detection, but
> that seems odd.

Yes, it does exactly that.

virtio-mmio isn't a discoverable bus, so we need some way to find the
devices.  Right now qemu simply appends device information to the kernel
command line.  Seabios can look there too.  That works only for direct
kernel boot though, so I'm trying to find something better.  The
alternative is doing it like aarch64: declare devices in device tree, or
in ACPI tables.  device tree doesn't work very well in x86.  ACPI works
fine, so I think this is the way to go.

FYI: qemu patch series is here:
   https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06144.html

seabios hasn't an ACPI parser though.  (ab)using the bootorder file to
find the virtio-mmio devices is the only other option we have, and it
looked alot simpler than adding an ACPI parser ...

cheers,
  Gerd

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Kevin O'Connor]

On Fri, Mar 20, 2020 at 07:13:02AM +0100, Gerd Hoffmann wrote:
> On Thu, Mar 19, 2020 at 07:48:19PM -0400, Kevin O'Connor wrote:
> > On Thu, Mar 19, 2020 at 08:39:33AM +0100, Gerd Hoffmann wrote:
> > > Use bootorder fw_cfg file to find bootable virtio-mmio devices.
> > > Also add a new virtio-mmio.c source file, providing a function
> > > to register virtio-mmio devices.
> >
> > Can you expand on this?  I'm not sure I understand what this patch
> > does.  It seems the bootorder file is used for device detection, but
> > that seems odd.
>
> Yes, it does exactly that.
>
> virtio-mmio isn't a discoverable bus, so we need some way to find the
> devices.  Right now qemu simply appends device information to the kernel
> command line.  Seabios can look there too.  That works only for direct
> kernel boot though, so I'm trying to find something better.  The
> alternative is doing it like aarch64: declare devices in device tree, or
> in ACPI tables.  device tree doesn't work very well in x86.

I'd be leery of overloading the bootorder file in this way.  The QEMU
to firmware interface is already complex and additional quirks like
this make it harder to document and understand that interface.

Can we just introduce a new fw_cfg file (eg, /etc/virtio-mmio-devices)
with the required information?
> ACPI works
> fine, so I think this is the way to go.
>
> FYI: qemu patch series is here:
>    https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06144.html
>
> seabios hasn't an ACPI parser though.  (ab)using the bootorder file to
> find the virtio-mmio devices is the only other option we have, and it
> looked alot simpler than adding an ACPI parser ...

If you mean a DSDT parser then I suspect a full implementation in
SeaBIOS would be too burdensome.  However, it might be possible to
introduce a minimal DSDT parser (eg, one that only supports extracting
constants).

FWIW, it's unclear to me if adding a parser would be a net gain when
compared to adding adhoc fw_cfg files.  In particular, it seems some
users don't want to enable acpi.

Cheers,
-Kevin

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Gerd Hoffmann]

Hi,
> If you mean a DSDT parser then I suspect a full implementation in
> SeaBIOS would be too burdensome.  However, it might be possible to
> introduce a minimal DSDT parser (eg, one that only supports extracting
> constants).

https://git.kraxel.org/cgit/seabios/commit/?h=dsdt&id=d7cdf49ec624f42fa6...

~500 lines of code.

qemu microvm:

[ ... ]
dsdt at 0x7ffe0040 (len 335)
    dsdt: RTC, hid PNP0b00, io 0x70 -> 0x71, 1 io+, irq 8
    dsdt: COM1, hid PNP0501, io 0x3f8 -> 0x3ff, irq 4
    dsdt: FWCF, hid QEMU0002, io 0x510 -> 0x51b, sta 0xb
    dsdt: VR07, hid LNRO0005, mem 0xc0000e00 -> 0xc0000fff, irq 23
    dsdt: VR06, hid LNRO0005, mem 0xc0000c00 -> 0xc0000dff, irq 22
[ ... ]

_HID == LNRO0005 are the virtio-mmio devices, so this can work.

qemu pc:
[ ... ]
dsdt at 0x07fe0040 (len 5131)
    dsdt: PCI0, hid PNP0a03
    dsdt: HPET, hid PNP0103, mem 0xfed00000 -> 0xfed003ff, sta method
    dsdt: PX13
    dsdt: ISA
    dsdt: RTC, hid PNP0b00, io 0x70 -> 0x71, 1 io+, irq 8
    dsdt: KBD, hid PNP0303, io 0x60 -> 0x60, 1 io+, irq 1, sta method
    dsdt: MOU, hid PNP0f13, irq 12, sta method
    dsdt: FLPA
    dsdt: FLPA
    dsdt: LPT, hid PNP0400, io 0x378 -> 0x37f, irq 7, sta method
    dsdt: COM1, hid PNP0501, io 0x3f8 -> 0x3ff, irq 4, sta method
    dsdt: COM2, hid PNP0501, io 0x2f8 -> 0x2ff, irq 3, sta method
    dsdt: LNKA, hid PNP0c0f, sta method
    dsdt: LNKB, hid PNP0c0f, sta method
    dsdt: LNKC, hid PNP0c0f, sta method
    dsdt: LNKD, hid PNP0c0f, sta method
    dsdt: LNKS, hid PNP0c0f, sta method
    dsdt: _SB\PCI0\PRES, hid PNP0a06, io 0xaf00 -> 0xaf0b
    dsdt: _SB\CPUS, hid ACPI0010
    dsdt: GPE0, hid PNP0A06, io 0xafe0 -> 0xafe3, sta 0xb
    dsdt: PHPR, hid PNP0A06, io 0xae00 -> 0xae13, sta 0xb
    dsdt: FWCF, hid QEMU0002, io 0x510 -> 0x51b, sta 0xb
    dsdt: S00
[ ... ]

If we have this _anyway_ we might use it to figure whenever specific
(isa) hardware is present, keyboard for example.  We need to fix qemu
to not use _STA methods for no good reason, like this one ...

Device (KBD)
        {
            Name (_HID, EisaId ("PNP0303") 
            Method (_STA, 0, NotSerialized)
            {
                Return (0x0F)
            }

... so it can be checked without a full-blown acpi interpreter.
> FWIW, it's unclear to me if adding a parser would be a net gain when
> compared to adding adhoc fw_cfg files.  In particular, it seems some
> users don't want to enable acpi.

Well, at least for the microvm disk-boot use case there is not really a
way around ACPI, because otherwise the kernel doesn't find the
virtio-mmio devices.

With direct kernel boot (qemu -kernel) one can add the virtio-mmio
devices to the linux command line instead of depending on acpi, but in
that case seabios doesn't need to find virtio-mmio devices anyway so it
would be happy without acpi too.

cheers,
  Gerd

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Kevin O'Connor]

On Wed, Mar 25, 2020 at 10:35:50AM +0100, Gerd Hoffmann wrote:
> Hi,
> > If you mean a DSDT parser then I suspect a full implementation in
> > SeaBIOS would be too burdensome.  However, it might be possible to
> > introduce a minimal DSDT parser (eg, one that only supports extracting
> > constants).
>
> https://git.kraxel.org/cgit/seabios/commit/?h=dsdt&id=d7cdf49ec624f42fa6...
>
> ~500 lines of code.

Nice.

Could we just parse the static device tree into a local data structure
though?  Something like:

struct hlist_head DSDTTree;

struct dsdt_entry {
    struct hlist_node node;
    char *name;  // eg, "_SB.PCI0.ISA.COM1._HID"
    int type; // eg: Device, Name, Integer, String, ResourceTemplate, other
    void *value; // points to integer, string, resource template, or NULL
};

Once the tree is parsed, it should be simple to walk the in-memory
linked-list to find desired values later.
> If we have this _anyway_ we might use it to figure whenever specific
> (isa) hardware is present, keyboard for example.  We need to fix qemu
> to not use _STA methods for no good reason, like this one ...
>     Device (KBD)
>     {
>         Name (_HID, EisaId ("PNP0303") 
>         Method (_STA, 0, NotSerialized)
>         {
>             Return (0x0F)
>         }
>
> ... so it can be checked without a full-blown acpi interpreter.

FWIW, it should not be difficult to detect the simple case above and
treat it as simple integer locally.  As before, it does raise the
question of how far one would want to go down that path though.

Cheers,
-Kevin

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Gerd Hoffmann]

Hi,
> Could we just parse the static device tree into a local data structure
> though?

Sure.
> Something like:
>
> struct hlist_head DSDTTree;
>
> struct dsdt_entry {
>     struct hlist_node node;
>     char *name;  // eg, "_SB.PCI0.ISA.COM1._HID"
>     int type; // eg: Device, Name, Integer, String, ResourceTemplate, other
>     void *value; // points to integer, string, resource template, or NULL
> };
>
> Once the tree is parsed, it should be simple to walk the in-memory
> linked-list to find desired values later.

I was thinking more about storing a list of devices with parsed data,
i.e. basically put into a linked list what the current code collects
in the parse_dev struct.

Then we can easily lookup the virtio-mmio devices later.  Maybe also
check for isa devices (don't bother waiting for ps2 probe timeout if
acpi says there isn't a keyboard ...).  I don't see any other use
cases.
> FWIW, it should not be difficult to detect the simple case above and
> treat it as simple integer locally.  As before, it does raise the
> question of how far one would want to go down that path though.

I don't want handle methods.

cheers,
  Gerd

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Kevin O'Connor]

On Fri, Mar 27, 2020 at 09:12:06AM +0100, Gerd Hoffmann wrote:
> > Something like:
> >
> > struct hlist_head DSDTTree;
> >
> > struct dsdt_entry {
> >     struct hlist_node node;
> >     char *name;  // eg, "_SB.PCI0.ISA.COM1._HID"
> >     int type; // eg: Device, Name, Integer, String, ResourceTemplate, other
> >     void *value; // points to integer, string, resource template, or NULL
> > };
> >
> > Once the tree is parsed, it should be simple to walk the in-memory
> > linked-list to find desired values later.
>
> I was thinking more about storing a list of devices with parsed data,
> i.e. basically put into a linked list what the current code collects
> in the parse_dev struct.

FWIW, I suspect it will be painful to grow 'struct parse_dev' if there
are future users.  (For example, if there is more than one memory
range needed, if it is necessary to check for the existence of some
name other than _STA, etc. .)  In contrast, I suspect a few helper
functions that can walk the tree would be sufficient to extract the
info currently in 'struct parse_dev' as needed.
> Then we can easily lookup the virtio-mmio devices later.  Maybe also
> check for isa devices (don't bother waiting for ps2 probe timeout if
> acpi says there isn't a keyboard ...).  I don't see any other use
> cases.

It would be helpful to extract the location of builtin sdcards from
the dsdt (currently, the "etc/sdcard" cbfs file is used instead).

In the past, it may have been useful to extract information on ATA DMA
capabilities from the dsdt.  (Though, now, it seems unlikely that is
worthwhile doing.)

-Kevin

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Gerd Hoffmann]

On Fri, Mar 27, 2020 at 09:15:19AM -0400, Kevin O'Connor wrote:
> On Fri, Mar 27, 2020 at 09:12:06AM +0100, Gerd Hoffmann wrote:
> > > Something like:
> > >
> > > struct hlist_head DSDTTree;
> > >
> > > struct dsdt_entry {
> > >     struct hlist_node node;
> > >     char *name;  // eg, "_SB.PCI0.ISA.COM1._HID"
> > >     int type; // eg: Device, Name, Integer, String, ResourceTemplate, other
> > >     void *value; // points to integer, string, resource template, or NULL
> > > };
> > >
> > > Once the tree is parsed, it should be simple to walk the in-memory
> > > linked-list to find desired values later.
> >
> > I was thinking more about storing a list of devices with parsed data,
> > i.e. basically put into a linked list what the current code collects
> > in the parse_dev struct.
>
> FWIW, I suspect it will be painful to grow 'struct parse_dev' if there
> are future users.  (For example, if there is more than one memory
> range needed, if it is necessary to check for the existence of some
> name other than _STA, etc. .)  In contrast, I suspect a few helper
> functions that can walk the tree would be sufficient to extract the
> info currently in 'struct parse_dev' as needed.

Went with a hybrid approach now.  parse_dev is still there, but for the
most part part it stores pointers into the dsdt table and parsing
happens later as needed.

sneak preview @ https://git.kraxel.org/cgit/seabios/log/?h=dsdt
> > Then we can easily lookup the virtio-mmio devices later.  Maybe also
> > check for isa devices (don't bother waiting for ps2 probe timeout if
> > acpi says there isn't a keyboard ...).  I don't see any other use
> > cases.
>
> It would be helpful to extract the location of builtin sdcards from
> the dsdt (currently, the "etc/sdcard" cbfs file is used instead).

That'll be used with coreboot, right?  How do sdcard dsdt entries look
like?

cheers,
  Gerd

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Kevin O'Connor]

On Tue, Mar 31, 2020 at 04:53:02PM +0200, Gerd Hoffmann wrote:
> On Fri, Mar 27, 2020 at 09:15:19AM -0400, Kevin O'Connor wrote:
> > On Fri, Mar 27, 2020 at 09:12:06AM +0100, Gerd Hoffmann wrote:
> > > > Something like:
> > > >
> > > > struct hlist_head DSDTTree;
> > > >
> > > > struct dsdt_entry {
> > > >     struct hlist_node node;
> > > >     char *name;  // eg, "_SB.PCI0.ISA.COM1._HID"
> > > >     int type; // eg: Device, Name, Integer, String, ResourceTemplate, other
> > > >     void *value; // points to integer, string, resource template, or NULL
> > > > };
> > > >
> > > > Once the tree is parsed, it should be simple to walk the in-memory
> > > > linked-list to find desired values later.
> > >
> > > I was thinking more about storing a list of devices with parsed data,
> > > i.e. basically put into a linked list what the current code collects
> > > in the parse_dev struct.
> >
> > FWIW, I suspect it will be painful to grow 'struct parse_dev' if there
> > are future users.  (For example, if there is more than one memory
> > range needed, if it is necessary to check for the existence of some
> > name other than _STA, etc. .)  In contrast, I suspect a few helper
> > functions that can walk the tree would be sufficient to extract the
> > info currently in 'struct parse_dev' as needed.
>
> Went with a hybrid approach now.  parse_dev is still there, but for the
> most part part it stores pointers into the dsdt table and parsing
> happens later as needed.
>
> sneak preview @ https://git.kraxel.org/cgit/seabios/log/?h=dsdt

Thanks.  FWIW, I think the code would be simpler if it first parsed
the tree into an internal structure, and then searched for _STA, _HID,
etc.  Attempting to do both at the same time seems to complicate the
code.
> > > Then we can easily lookup the virtio-mmio devices later.  Maybe also
> > > check for isa devices (don't bother waiting for ps2 probe timeout if
> > > acpi says there isn't a keyboard ...).  I don't see any other use
> > > cases.
> >
> > It would be helpful to extract the location of builtin sdcards from
> > the dsdt (currently, the "etc/sdcard" cbfs file is used instead).
>
> That'll be used with coreboot, right?  How do sdcard dsdt entries look
> like?

Yes, with coreboot.  I don't recall the details right now - perhaps
Matt or John recall?

-Kevin

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Matt DeVillier]

On Tue, Mar 31, 2020 at 12:29 PM Kevin O'Connor  wrote:
> On Tue, Mar 31, 2020 at 04:53:02PM +0200, Gerd Hoffmann wrote:
> > On Fri, Mar 27, 2020 at 09:15:19AM -0400, Kevin O'Connor wrote:
> > > On Fri, Mar 27, 2020 at 09:12:06AM +0100, Gerd Hoffmann wrote:
> > > > > Something like:
> > > > >
> > > > > struct hlist_head DSDTTree;
> > > > >
> > > > > struct dsdt_entry {
> > > > >     struct hlist_node node;
> > > > >     char *name;  // eg, "_SB.PCI0.ISA.COM1._HID"
> > > > >     int type; // eg: Device, Name, Integer, String, ResourceTemplate, other
> > > > >     void *value; // points to integer, string, resource template, or NULL
> > > > > };
> > > > >
> > > > > Once the tree is parsed, it should be simple to walk the in-memory
> > > > > linked-list to find desired values later.
> > > >
> > > > I was thinking more about storing a list of devices with parsed data,
> > > > i.e. basically put into a linked list what the current code collects
> > > > in the parse_dev struct.
> > >
> > > FWIW, I suspect it will be painful to grow 'struct parse_dev' if there
> > > are future users.  (For example, if there is more than one memory
> > > range needed, if it is necessary to check for the existence of some
> > > name other than _STA, etc. .)  In contrast, I suspect a few helper
> > > functions that can walk the tree would be sufficient to extract the
> > > info currently in 'struct parse_dev' as needed.
> >
> > Went with a hybrid approach now.  parse_dev is still there, but for the
> > most part part it stores pointers into the dsdt table and parsing
> > happens later as needed.
> >
> > sneak preview @ https://git.kraxel.org/cgit/seabios/log/?h=dsdt
>
> Thanks.  FWIW, I think the code would be simpler if it first parsed
> the tree into an internal structure, and then searched for _STA, _HID,
> etc.  Attempting to do both at the same time seems to complicate the
> code.
> > > > Then we can easily lookup the virtio-mmio devices later.  Maybe also
> > > > check for isa devices (don't bother waiting for ps2 probe timeout if
> > > > acpi says there isn't a keyboard ...).  I don't see any other use
> > > > cases.
> > >
> > > It would be helpful to extract the location of builtin sdcards from
> > > the dsdt (currently, the "etc/sdcard" cbfs file is used instead).
> >
> > That'll be used with coreboot, right?  How do sdcard dsdt entries look
> > like?
>
> Yes, with coreboot.  I don't recall the details right now - perhaps
> Matt or John recall?

currently, the etc/sdcard entries point to the PCI BAR0 for the
eMMC/SD controllers on Baytrail/Braswell devices which coreboot puts
into ACPI mode. Those values are somewhat build-specific (ie, they
differ between the stock Google firmware and upstream coreboot), and
are extracted from the cbmem log of a booted system. They aren't
directly contained in the DSDT anywhere either -- you'll just find
references to BAR0 inside the EMMC/SDCD objects, which is a GNVS
variable.
> -Kevin

[PATCH 2/6] paravirt/qemu: virtio-mmio device discovery

[Gerd Hoffmann]

Hi,
> > Went with a hybrid approach now.  parse_dev is still there, but for the
> > most part part it stores pointers into the dsdt table and parsing
> > happens later as needed.
> >
> > sneak preview @ https://git.kraxel.org/cgit/seabios/log/?h=dsdt
>
> Thanks.  FWIW, I think the code would be simpler if it first parsed
> the tree into an internal structure, and then searched for _STA, _HID,
> etc.

I don't think so.  When just storing everything the data structures
needed become more complicated, we'll need a tree then and can't work
with a simple linked list for the devices found.

But, yes, we don't need to actually decode anything while parsing the
tree (except for the namestrings).  We can simply store pointers into
the aml and decode things later when extracting information.

I'll send the full patch series in a moment.

cheers,
  Gerd