November 2021 - SeaBIOS - mail.coreboot.org

Re: INT 14H
by Paul Edwards Sept. 29, 2022

Sept. 29, 2022

Hi Gerd. Thanks for your reply. Sorry for the delay, was swamped with other things. >> Currently I don't know what to recommend, because I >> don't understand the other side of the INT 13H/14H, >> only the OS side. > Well, the other side talks to the hardware, > and needs drivers to do so. See src/hw > Surely one can write a serial driver which connects to a remote place > using some bluetooth protocol instead of talking to a 16550. That'll > be *alot* of work though, you basically have to implement both > bluetooth stack and hardware driver. I expected to assemble, rather than write, such a thing. If not from the Linux source code, then FreeBSD or ReactOS. If for some reason (what would that be?) no-one has abstracted bluetooth to have a clean interface that I could have trivially reused, then I will put aside my bluetooth dreams and switch to a different technology. How about Wifi? Does that exist in a state usable for my purposes? >> Sure. For now, I want PDOS/386 to remain traditional. >> When I have exhausted everything that is possible >> via the BIOS, I will consider adding UEFI support. > If you want your firmware provide drivers to you for modern hardware you > are in a *much* better position with UEFI. Well, on some/many/most modern machines there is absolutely no other choice - the BIOS doesn't exist, even as CSM. > There is a bluetooth > protocol specification for example, although I'm not sure how common it > is to find an actual uefi driver for bluetooth hardware in the firmware. > Talking to ethernet using the firmware-provided uefi driver shouldn't be > much of a problem though b/c network boot is a rather standard feature. Ok, maybe in the medium term I should use a hardware device, forget the name, that turns ethernet into Wifi. So then the problem is reduced to me needing to provide a BOOTX64.EFI that converts INT 14H into UEFI ethernet calls. >> But I'd like to transport all of that software, designed for >> that environment, and change nothing at all, and have >> it work on a new environment that includes Wifi, bluetooth >> and maybe ethernet, and maybe infrared, and maybe >> other things I don't know about, but make sense (at >> some level) to be accessed via INT 14H. > Well. In the server world it is rather common to provide access to the > serial line over the network for system management purposes. > One approach for that is to have a separate management controller (bmc), > and the serial port of the machine is linked to the management > controller instead of a D9 socket. You can then connect to the bmc to > manage the machine, and one of the options available is to get serial > console access. Ok, this would allow me to have a BBS. But not allow me to have a "virtual modem" so that I can dial out. > Another approach is to have a virtual 16550 provided by the firmware. > Accessing the virtual 16550 will trap into SMM mode where the firmware > emulates a serial device and allows to access the serial port remotely > over the network, roughly comparable to how qemu provides an virtual > 16550 to virtual machines. Ok, but in the case of qemu I have the same issue - at the end of the day I want to drive a modem, so with qemu I can get it to talk to a "virtual modem". If the firmware is providing the serial to network conversion, I also need to stick in a virtual modem there. And that's what I was hoping SeaBIOS would allow me to do. > Note that both approaches work at hardware level not int14h level, > because modern operating systems use int14h services in bootloaders > only if at all. My operating system, PDOS/386, may not be considered "modern", even though it is in active development, but I do wish to use INT 14H, as that was the traditional BIOS service provided, and I wish to maintain contact with the original 80386 machines. I expect a modern environment to have the CPU, memory etc required to support this basic interface. BFN. Paul.

2 4

[PATCH] fw/coreboot.c: Use coreboot table to find cbfs
by Arthur Heymans April 21, 2022

April 21, 2022

