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
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- 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++; + } +}
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 kraxel@redhat.com --- 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"); +}
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 kraxel@redhat.com --- 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); +}
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 kraxel@redhat.com --- 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);
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 kraxel@redhat.com --- 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;
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 kraxel@redhat.com --- 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);
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 kraxel@redhat.com
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 -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
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
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
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
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
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
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
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
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
On Tue, Mar 31, 2020 at 12:29 PM Kevin O'Connor kevin@koconnor.net 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
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