The "cbfs master header" cbfs file is considered a legacy feature in coreboot and is planned for removal in the master branch. Since 2015 the coreboot tables have exported info about the active cbfs. This change uses the cbfs information in the coreboot tables instead of following the cbfs master header pointer in the bootblock to find cbfs. This change makes it possible to access CBFS fmap regions that don't feature a "cbfs master header", which is the case with Googles VBOOT solution. This breaks compatibility with very old coreboot build (build before fb5d5b16 "2015-07-14, cbtable: describe boot media"). Keeping backward compatibility with the "cbfs master header" would complicate the code. Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> diff --git a/src/Kconfig b/src/Kconfig index 3a8ffa1..51ba827 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -93,15 +93,6 @@ endchoice help Support CBFS files compressed using the lzma decompression algorithm. - config CBFS_LOCATION - depends on COREBOOT_FLASH - hex "CBFS memory end location" - default 0 - help - Memory address of where the CBFS data ends. This should - be zero for normal builds. It may be a non-zero value if - the CBFS filesystem is at a non-standard location (eg, - 0xffe00000 if CBFS ends 2Meg below the end of flash). config MULTIBOOT depends on COREBOOT diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c index 7c0954b..8a35c1d 100644 --- a/src/fw/coreboot.c +++ b/src/fw/coreboot.c @@ -88,6 +88,18 @@ struct cbmem_console { #define CBMC_OVERFLOW (1 << 31) static struct cbmem_console *cbcon = NULL; +#define CB_TAG_BOOT_MEDIA_PARAMS 0x30 + +struct cb_boot_media_params { + u32 tag; + u32 size; + /* offsets are relative to start of boot media */ + u64 fmap_offset; + u64 cbfs_offset; + u64 cbfs_size; + u64 boot_media_size; +}; + static u16 ipchksum(char *buf, int count) { @@ -146,7 +158,12 @@ find_cb_subtable(struct cb_header *cbh, u32 tag) struct cb_header * find_cb_table(void) { - struct cb_header *cbh = find_cb_header(0, 0x1000); + static struct cb_header *cbh; + + if (cbh) + return cbh; + + cbh= find_cb_header(0, 0x1000); if (!cbh) return NULL; struct cb_forward *cbf = find_cb_subtable(cbh, CB_TAG_FORWARD); @@ -317,20 +334,8 @@ ulzma(u8 *dst, u32 maxlen, const u8 *src, u32 srclen) * Coreboot flash format ****************************************************************/ -#define CBFS_HEADER_MAGIC 0x4F524243 -#define CBFS_VERSION1 0x31313131 - -struct cbfs_header { - u32 magic; - u32 version; - u32 romsize; - u32 bootblocksize; - u32 align; - u32 offset; - u32 pad[2]; -} PACKED; - #define CBFS_FILE_MAGIC 0x455649484352414cLL // LARCHIVE +#define CBFS_ALIGNMENT 64 struct cbfs_file { u64 magic; @@ -429,30 +434,50 @@ coreboot_cbfs_init(void) if (!CONFIG_COREBOOT_FLASH) return; - struct cbfs_header *hdr = *(void **)(CONFIG_CBFS_LOCATION - 4); - if ((u32)hdr & 0x03) { - dprintf(1, "Invalid CBFS pointer %p\n", hdr); + dprintf(3, "Attempting to initialize coreboot cbfs\n"); + + struct cb_header *cbh = find_cb_table(); + if (!cbh) { + dprintf(1, "cbfs_init: unable to find cb table\n"); return; } - if (CONFIG_CBFS_LOCATION && (u32)hdr > CONFIG_CBFS_LOCATION) - // Looks like the pointer is relative to CONFIG_CBFS_LOCATION - hdr = (void*)hdr + CONFIG_CBFS_LOCATION; - if (hdr->magic != cpu_to_be32(CBFS_HEADER_MAGIC)) { - dprintf(1, "Unable to find CBFS (ptr=%p; got %x not %x)\n" - , hdr, hdr->magic, cpu_to_be32(CBFS_HEADER_MAGIC)); + + struct cb_boot_media_params *cbbmp = + find_cb_subtable(cbh, CB_TAG_BOOT_MEDIA_PARAMS); + + if (!cbbmp) { + dprintf(1, "cbfs_init: unable to find boot media params\n"); return; } - dprintf(1, "Found CBFS header at %p\n", hdr); - u32 romsize = be32_to_cpu(hdr->romsize); - u32 romstart = CONFIG_CBFS_LOCATION - romsize; - struct cbfs_file *fhdr = (void*)romstart + be32_to_cpu(hdr->offset); + const u32 romsize = cbbmp->boot_media_size; + const u32 cbfs_start = cbbmp->cbfs_offset - romsize; + const u32 cbfs_end = cbfs_start + cbbmp->cbfs_size - 1; + + dprintf(3, "cbfs_init: cbfs_start=0x%08x, cbfs_end=0x%08x\n", cbfs_start, + cbfs_end); + + u32 offset = cbfs_start; + for (;;) { - if ((u32)fhdr - romstart > romsize) - break; - u64 magic = fhdr->magic; - if (magic != CBFS_FILE_MAGIC) + if (offset > cbfs_end || offset < cbfs_start) break; + + struct cbfs_file *fhdr = (void*)offset; + + if (fhdr->magic != CBFS_FILE_MAGIC) { + offset++; + offset = ALIGN(offset, CBFS_ALIGNMENT); + continue; + } + + /* Skip empty space */ + if (strcmp(fhdr->filename, "") == 0) { + offset += ALIGN(be32_to_cpu(fhdr->offset) + be32_to_cpu(fhdr->len) + , CBFS_ALIGNMENT); + continue; + } + struct cbfs_romfile_s *cfile = malloc_tmp(sizeof(*cfile)); if (!cfile) { warn_noalloc(); @@ -473,8 +498,8 @@ coreboot_cbfs_init(void) } romfile_add(&cfile->file); - fhdr = (void*)ALIGN((u32)cfile->data + cfile->rawsize - , be32_to_cpu(hdr->align)); + offset = ALIGN((u32)cfile->data + cfile->rawsize + , CBFS_ALIGNMENT); } process_links_file(); -- 2.30.2

4 8

[PATCH v3 0/4] nvme: Add PRP List support
by Alexander Graf Jan. 18, 2022

Jan. 18, 2022

While looking at VM bootup times, we stumbled over the fact that the NVMe code only does I/O operations of up to 4kb at a given point in time. This is usually ok, but if you have an OS that loads a lot of data on boot in combination to network backed storage, it shows in bootup times. There is no need to restrict ourselves to 4kb though. The INT13 call we receive gives us much larger chunks which we can just map into a native bigger NVMe I/O call if the request buffer is page aligned. This patch implements all logic required to do the above and gives a substantial performance boost on boot. v1 -> v2: - fix dprintf - Fix bounds check on PRP list add logic - Reduce PRP list to 15 entries embedded in the ns struct. This reduces BIOS reserved memory footprint by 4kb. v2 -> v3: - new patch: nvme: Split requests by maximum allowed size - Adjust the maximum request size to sector units. The hardware field describes it in pages. Alexander Graf (4): nvme: Record maximum allowed request size nvme: Allow to set PRP2 nvme: Pass large I/O requests as PRP lists nvme: Split requests by maximum allowed size src/hw/nvme-int.h | 16 ++++++- src/hw/nvme.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 121 insertions(+), 17 deletions(-) -- 2.16.4 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879

6 16

new november release?
by Gerd Hoffmann Jan. 14, 2022

Jan. 14, 2022

Hi, It's been a while since the last release, we have some nice but unreleased bits in the master branch (tpm and nvme improvements for example), time for a new release I think. How about freeze in a week, release by roughly mid-november? take care, Gerd

3 13

[PATCH v3 0/2] pci: reserve resources for pcie-pci-bridge to fix regressed hotplug on q35 with ACPI hotplug enabled
by Igor Mammedov Dec. 18, 2021

Dec. 18, 2021

Since QEMU-6.1, Q35 machine is using ACPI PCI hotplug by default. However SeaBIOS did not take in account pcie-pci-bridge and treated it as any other PCIe device which is not correct in the case of the bridge. Patch [1/2] fixes main issue by treating the bridge as a device with possible SHPC capability. Patch [2/2] fixes missed IO reservation, by making sure that in the case of the bridge firmware reserves IO resource. Igor Mammedov (2): pci: reserve resources for pcie-pci-bridge to fix regressed hotplug on q35 pci: let firmware reserve IO for pcie-pci-bridge src/fw/pciinit.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) -- 2.27.0

5 9

[PATCH 00/19] Support SMBIOS 3.0 entry points
by Eduardo Habkost Dec. 18, 2021

Dec. 18, 2021

This series implements support for SMBIOS 3.0 entry points in SeaBIOS. The main advantage of SMBIOS 3.0 entry points is the higher limit for total table size. The SMBIOS 2.1 64435 bytes limit can be easily hit in QEMU if creating virtual machines with more than 720 VCPUs. Eduardo Habkost (19): biostables: copy_fseg_table() function util.h: Delete unused get_smbios_entry_point() prototype smbios: Rename code specific for SMBIOS 2.1 entry points smbios: Generic smbios_next() function smbios: smbios_get_tables() function smbios: Use smbios_get_tables()/smbios_next() at display_uuid() smbios: smbios_major_version()/smbios_minor_version() helpers tpm: Use smbios_get_tables() csm: Don't check SMBios21Addr before calling copy_smbios_21() smbios: Make SMBios21Addr variable static smbios: Use smbios_next() at smbios_romfile_setup() smbios: Extract SMBIOS table building code to separate function smbios: Make smbios_build_tables() more generic smbios: smbios_21_setup_entry_point() function smbios: Make some smbios_build_tables() arguments optional smbios: Make smbios_build_tables() ready for 64-bit tables smbios: copy_smbios_30() function smbios: Support SMBIOS 3.0 entry point at copy_table() smbios: Support SMBIOS 3.0 entry point at smbios_romfile_setup() src/std/smbios.h | 17 ++- src/util.h | 5 +- src/fw/biostables.c | 259 +++++++++++++++++++++++++++++++++----------- src/fw/csm.c | 6 +- src/fw/smbios.c | 12 +- src/tcgbios.c | 12 +- 6 files changed, 227 insertions(+), 84 deletions(-) -- 2.28.0

3 24

[PATCH] vgasrc/svgamodes: added 4k resolution
by vitaly＠cyberhaven.com Dec. 13, 2021

Dec. 13, 2021

From: Vitaly Chipounov <vitaly(a)cyberhaven.com> This requires at least 32MB of video memory. Tested with Windows 10's default display driver and virtio-vga. Signed-off-by: Vitaly Chipounov <vitaly(a)cyberhaven.com> --- vgasrc/bochsvga.h | 4 ++-- vgasrc/svgamodes.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/vgasrc/bochsvga.h b/vgasrc/bochsvga.h index ae5f75d..f08219f 100644 --- a/vgasrc/bochsvga.h +++ b/vgasrc/bochsvga.h @@ -6,8 +6,8 @@ #define VBE_DISPI_BANK_ADDRESS 0xA0000 #define VBE_DISPI_BANK_SIZE_KB 64 -#define VBE_DISPI_MAX_XRES 2560 -#define VBE_DISPI_MAX_YRES 1600 +#define VBE_DISPI_MAX_XRES 3840 +#define VBE_DISPI_MAX_YRES 2160 #define VBE_DISPI_IOPORT_INDEX 0x01CE #define VBE_DISPI_IOPORT_DATA 0x01CF diff --git a/vgasrc/svgamodes.c b/vgasrc/svgamodes.c index 6e494c7..63ef040 100644 --- a/vgasrc/svgamodes.c +++ b/vgasrc/svgamodes.c @@ -92,5 +92,6 @@ struct generic_svga_mode svga_modes[] VAR16 = { { 0x196, { MM_DIRECT, 2560, 1440, 16, 8, 16, SEG_GRAPH } }, { 0x197, { MM_DIRECT, 2560, 1440, 24, 8, 16, SEG_GRAPH } }, { 0x198, { MM_DIRECT, 2560, 1440, 32, 8, 16, SEG_GRAPH } }, + { 0x199, { MM_DIRECT, 3840, 2160, 32, 8, 16, SEG_GRAPH } }, }; unsigned int svga_mcount VAR16 = ARRAY_SIZE(svga_modes); -- 2.25.1

5 4

[PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
by Andy Pei Nov. 30, 2021

Nov. 30, 2021

according to virtio spec, add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX parse to virtio blk driver. Signed-off-by: Ding Limin <dinglimin(a)cmss.chinamobile.com> Signed-off-by: Andy Pei <andy.pei(a)intel.com> --- src/block.h | 2 ++ src/hw/virtio-blk.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++-- src/hw/virtio-blk.h | 3 +++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/block.h b/src/block.h index c1b8d73..3af2d71 100644 --- a/src/block.h +++ b/src/block.h @@ -57,6 +57,8 @@ struct drive_s { u8 translation; // type of translation u16 blksize; // block size struct chs_s pchs; // Physical CHS + u32 max_segment_size; //max_segment_size + u32 max_segments; //max_segments }; #define DISK_SECTOR_SIZE 512 diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 3b19896..7935f0f 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -121,12 +121,16 @@ init_virtio_blk(void *data) u64 version1 = 1ull << VIRTIO_F_VERSION_1; u64 iommu_platform = 1ull << VIRTIO_F_IOMMU_PLATFORM; u64 blk_size = 1ull << VIRTIO_BLK_F_BLK_SIZE; + u64 max_segments = 1ull << VIRTIO_BLK_F_SEG_MAX; + u64 max_segment_size = 1ull << VIRTIO_BLK_F_SIZE_MAX; + if (!(features & version1)) { dprintf(1, "modern device without virtio_1 feature bit: %pP\n", pci); goto fail; } - features = features & (version1 | iommu_platform | blk_size); + features = features & (version1 | iommu_platform | blk_size + | max_segments | max_segment_size); vp_set_features(vp, features); status |= VIRTIO_CONFIG_S_FEATURES_OK; vp_set_status(vp, status); @@ -135,6 +139,22 @@ init_virtio_blk(void *data) goto fail; } + if (features & max_segment_size) + vdrive->drive.max_segment_size = + vp_read(&vp->device, struct virtio_blk_config, size_max); + else + vdrive->drive.max_segment_size = 0; + dprintf(3, "virtio-blk %pP size_max=%u.\n", pci, + vdrive->drive.max_segment_size); + + if (features & max_segments) + vdrive->drive.max_segments = + vp_read(&vp->device, struct virtio_blk_config, seg_max); + else + vdrive->drive.max_segments = 0; + dprintf(3, "virtio-blk %pP seg_max=%u.\n", pci, + vdrive->drive.max_segments); + vdrive->drive.sectors = vp_read(&vp->device, struct virtio_blk_config, capacity); if (features & blk_size) { @@ -177,6 +197,21 @@ init_virtio_blk(void *data) vdrive->drive.pchs.cylinder = cfg.cylinders; vdrive->drive.pchs.head = cfg.heads; vdrive->drive.pchs.sector = cfg.sectors; + + if (f & (1 << VIRTIO_BLK_F_SIZE_MAX)) + vdrive->drive.max_segment_size = cfg.size_max; + else + vdrive->drive.max_segment_size = 0; + dprintf(3, "virtio-blk %pP size_max=%u.\n", pci, + vdrive->drive.max_segment_size); + + if (f & (1 << VIRTIO_BLK_F_SEG_MAX)) + vdrive->drive.max_segments = cfg.seg_max; + else + vdrive->drive.max_segments = 0; + dprintf(3, "virtio-blk %pP seg_max=%u.\n", pci, + vdrive->drive.max_segments); + } char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%pP", pci); @@ -218,8 +253,11 @@ init_virtio_blk_mmio(void *mmio) u64 features = vp_get_features(vp); u64 version1 = 1ull << VIRTIO_F_VERSION_1; u64 blk_size = 1ull << VIRTIO_BLK_F_BLK_SIZE; + u64 max_segments = 1ull << VIRTIO_BLK_F_SEG_MAX; + u64 max_segment_size = 1ull << VIRTIO_BLK_F_SIZE_MAX; - features = features & (version1 | blk_size); + features = features & (version1 | blk_size + | max_segments | max_segment_size); vp_set_features(vp, features); status |= VIRTIO_CONFIG_S_FEATURES_OK; vp_set_status(vp, status); @@ -228,6 +266,20 @@ init_virtio_blk_mmio(void *mmio) goto fail; } + if (features & max_segment_size) + vdrive->drive.max_segment_size = + vp_read(&vp->device, struct virtio_blk_config, size_max); + else + vdrive->drive.max_segment_size = 0; + dprintf(3, "virtio-blk size_max=%u.\n", vdrive->drive.max_segment_size); + + if (features & max_segments) + vdrive->drive.max_segments = + vp_read(&vp->device, struct virtio_blk_config, seg_max); + else + vdrive->drive.max_segments = 0; + dprintf(3, "virtio-blk seg_max=%u.\n", vdrive->drive.max_segments); + vdrive->drive.sectors = vp_read(&vp->device, struct virtio_blk_config, capacity); if (features & blk_size) { diff --git a/src/hw/virtio-blk.h b/src/hw/virtio-blk.h index d20461a..0294291 100644 --- a/src/hw/virtio-blk.h +++ b/src/hw/virtio-blk.h @@ -16,6 +16,9 @@ struct virtio_blk_config u32 opt_io_size; } __attribute__((packed)); +/* Feature bits */ +#define VIRTIO_BLK_F_SIZE_MAX 1 /* Maximum size of any single segment */ +#define VIRTIO_BLK_F_SEG_MAX 2 /* Maximum number of segments in a request */ #define VIRTIO_BLK_F_BLK_SIZE 6 /* These two define direction. */ -- 1.8.3.1

3 3

[PATCH v2] pci: reserve resources for pcie-pci-bridge to fix regressed hotplug on q35 with ACPI hotplug enabled
by Igor Mammedov Nov. 29, 2021

Nov. 29, 2021

If QEMU is started with unpopulated pcie-pci-bridge with ACPI PCI hotplug enabled (default since QEMU-6.1), hotplugging a PCI device into one of the bridge slots fails due to lack of resources. #QEMU-6.2-rc1 -machine q35 -device pcie-pci-bridge,id=pcie-pci-bridge-0,addr=0x4 once linux guest is booted (test used Fedora 34), hotplug NIC from QEMU monitor: (qemu) device_add rtl8139,bus=pcie-pci-bridge-0,addr=0x2 guest fails hotplug with: pci 0000:01:02.0: [10ec:8139] type 00 class 0x020000 pci 0000:01:02.0: reg 0x10: [io 0x0000-0x00ff] pci 0000:01:02.0: reg 0x14: [mem 0x00000000-0x000000ff] pci 0000:01:02.0: reg 0x30: [mem 0x00000000-0x0003ffff pref] pci 0000:01:02.0: BAR 6: no space for [mem size 0x00040000 pref] pci 0000:01:02.0: BAR 6: failed to assign [mem size 0x00040000 pref] pci 0000:01:02.0: BAR 0: no space for [io size 0x0100] pci 0000:01:02.0: BAR 0: failed to assign [io size 0x0100] pci 0000:01:02.0: BAR 1: no space for [mem size 0x00000100] pci 0000:01:02.0: BAR 1: failed to assign [mem size 0x00000100] 8139cp: 8139cp: 10/100 PCI Ethernet driver v1.3 (Mar 22, 2004) PCI Interrupt Link [GSIG] enabled at IRQ 22 8139cp 0000:01:02.0: no MMIO resource 8139cp: probe of 0000:01:02.0 failed with error -5 Reason for this is that commit [1] didn't take into account pcie-pci-bridge, marking bridge as non hotpluggable instead of handling it as possibly SHPC capable bridge. Fix issue by checking if pcie-pci-bridge is SHPC capable and if it is mark it as hotpluggable. With this hotplug of rtl8139 succeeds, with caveat that it fails initialize IO bar, which is caused by [2] which makes firmware skip IO reservation for any PCI device which isn't correct in case of pcie-pci-bridge. Fix it by exposing hotplug type and making resource optional only if PCIe hotplug is in use. Fixes regression in QEMU-6.1 and later, since it was switched to ACPI based PCI hotplug on Q35 by default at that time. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2001732 [1] Fixes: 3aa31d7d637 ("hw/pci: reserve IO and mem for pci express downstream ports with no devices attached") [2] Fixes: 76327b9f32a ("fw/pci: do not automatically allocate IO region for PCIe bridges") Signed-off-by: Igor Mammedov <imammedo(a)redhat.com> CC: mapfelba(a)redhat.com CC: kraxel(a)redhat.com CC: mst(a)redhat.com CC: lvivier(a)redhat.com CC: jusual(a)redhat.com --- src/fw/pciinit.c | 49 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index d25931bb..a6686abe 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -793,7 +793,13 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, return entry; } -static int pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap) +typedef enum hotplug_type_t { + HOTPLUG_NO_SUPPORTED = 0, + HOTPLUG_PCIE, + HOTPLUG_SHPC +} hotplug_type_t; + +static hotplug_type_t pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap) { u8 shpc_cap; @@ -804,22 +810,31 @@ static int pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap) (__builtin_ffs(PCI_EXP_FLAGS_TYPE) - 1)); u8 downstream_port = (port_type == PCI_EXP_TYPE_DOWNSTREAM) || (port_type == PCI_EXP_TYPE_ROOT_PORT); - /* - * PCI Express SPEC, 7.8.2: - * Slot Implemented – When Set, this bit indicates that the Link - * HwInit associated with this Port is connected to a slot (as - * compared to being connected to a system-integrated device or - * being disabled). - * This bit is valid for Downstream Ports. This bit is undefined - * for Upstream Ports. - */ - u16 slot_implemented = pcie_flags & PCI_EXP_FLAGS_SLOT; - - return downstream_port && slot_implemented; + + switch (port_type) { + case PCI_EXP_TYPE_PCI_BRIDGE: + /* do nothing and check later if SHPC is enabled */ + break; + default: { + /* + * PCI Express SPEC, 7.8.2: + * Slot Implemented – When Set, this bit indicates that the Link + * HwInit associated with this Port is connected to a slot (as + * compared to being connected to a system-integrated device or + * being disabled). + * This bit is valid for Downstream Ports. This bit is undefined + * for Upstream Ports. + */ + u16 slot_implemented = pcie_flags & PCI_EXP_FLAGS_SLOT; + return downstream_port && slot_implemented ? + HOTPLUG_PCIE : HOTPLUG_NO_SUPPORTED; + break; + } + } } shpc_cap = pci_find_capability(bus->bus_dev->bdf, PCI_CAP_ID_SHPC, 0); - return !!shpc_cap; + return !!shpc_cap ? HOTPLUG_SHPC : HOTPLUG_NO_SUPPORTED; } /* Test whether bridge support forwarding of transactions @@ -904,7 +919,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) u8 pcie_cap = pci_find_capability(bdf, PCI_CAP_ID_EXP, 0); u8 qemu_cap = pci_find_resource_reserve_capability(bdf); - int hotplug_support = pci_bus_hotplug_support(s, pcie_cap); + hotplug_type_t hotplug_support = pci_bus_hotplug_support(s, pcie_cap); for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; @@ -948,7 +963,9 @@ static int pci_bios_check_devices(struct pci_bus *busses) if (pci_region_align(&s->r[type]) > align) align = pci_region_align(&s->r[type]); u64 sum = pci_region_sum(&s->r[type]); - int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO); + int resource_optional = 0; + if (hotplug_support == HOTPLUG_PCIE) + resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO); if (!sum && hotplug_support && !resource_optional) sum = align; /* reserve min size for hot-plug */ if (size > sum) { -- 2.27.0

2 1

[PATCH] pci: reserve resources for pcie-pci-bridge to fix regressed hotplug on q35 with ACPI hotplug enabled
by Igor Mammedov Nov. 27, 2021

Nov. 27, 2021

hotplug of a PCI device to empty at startup pcie-pci-bridge fails when ACPI PCI hotplug (default since 6.1) is enabled due to lack of resources. Reproduced with: #QEMU-6.2-rc1 -machine q35 -device pcie-pci-bridge,id=pcie-pci-bridge-0,addr=0x4 once linux guest is booted (test used Fedora 34), hotplug NIC from QEMU monitor: (qemu) device_add rtl8139,bus=pcie-pci-bridge-0,addr=0x2 guest fails hotplug with: pci 0000:01:02.0: [10ec:8139] type 00 class 0x020000 pci 0000:01:02.0: reg 0x10: [io 0x0000-0x00ff] pci 0000:01:02.0: reg 0x14: [mem 0x00000000-0x000000ff] pci 0000:01:02.0: reg 0x30: [mem 0x00000000-0x0003ffff pref] pci 0000:01:02.0: BAR 6: no space for [mem size 0x00040000 pref] pci 0000:01:02.0: BAR 6: failed to assign [mem size 0x00040000 pref] pci 0000:01:02.0: BAR 0: no space for [io size 0x0100] pci 0000:01:02.0: BAR 0: failed to assign [io size 0x0100] pci 0000:01:02.0: BAR 1: no space for [mem size 0x00000100] pci 0000:01:02.0: BAR 1: failed to assign [mem size 0x00000100] 8139cp: 8139cp: 10/100 PCI Ethernet driver v1.3 (Mar 22, 2004) PCI Interrupt Link [GSIG] enabled at IRQ 22 8139cp 0000:01:02.0: no MMIO resource 8139cp: probe of 0000:01:02.0 failed with error -5 Reason for this is that commit [1] didn't take into account pcie-pci-bridge, marking bridge as non hotpluggable instead of handling it as possibly SHPC capable bridge. Fix issue by checking if pcie-pci-bridge is SHPC capable and if it is mark it as hotpluggable. With this hotplug of rtl8139 succeeds, with caveat that it fail initialize IO bar, which is caused by [2] which makes firmware skip IO reservation for any PCI device which isn't correct in case of pcie-pci-bridge. Fix it by exposing hotplug type and making resource optional only if PCIe hotplug is in use. Fixes regression in QEMU-6.1 and later, since it's switched to ACPI based PCI hotplug on Q35 by default at that time. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2001732 [1] Fixes: 3aa31d7d637 ("hw/pci: reserve IO and mem for pci express downstream ports with no devices attached") [2] Fixes: 76327b9f32a ("fw/pci: do not automatically allocate IO region for PCIe bridges") Signed-off-by: Igor Mammedov <imammedo(a)redhat.com> CC: mapfelba(a)redhat.com CC: kraxel(a)redhat.com CC: mst(a)redhat.com CC: lvivier(a)redhat.com CC: jusual(a)redhat.com --- src/fw/pciinit.c | 49 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index d25931bb..3107a0e6 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -793,7 +793,13 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, return entry; } -static int pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap) +typedef enum hotplug_type_t { + HOTPLUG_NO_SUPPORTED = 0, + HOTPLUG_PCIE, + HOTPLUG_SHPC +} hotplug_type_t; + +static hotplug_type_t pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap) { u8 shpc_cap; @@ -804,22 +810,31 @@ static int pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap) (__builtin_ffs(PCI_EXP_FLAGS_TYPE) - 1)); u8 downstream_port = (port_type == PCI_EXP_TYPE_DOWNSTREAM) || (port_type == PCI_EXP_TYPE_ROOT_PORT); - /* - * PCI Express SPEC, 7.8.2: - * Slot Implemented – When Set, this bit indicates that the Link - * HwInit associated with this Port is connected to a slot (as - * compared to being connected to a system-integrated device or - * being disabled). - * This bit is valid for Downstream Ports. This bit is undefined - * for Upstream Ports. - */ - u16 slot_implemented = pcie_flags & PCI_EXP_FLAGS_SLOT; - - return downstream_port && slot_implemented; + + switch (port_type) { + case PCI_EXP_TYPE_PCI_BRIDGE: + /* fall-through to check if SHPC is enabled on bridge */ + break; + default: { + /* + * PCI Express SPEC, 7.8.2: + * Slot Implemented – When Set, this bit indicates that the Link + * HwInit associated with this Port is connected to a slot (as + * compared to being connected to a system-integrated device or + * being disabled). + * This bit is valid for Downstream Ports. This bit is undefined + * for Upstream Ports. + */ + u16 slot_implemented = pcie_flags & PCI_EXP_FLAGS_SLOT; + return downstream_port && slot_implemented ? + HOTPLUG_PCIE : HOTPLUG_NO_SUPPORTED; + break; + } + } } shpc_cap = pci_find_capability(bus->bus_dev->bdf, PCI_CAP_ID_SHPC, 0); - return !!shpc_cap; + return !!shpc_cap ? HOTPLUG_SHPC : HOTPLUG_NO_SUPPORTED; } /* Test whether bridge support forwarding of transactions @@ -904,7 +919,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) u8 pcie_cap = pci_find_capability(bdf, PCI_CAP_ID_EXP, 0); u8 qemu_cap = pci_find_resource_reserve_capability(bdf); - int hotplug_support = pci_bus_hotplug_support(s, pcie_cap); + hotplug_type_t hotplug_support = pci_bus_hotplug_support(s, pcie_cap); for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; @@ -948,7 +963,9 @@ static int pci_bios_check_devices(struct pci_bus *busses) if (pci_region_align(&s->r[type]) > align) align = pci_region_align(&s->r[type]); u64 sum = pci_region_sum(&s->r[type]); - int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO); + int resource_optional = 0; + if (hotplug_support == HOTPLUG_PCIE) + resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO); if (!sum && hotplug_support && !resource_optional) sum = align; /* reserve min size for hot-plug */ if (size > sum) { -- 2.27.0

2 3