SeaBIOS August 2017

seabios@seabios.org

12 participants
33 discussions

[PATCH v7 0/3] Red Hat PCI bridge resource reserve capability
by Aleksandr Bezzubikov 17 Aug '17

17 Aug '17

Now PCI bridges get a bus range number on a system init, basing on currently plugged devices. That's why when one wants to hotplug another bridge, it needs his child bus, which the parent is unable to provide (speaking about virtual device). The suggested workaround is to have vendor-specific capability in Red Hat PCI bridges that contains number of additional bus to reserve (as well as IO/MEM/PREF space limit hints) on BIOS PCI init. So this capability is intended only for pure QEMU->SeaBIOS usage. Considering all aforesaid, this series is directly connected with QEMU series "Generic PCIE-PCI Bridge". Although the new PCI capability is supposed to contain various limits along with bus number to reserve, now only its full layout is proposed. And only bus_reserve field is used in QEMU and BIOS. Limits usage is still a subject for implementation as now the main goal of this series to provide necessary support from the firmware side to PCIE-PCI bridge hotplug. Changes v6->v7: 1. Do not use alignment in case of IO reservation cap usage. 2. Log additional buses reservation events. Changes v5->v6: 1. Remove unnecessary indents and line breaks (addresses Marcel's comments) 2. Count IO/MEM/PREF region size as a maximum of necessary size and one provide in RESOURCE_RESERVE capability (addresses Marcel's comment). 3. Make the cap debug message more detailed (addresses Marcel's comment). 4. Change pref_32 and pref_64 cap fields comment. Changes v4->v5: 1. Rename capability-related #defines 2. Move capability IO/MEM/PREF fields values usage to the regions creation stage (addresses Marcel's comment) 3. The capability layout change: separate pref_mem into pref_mem_32 and pref_mem_64 fields (QEMU side has the same changes) (addresses Laszlo's comment) 4. Extract the capability lookup and check to the separate function (addresses Marcel's comment) - despite of Marcel's comment do not extract field check for -1 since it increases code length and doesn't look nice because of different field types 5. Fix the capability's comment (addresses Marcel's comment) 6. Fix the 3rd patch message Changes v3->v4: 1. Use all QEMU PCI capability fields - addresses Michael's comment 2. Changes of the capability layout (QEMU side has the same changes): - change reservation fields types: bus_res - uint32_t, others - uint64_t - interpret -1 value as 'ignore' Changes v2->v3: 1. Merge commit 2 (Red Hat vendor ID) into commit 4 - addresses Marcel's comment, and add Generic PCIE Root Port device ID - addresses Michael's comment. 2. Changes of the capability layout (QEMU side has the same changes): - add 'type' field to distinguish multiple RedHat-specific capabilities - addresses Michael's comment - do not mimiс PCI Config space register layout, but use mutually exclusive differently sized fields for IO and prefetchable memory limits - addresses Laszlo's comment - use defines instead of structure and offsetof - addresses Michael's comment 3. Interpret 'bus_reserve' field as a minimum necessary range to reserve - addresses Gerd's comment 4. pci_find_capability moved to pci.c - addresses Kevin's comment 5. Move capability layout header to src/fw/dev-pci.h - addresses Kevin's comment 6. Add the capability documentation - addresses Michael's comment 7. Add capability length and bus_reserve field sanity checks - addresses Michael's comment Changes v1->v2: 1. New #define for Red Hat vendor added (addresses Konrad's comment). 2. Refactored pci_find_capability function (addresses Marcel's comment). 3. Capability reworked: - data type added; - reserve space in a structure for IO, memory and prefetchable memory limits. Aleksandr Bezzubikov (3): pci: refactor pci_find_capapibilty to get bdf as the first argument instead of the whole pci_device pci: add QEMU-specific PCI capability structure pci: enable RedHat PCI bridges to reserve additional resources on PCI init src/fw/dev-pci.h | 53 ++++++++++++++++++++++++++ src/fw/pciinit.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++--- src/hw/pci.c | 25 ++++++++++++ src/hw/pci.h | 1 + src/hw/pci_ids.h | 3 ++ src/hw/pcidevice.c | 24 ------------ src/hw/pcidevice.h | 1 - src/hw/virtio-pci.c | 6 +-- 8 files changed, 188 insertions(+), 33 deletions(-) create mode 100644 src/fw/dev-pci.h -- 2.7.4

1 3

[PATCH v6 0/4] Generic PCIE-PCI Bridge
by Aleksandr Bezzubikov 16 Aug '17

16 Aug '17

This series introduces a new device - Generic PCI Express to PCI bridge, and also makes all necessary changes to enable hotplug of the bridge itself and any device into the bridge. Changes v5->v6: 1. Fix indentation in the cap creation function (addresses Marcel's comment) 2. Simplify capability pref_mem_* fields assignment (addresses Marcel's comment) 3. Documentation fixes: - fix mutually exclusive fields definition (addresses Laszlo's comment) - fix pcie-pci-bridge usage example (addresses Marcel's comment) Changes v4->v5: 1. Change PCIE-PCI Bridge license (addresses Marcel's comment) 2. The capability layout changes (adress Laszlo' comments): - separate pref_mem into pref_mem_32 and pref_mem_64 fields (SeaBIOS side has the same changes) - accordingly change the Generic Root Port's properties 3. Do not add the capability to the root port if no valid values are provided (adresses Michael's comment) 4. Rename the capability type to 'RESOURCE_RESERVE' (addresses Marcel's comment) 5. Remove shpc_present check function (addresses Marcel's comment) 6. Fix the 4th patch message (adresses Michael's comment) 7. Patch for SHPC enabling in _OSC method has been already merged Changes v3->v4: 1. PCIE-PCI Bridge device: "msi_enable"->"msi", "shpc"->"shpc_bar", remove local_err, make "msi" property OnOffAuto, shpc_present() is still here to avoid SHPC_VMSTATE refactoring (address Marcel's comments). 2. Change QEMU PCI capability layout (SeaBIOS side has the same changes): - change reservation fields types: bus_res - uint32_t, others - uint64_t - rename 'non_pref' and 'pref' fields - interpret -1 value as 'ignore' 3. Use parent_realize in Generic PCI Express Root Port properly. 4. Fix documentation: fully replace the DMI-PCI bridge references with the new PCIE-PCI bridge, "PCIE"->"PCI Express", small mistakes and typos - address Laszlo's and Marcel's comments. 5. Rename QEMU PCI cap creation fucntion - addresses Marcel's comment. Changes v2->v3: (0). 'do_not_use' capability field flag is still _not_ in here since we haven't come to consesus on it yet. 1. Merge commits 5 (bus_reserve property creation) and 6 (property usage) together - addresses Michael's comment. 2. Add 'bus_reserve' property and QEMU PCI capability only to Generic PCIE Root Port - addresses Michael's and Marcel's comments. 3. Change 'bus_reserve' property's default value to 0 - addresses Michael's comment. 4. Rename QEMU bridge-specific PCI capability creation function - addresses Michael's comment. 5. Init the whole QEMU PCI capability with zeroes - addresses Michael's and Laszlo's comments. 6. Change QEMU PCI capability layout (SeaBIOS side has the same changes) - add 'type' field to distinguish multiple RedHat-specific capabilities - addresses Michael's comment - do not mimiс PCI Config space register layout, but use mutually exclusive differently sized fields for IO and prefetchable memory limits - addresses Laszlo's comment 7. Correct error handling in PCIE-PCI bridge realize function. 8. Replace a '2' constant with PCI_CAP_FLAGS in the capability creation function - addresses Michael's comment. 9. Remove a comment on _OSC which isn't correct anymore - address Marcel's comment. 10. Add documentation for the Generic PCIE-PCI Bridge and QEMU PCI capability - addresses Michael's comment. Changes v1->v2: 1. Enable SHPC for the bridge. 2. Enable SHPC support for the Q35 machine (ACPI stuff). 3. Introduce PCI capability to help firmware on the system init. This allows the bridge to be hotpluggable. Now it's supported only for pcie-root-port. Now it's supposed to used with SeaBIOS only, look at the SeaBIOS corresponding series "Allow RedHat PCI bridges reserve more buses than necessary during init". Aleksandr Bezzubikov (4): hw/pci: introduce pcie-pci-bridge device hw/pci: introduce bridge-only vendor-specific capability to provide some hints to firmware hw/pci: add QEMU-specific PCI capability to the Generic PCI Express Root Port docs: update documentation considering PCIE-PCI bridge docs/pcie.txt | 49 +++++----- docs/pcie_pci_bridge.txt | 114 ++++++++++++++++++++++ hw/pci-bridge/Makefile.objs | 2 +- hw/pci-bridge/gen_pcie_root_port.c | 36 +++++++ hw/pci-bridge/pcie_pci_bridge.c | 192 +++++++++++++++++++++++++++++++++++++ hw/pci/pci_bridge.c | 46 +++++++++ include/hw/pci/pci.h | 1 + include/hw/pci/pci_bridge.h | 25 +++++ include/hw/pci/pcie_port.h | 1 + 9 files changed, 442 insertions(+), 24 deletions(-) create mode 100644 docs/pcie_pci_bridge.txt create mode 100644 hw/pci-bridge/pcie_pci_bridge.c -- 2.7.4

2 6

[PATCH v6 0/3] Red Hat PCI bridge resource reserve capability
by Aleksandr Bezzubikov 16 Aug '17

16 Aug '17

Now PCI bridges get a bus range number on a system init, basing on currently plugged devices. That's why when one wants to hotplug another bridge, it needs his child bus, which the parent is unable to provide (speaking about virtual device). The suggested workaround is to have vendor-specific capability in Red Hat PCI bridges that contains number of additional bus to reserve (as well as IO/MEM/PREF space limit hints) on BIOS PCI init. So this capability is intended only for pure QEMU->SeaBIOS usage. Considering all aforesaid, this series is directly connected with QEMU series "Generic PCIE-PCI Bridge". Although the new PCI capability is supposed to contain various limits along with bus number to reserve, now only its full layout is proposed. And only bus_reserve field is used in QEMU and BIOS. Limits usage is still a subject for implementation as now the main goal of this series to provide necessary support from the firmware side to PCIE-PCI bridge hotplug. Changes v5->v6: 1. Remove unnecessary indents and line breaks (addresses Marcel's comments) 2. Count IO/MEM/PREF region size as a maximum of necessary size and one provide in RESOURCE_RESERVE capability (addresses Marcel's comment). 3. Make the cap debug message more detailed (addresses Marcel's comment). 4. Change pref_32 and pref_64 cap fields comment. Changes v4->v5: 1. Rename capability-related #defines 2. Move capability IO/MEM/PREF fields values usage to the regions creation stage (addresses Marcel's comment) 3. The capability layout change: separate pref_mem into pref_mem_32 and pref_mem_64 fields (QEMU side has the same changes) (addresses Laszlo's comment) 4. Extract the capability lookup and check to the separate function (addresses Marcel's comment) - despite of Marcel's comment do not extract field check for -1 since it increases code length and doesn't look nice because of different field types 5. Fix the capability's comment (addresses Marcel's comment) 6. Fix the 3rd patch message Changes v3->v4: 1. Use all QEMU PCI capability fields - addresses Michael's comment 2. Changes of the capability layout (QEMU side has the same changes): - change reservation fields types: bus_res - uint32_t, others - uint64_t - interpret -1 value as 'ignore' Changes v2->v3: 1. Merge commit 2 (Red Hat vendor ID) into commit 4 - addresses Marcel's comment, and add Generic PCIE Root Port device ID - addresses Michael's comment. 2. Changes of the capability layout (QEMU side has the same changes): - add 'type' field to distinguish multiple RedHat-specific capabilities - addresses Michael's comment - do not mimiс PCI Config space register layout, but use mutually exclusive differently sized fields for IO and prefetchable memory limits - addresses Laszlo's comment - use defines instead of structure and offsetof - addresses Michael's comment 3. Interpret 'bus_reserve' field as a minimum necessary range to reserve - addresses Gerd's comment 4. pci_find_capability moved to pci.c - addresses Kevin's comment 5. Move capability layout header to src/fw/dev-pci.h - addresses Kevin's comment 6. Add the capability documentation - addresses Michael's comment 7. Add capability length and bus_reserve field sanity checks - addresses Michael's comment Changes v1->v2: 1. New #define for Red Hat vendor added (addresses Konrad's comment). 2. Refactored pci_find_capability function (addresses Marcel's comment). 3. Capability reworked: - data type added; - reserve space in a structure for IO, memory and prefetchable memory limits. Aleksandr Bezzubikov (3): pci: refactor pci_find_capapibilty to get bdf as the first argument instead of the whole pci_device pci: add QEMU-specific PCI capability structure pci: enable RedHat PCI bridges to reserve additional resource on PCI init src/fw/dev-pci.h | 53 +++++++++++++++++++++++++++ src/fw/pciinit.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++--- src/hw/pci.c | 25 +++++++++++++ src/hw/pci.h | 1 + src/hw/pci_ids.h | 3 ++ src/hw/pcidevice.c | 24 ------------- src/hw/pcidevice.h | 1 - src/hw/virtio-pci.c | 6 ++-- 8 files changed, 181 insertions(+), 33 deletions(-) create mode 100644 src/fw/dev-pci.h -- 2.7.4

2 6

[PATCH v5 0/3] Red Hat PCI bridge resource reserve capability (was: Allow RedHat PCI bridges reserve more buses than necessary during init)
by Aleksandr Bezzubikov 13 Aug '17

13 Aug '17

Now PCI bridges get a bus range number on a system init, basing on currently plugged devices. That's why when one wants to hotplug another bridge, it needs his child bus, which the parent is unable to provide (speaking about virtual device). The suggested workaround is to have vendor-specific capability in Red Hat PCI bridges that contains number of additional bus to reserve (as well as IO/MEM/PREF space limit hints) on BIOS PCI init. So this capability is intended only for pure QEMU->SeaBIOS usage. Considering all aforesaid, this series is directly connected with QEMU series "Generic PCIE-PCI Bridge". Although the new PCI capability is supposed to contain various limits along with bus number to reserve, now only its full layout is proposed. And only bus_reserve field is used in QEMU and BIOS. Limits usage is still a subject for implementation as now the main goal of this series to provide necessary support from the firmware side to PCIE-PCI bridge hotplug. Changes v4->v5: 1. Rename capability-related #defines 2. Move capability IO/MEM/PREF fields values usage to the regions creation stage (addresses Marcel's comment) 3. The capability layout change: separate pref_mem into pref_mem_32 and pref_mem_64 fields (QEMU side has the same changes) (addresses Laszlo's comment) 4. Extract the capability lookup and check to the separate function (addresses Marcel's comment) - despite of Marcel's comment do not extract field check for -1 since it increases code length and doesn't look nice because of different field types 5. Fix the capability's comment (addresses Marcel's comment) 6. Fix the 3rd patch message Changes v3->v4: 1. Use all QEMU PCI capability fields - addresses Michael's comment 2. Changes of the capability layout (QEMU side has the same changes): - change reservation fields types: bus_res - uint32_t, others - uint64_t - interpret -1 value as 'ignore' Changes v2->v3: 1. Merge commit 2 (Red Hat vendor ID) into commit 4 - addresses Marcel's comment, and add Generic PCIE Root Port device ID - addresses Michael's comment. 2. Changes of the capability layout (QEMU side has the same changes): - add 'type' field to distinguish multiple RedHat-specific capabilities - addresses Michael's comment - do not mimiс PCI Config space register layout, but use mutually exclusive differently sized fields for IO and prefetchable memory limits - addresses Laszlo's comment - use defines instead of structure and offsetof - addresses Michael's comment 3. Interpret 'bus_reserve' field as a minimum necessary range to reserve - addresses Gerd's comment 4. pci_find_capability moved to pci.c - addresses Kevin's comment 5. Move capability layout header to src/fw/dev-pci.h - addresses Kevin's comment 6. Add the capability documentation - addresses Michael's comment 7. Add capability length and bus_reserve field sanity checks - addresses Michael's comment Changes v1->v2: 1. New #define for Red Hat vendor added (addresses Konrad's comment). 2. Refactored pci_find_capability function (addresses Marcel's comment). 3. Capability reworked: - data type added; - reserve space in a structure for IO, memory and prefetchable memory limits. Aleksandr Bezzubikov (3): pci: refactor pci_find_capapibilty to get bdf as the first argument instead of the whole pci_device pci: add QEMU-specific PCI capability structure pci: enable RedHat PCI bridges to reserve additional resource on PCI init src/fw/dev-pci.h | 52 +++++++++++++++++++++ src/fw/pciinit.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++------ src/hw/pci.c | 25 +++++++++++ src/hw/pci.h | 1 + src/hw/pci_ids.h | 3 ++ src/hw/pcidevice.c | 24 ---------- src/hw/pcidevice.h | 1 - src/hw/virtio-pci.c | 6 +-- 8 files changed, 197 insertions(+), 42 deletions(-) create mode 100644 src/fw/dev-pci.h -- 2.7.4

2 6

[PATCH v5 0/4] Generic PCIE-PCI Bridge
by Aleksandr Bezzubikov 13 Aug '17

13 Aug '17

This series introduces a new device - Generic PCI Express to PCI bridge, and also makes all necessary changes to enable hotplug of the bridge itself and any device into the bridge. Changes v4->v5: 1. Change PCIE-PCI Bridge license (addresses Marcel's comment) 2. The capability layout changes (adress Laszlo' comments): - separate pref_mem into pref_mem_32 and pref_mem_64 fields (SeaBIOS side has the same changes) - accordingly change the Generic Root Port's properties 3. Do not add the capability to the root port if no valid values are provided (adresses Michael's comment) 4. Rename the capability type to 'RESOURCE_RESERVE' (addresses Marcel's comment) 5. Remove shpc_present check function (addresses Marcel's comment) 6. Fix the 4th patch message (adresses Michael's comment) 7. Patch for SHPC enabling in _OSC method has been already merged Changes v3->v4: 1. PCIE-PCI Bridge device: "msi_enable"->"msi", "shpc"->"shpc_bar", remove local_err, make "msi" property OnOffAuto, shpc_present() is still here to avoid SHPC_VMSTATE refactoring (address Marcel's comments). 2. Change QEMU PCI capability layout (SeaBIOS side has the same changes): - change reservation fields types: bus_res - uint32_t, others - uint64_t - rename 'non_pref' and 'pref' fields - interpret -1 value as 'ignore' 3. Use parent_realize in Generic PCI Express Root Port properly. 4. Fix documentation: fully replace the DMI-PCI bridge references with the new PCIE-PCI bridge, "PCIE"->"PCI Express", small mistakes and typos - address Laszlo's and Marcel's comments. 5. Rename QEMU PCI cap creation fucntion - addresses Marcel's comment. Changes v2->v3: (0). 'do_not_use' capability field flag is still _not_ in here since we haven't come to consesus on it yet. 1. Merge commits 5 (bus_reserve property creation) and 6 (property usage) together - addresses Michael's comment. 2. Add 'bus_reserve' property and QEMU PCI capability only to Generic PCIE Root Port - addresses Michael's and Marcel's comments. 3. Change 'bus_reserve' property's default value to 0 - addresses Michael's comment. 4. Rename QEMU bridge-specific PCI capability creation function - addresses Michael's comment. 5. Init the whole QEMU PCI capability with zeroes - addresses Michael's and Laszlo's comments. 6. Change QEMU PCI capability layout (SeaBIOS side has the same changes) - add 'type' field to distinguish multiple RedHat-specific capabilities - addresses Michael's comment - do not mimiс PCI Config space register layout, but use mutually exclusive differently sized fields for IO and prefetchable memory limits - addresses Laszlo's comment 7. Correct error handling in PCIE-PCI bridge realize function. 8. Replace a '2' constant with PCI_CAP_FLAGS in the capability creation function - addresses Michael's comment. 9. Remove a comment on _OSC which isn't correct anymore - address Marcel's comment. 10. Add documentation for the Generic PCIE-PCI Bridge and QEMU PCI capability - addresses Michael's comment. Changes v1->v2: 1. Enable SHPC for the bridge. 2. Enable SHPC support for the Q35 machine (ACPI stuff). 3. Introduce PCI capability to help firmware on the system init. This allows the bridge to be hotpluggable. Now it's supported only for pcie-root-port. Now it's supposed to used with SeaBIOS only, look at the SeaBIOS corresponding series "Allow RedHat PCI bridges reserve more buses than necessary during init". Aleksandr Bezzubikov (4): hw/pci: introduce pcie-pci-bridge device hw/pci: introduce bridge-only vendor-specific capability to provide some hints to firmware hw/pci: add QEMU-specific PCI capability to the Generic PCI Express Root Port docs: update documentation considering PCIE-PCI bridge docs/pcie.txt | 49 +++++----- docs/pcie_pci_bridge.txt | 115 ++++++++++++++++++++++ hw/pci-bridge/Makefile.objs | 2 +- hw/pci-bridge/gen_pcie_root_port.c | 36 +++++++ hw/pci-bridge/pcie_pci_bridge.c | 192 +++++++++++++++++++++++++++++++++++++ hw/pci/pci_bridge.c | 54 +++++++++++ include/hw/pci/pci.h | 1 + include/hw/pci/pci_bridge.h | 24 +++++ include/hw/pci/pcie_port.h | 1 + 9 files changed, 450 insertions(+), 24 deletions(-) create mode 100644 docs/pcie_pci_bridge.txt create mode 100644 hw/pci-bridge/pcie_pci_bridge.c -- 2.7.4

3 8

Re: [SeaBIOS] [Qemu-devel] [PATCH v4 5/5] docs: update documentation considering PCIE-PCI bridge
by Laszlo Ersek 09 Aug '17

09 Aug '17

On 08/09/17 18:52, Aleksandr Bezzubikov wrote: > 2017-08-09 13:18 GMT+03:00 Laszlo Ersek <lersek(a)redhat.com>: >> On 08/08/17 21:21, Aleksandr Bezzubikov wrote: >>> 2017-08-08 18:11 GMT+03:00 Laszlo Ersek <lersek(a)redhat.com>: >>>> one comment below >>>> >>>> On 08/05/17 22:27, Aleksandr Bezzubikov wrote: >>>> >>>>> +Capability layout (defined in include/hw/pci/pci_bridge.h): >>>>> + >>>>> + uint8_t id; Standard PCI capability header field >>>>> + uint8_t next; Standard PCI capability header field >>>>> + uint8_t len; Standard PCI vendor-specific capability header field >>>>> + >>>>> + uint8_t type; Red Hat vendor-specific capability type >>>>> + List of currently existing types: >>>>> + QEMU_RESERVE = 1 >>>>> + >>>>> + >>>>> + uint32_t bus_res; Minimum number of buses to reserve >>>>> + >>>>> + uint64_t io; IO space to reserve >>>>> + uint64_t mem Non-prefetchable memory to reserve >>>>> + uint64_t mem_pref; Prefetchable memory to reserve >>>> >>>> (I apologize if I missed any concrete points from the past messages >>>> regarding this structure.) >>>> >>>> How is the firmware supposed to know whether the prefetchable MMIO >>>> reservation should be made in 32-bit or 64-bit address space? If we >>>> reserve prefetchable MMIO outside of the 32-bit address space, then >>>> hot-plugging a device without 64-bit MMIO support could fail. >>>> >>>> My earlier request, to distinguish "prefetchable_32" from >>>> "prefetchable_64" (mutually exclusively), was so that firmware would >>>> know whether to restrict the MMIO reservation to 32-bit address >>>> space. >>> >>> IIUC now (in SeaBIOS at least) we just assign this PREF registers >>> unconditionally, >>> so the decision about the mode can be made basing on !=0 >>> UPPER_PREF_LIMIT register. >>> My idea was the same - we can just check if the value doesn't fit into >>> 16-bit (PREF_LIMIT reg size, 32-bit MMIO). Do we really need separate >>> fields for that? >> >> The PciBusDxe driver in edk2 tracks 32-bit and 64-bit MMIO resources >> separately from each other, and other (independent) logic exists in it >> that, on some conditions, allocates 64-bit MMIO BARs from 32-bit address >> space. This is just to say that the distinction is intentional in >> PciBusDxe. >> >> Furthermore, the Platform Init spec v1.6 says the following (this is >> what OVMF will have to comply with, in the "platform hook" called by >> PciBusDxe): >> >>> 12.6 PCI Hot Plug PCI Initialization Protocol >>> EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding() >>> ... >>> Padding The amount of resource padding that is required by the PCI >>> bus under the control of the specified HPC. Because the >>> caller does not know the size of this buffer, this buffer is >>> allocated by the callee and freed by the caller. >>> ... >>> The padding is returned in the form of ACPI (2.0 & 3.0) resource >>> descriptors. The exact definition of each of the fields is the same as >>> in the >>> EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL.SubmitResources() >>> function. See the section 10.8 for the definition of this function. >> >> Following that pointer: >> >>> 10.8 PCI HostBridge Code Definitions >>> 10.8.2 PCI Host Bridge Resource Allocation Protocol >>> >>> Table 8. ACPI 2.0 & 3.0 QWORD Address Space Descriptor Usage >>> >>> Byte Byte Data Description >>> Offset Length >>> ... >>> 0x03 0x01 Resource type: >>> 0: Memory range >>> 1: I/O range >>> 2: Bus number range >>> ... >>> 0x05 0x01 Type-specific flags. Ignored except as defined >>> in Table 3-3 and Table 3-4 below. >>> >>> 0x06 0x08 Address Space Granularity. Used to differentiate >>> between a 32-bit memory request and a 64-bit >>> memory request. For a 32-bit memory request, >>> this field should be set to 32. For a 64-bit >>> memory request, this field should be set to 64. >>> Ignored for I/O and bus resource requests. >>> Ignored during GetProposedResources(). >> >> The "Table 3-3" and "Table 3-4" references under "Type-specific flags" >> are out of date (spec bug); in reality those are: >> - Table 10. I/O Resource Flag (Resource Type = 1) Usage, >> - Table 11. Memory Resource Flag (Resource Type = 0) Usage. >> >> The latter is relevant here: >> >>> Table 11. Memory Resource Flag (Resource Type = 0) Usage >>> >>> Bits Meaning >>> ... >>> Bit[2:1] _MEM. Memory attributes. >>> Value and Meaning: >>> 0 The memory is nonprefetchable. >>> 1 Invalid. >>> 2 Invalid. >>> 3 The memory is prefetchable. >>> Note: The interpretation of these bits is somewhat different >>> from the ACPI Specification. According to the ACPI >>> Specification, a value of 0 implies noncacheable memory and >>> the value of 3 indicates prefetchable and cacheable memory. >> >> So whatever OVMF sees in the capability, it must be able to translate to >> the above representation. > > OK, I got it. > Then I suggest this part of the cap look like > > uint64_t mem_pref_32; > uint64_t mem_pref_64; > > 'mem_pref_32' field can be uint32_t, but this will require 4-byte padding, > so what looks more preferable here - uint64_t for 32-bit value or > 4-byte padding in the middle > of the capapbility? The last field before this part is "uint64_t io", and it is naturally aligned. So, how about: - uint32_t mem; /* non-prefetchable, 32-bit only */ - uint32_t mem_pref_32; /* prefetchable, 32-bit, * mutually exclusive with mem_pref_64 */ - uint64_t mem_pref_64; /* prefetchable, 64-bit, * mutually exclusive with mem_pref_32 */ Again, the comments to the right come from the email that I got earlier from Alex Williamson (which he wrote "according to Table 3-2 in the PCI-to-PCI bridge spec rev 1.2"). IOW, "mem" need not be uint64_t, it can be uint32_t just as well, and then we don't need padding for "mem_pref_32" either. (I also think "uint64_t io" is overkill, but I care precious little about IO reservation, beyond *disabling* it :) I intend to implement "io" as well, of course.) Thanks! Laszlo

1 0

Re: [SeaBIOS] [Qemu-devel] [PATCH v4 5/5] docs: update documentation considering PCIE-PCI bridge
by Laszlo Ersek 09 Aug '17

09 Aug '17

On 08/08/17 21:21, Aleksandr Bezzubikov wrote: > 2017-08-08 18:11 GMT+03:00 Laszlo Ersek <lersek(a)redhat.com>: >> one comment below >> >> On 08/05/17 22:27, Aleksandr Bezzubikov wrote: >> >>> +Capability layout (defined in include/hw/pci/pci_bridge.h): >>> + >>> + uint8_t id; Standard PCI capability header field >>> + uint8_t next; Standard PCI capability header field >>> + uint8_t len; Standard PCI vendor-specific capability header field >>> + >>> + uint8_t type; Red Hat vendor-specific capability type >>> + List of currently existing types: >>> + QEMU_RESERVE = 1 >>> + >>> + >>> + uint32_t bus_res; Minimum number of buses to reserve >>> + >>> + uint64_t io; IO space to reserve >>> + uint64_t mem Non-prefetchable memory to reserve >>> + uint64_t mem_pref; Prefetchable memory to reserve >> >> (I apologize if I missed any concrete points from the past messages >> regarding this structure.) >> >> How is the firmware supposed to know whether the prefetchable MMIO >> reservation should be made in 32-bit or 64-bit address space? If we >> reserve prefetchable MMIO outside of the 32-bit address space, then >> hot-plugging a device without 64-bit MMIO support could fail. >> >> My earlier request, to distinguish "prefetchable_32" from >> "prefetchable_64" (mutually exclusively), was so that firmware would >> know whether to restrict the MMIO reservation to 32-bit address >> space. > > IIUC now (in SeaBIOS at least) we just assign this PREF registers > unconditionally, > so the decision about the mode can be made basing on !=0 > UPPER_PREF_LIMIT register. > My idea was the same - we can just check if the value doesn't fit into > 16-bit (PREF_LIMIT reg size, 32-bit MMIO). Do we really need separate > fields for that? The PciBusDxe driver in edk2 tracks 32-bit and 64-bit MMIO resources separately from each other, and other (independent) logic exists in it that, on some conditions, allocates 64-bit MMIO BARs from 32-bit address space. This is just to say that the distinction is intentional in PciBusDxe. Furthermore, the Platform Init spec v1.6 says the following (this is what OVMF will have to comply with, in the "platform hook" called by PciBusDxe): > 12.6 PCI Hot Plug PCI Initialization Protocol > EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding() > ... > Padding The amount of resource padding that is required by the PCI > bus under the control of the specified HPC. Because the > caller does not know the size of this buffer, this buffer is > allocated by the callee and freed by the caller. > ... > The padding is returned in the form of ACPI (2.0 & 3.0) resource > descriptors. The exact definition of each of the fields is the same as > in the > EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL.SubmitResources() > function. See the section 10.8 for the definition of this function. Following that pointer: > 10.8 PCI HostBridge Code Definitions > 10.8.2 PCI Host Bridge Resource Allocation Protocol > > Table 8. ACPI 2.0 & 3.0 QWORD Address Space Descriptor Usage > > Byte Byte Data Description > Offset Length > ... > 0x03 0x01 Resource type: > 0: Memory range > 1: I/O range > 2: Bus number range > ... > 0x05 0x01 Type-specific flags. Ignored except as defined > in Table 3-3 and Table 3-4 below. > > 0x06 0x08 Address Space Granularity. Used to differentiate > between a 32-bit memory request and a 64-bit > memory request. For a 32-bit memory request, > this field should be set to 32. For a 64-bit > memory request, this field should be set to 64. > Ignored for I/O and bus resource requests. > Ignored during GetProposedResources(). The "Table 3-3" and "Table 3-4" references under "Type-specific flags" are out of date (spec bug); in reality those are: - Table 10. I/O Resource Flag (Resource Type = 1) Usage, - Table 11. Memory Resource Flag (Resource Type = 0) Usage. The latter is relevant here: > Table 11. Memory Resource Flag (Resource Type = 0) Usage > > Bits Meaning > ... > Bit[2:1] _MEM. Memory attributes. > Value and Meaning: > 0 The memory is nonprefetchable. > 1 Invalid. > 2 Invalid. > 3 The memory is prefetchable. > Note: The interpretation of these bits is somewhat different > from the ACPI Specification. According to the ACPI > Specification, a value of 0 implies noncacheable memory and > the value of 3 indicates prefetchable and cacheable memory. So whatever OVMF sees in the capability, it must be able to translate to the above representation. Thanks Laszlo > >> >> This is based on an earlier email from Alex to me: >> >> On 10/03/16 18:01, Alex Williamson wrote: >>> I don't think there's such a thing as a 64-bit non-prefetchable >>> aperture. In fact, there are not separate 32 and 64 bit >>> prefetchable apertures. The apertures are: >>> >>> I/O base/limit - (default 16bit, may be 32bit) >>> Memory base/limit - (32bit only, non-prefetchable) >>> Prefetchable Memory base/limit - (default 32bit, may be 64bit) >>> >>> This is according to Table 3-2 in the PCI-to-PCI bridge spec rev >>> 1.2. >> >> I don't care much about the 16-bit vs. 32-bit IO difference (that's >> entirely academic and the Platform Spec init doesn't even provide a >> way for OVMF to express such a difference). However, the optional >> restriction to 32-bit matters for the prefetchable MMIO aperture. >> >> Other than this, the patch looks good to me, and I'm ready to R-b. >> >> Thanks! >> Laszlo

2 1

Re: [SeaBIOS] [Qemu-devel] >256 Virtio-net-pci hotplug Devices
by Marcel Apfelbaum 09 Aug '17

09 Aug '17

On 07/08/2017 22:00, Kinsella, Ray wrote: > Hi Marcel, > Hi Ray, Please have a look on this thread, I think Laszlo and Paolo found the root cause. https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01368.html It seems hot-plugging the devices would not help. Thanks, MArcel > Yup - I am using Seabios by default. > I took all the measures from the Kernel time reported in syslog. > As Seabios wasn't exhibiting any obvious scaling problem. > > Ray K > > -----Original Message----- > From: Marcel Apfelbaum [mailto:marcel@redhat.com] > Sent: Wednesday, August 2, 2017 5:43 AM > To: Kinsella, Ray <ray.kinsella(a)intel.com>; Kevin O'Connor <kevin(a)koconnor.net> > Cc: Tan, Jianfeng <jianfeng.tan(a)intel.com>; seabios(a)seabios.org; Michael Tsirkin <mst(a)redhat.com>; qemu-devel(a)nongnu.org; Gerd Hoffmann <kraxel(a)redhat.com> > Subject: Re: [Qemu-devel] >256 Virtio-net-pci hotplug Devices > > It is an issue worth looking into it, one more question, all the measurements are from OS boot? Do you use SeaBIOS? > No problems with the firmware? > > Thanks, > Marcel > >

2 1

Re: [SeaBIOS] [PATCH v4 4/5] hw/pci: add QEMU-specific PCI capability to the Generic PCI Express Root Port
by Michael S. Tsirkin 08 Aug '17

08 Aug '17

On Tue, Aug 08, 2017 at 11:10:58PM +0300, Aleksandr Bezzubikov wrote: > 2017-08-08 22:54 GMT+03:00 Michael S. Tsirkin <mst(a)redhat.com>: > > On Sat, Aug 05, 2017 at 11:27:37PM +0300, Aleksandr Bezzubikov wrote: > >> To enable hotplugging of a newly created pcie-pci-bridge, > >> we need to tell firmware (SeaBIOS in this case) > > > > Why SeaBIOS is this case? > > > > It is the default BIOS for QEMU and therefore it is the best place to > try out a new feature like this reservation cap. So "e.g. SeaBIOS" would be more appropriate then. > >> to reserve > >> additional buses or IO/MEM/PREF space for pcie-root-port. > >> Additional bus reservation allows us to hotplug pcie-pci-bridge into this root port. > >> The number of buses to reserve is provided to the device via a corresponding > >> property, and to the firmware via new PCI capability. > >> The properties' default value is -1 to keep default behavior unchanged. > >> > >> Signed-off-by: Aleksandr Bezzubikov <zuban32s(a)gmail.com> > >> --- > >> hw/pci-bridge/gen_pcie_root_port.c | 33 +++++++++++++++++++++++++++++++++ > >> include/hw/pci/pcie_port.h | 1 + > >> 2 files changed, 34 insertions(+) > >> > >> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c > >> index cb694d6..ff8d04c 100644 > >> --- a/hw/pci-bridge/gen_pcie_root_port.c > >> +++ b/hw/pci-bridge/gen_pcie_root_port.c > >> @@ -16,6 +16,8 @@ > >> #include "hw/pci/pcie_port.h" > >> > >> #define TYPE_GEN_PCIE_ROOT_PORT "pcie-root-port" > >> +#define GEN_PCIE_ROOT_PORT(obj) \ > >> + OBJECT_CHECK(GenPCIERootPort, (obj), TYPE_GEN_PCIE_ROOT_PORT) > >> > >> #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100 > >> #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1 > >> @@ -26,6 +28,12 @@ typedef struct GenPCIERootPort { > >> /*< public >*/ > >> > >> bool migrate_msix; > >> + > >> + /* additional buses to reserve on firmware init */ > >> + uint32_t bus_reserve; > >> + uint64_t io_reserve; > >> + uint64_t mem_reserve; > >> + uint64_t pref_reserve; > >> } GenPCIERootPort; > >> > >> static uint8_t gen_rp_aer_vector(const PCIDevice *d) > >> @@ -60,6 +68,23 @@ static bool gen_rp_test_migrate_msix(void *opaque, int version_id) > >> return rp->migrate_msix; > >> } > >> > >> +static void gen_rp_realize(DeviceState *dev, Error **errp) > >> +{ > >> + PCIDevice *d = PCI_DEVICE(dev); > >> + GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d); > >> + PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d); > >> + > >> + rpc->parent_realize(dev, errp); > >> + > >> + int rc = pci_bridge_qemu_reserve_cap_init(d, 0, grp->bus_reserve, > >> + grp->io_reserve, grp->mem_reserve, grp->pref_reserve, errp); > > > > You have to skip adding this capability by default to avoid > > breaking migration to/from old QEMU. A simple way to do this is > > to check whether any have been specified as != -1. > > > > Agreed, I can't understand how the capability that is brand new > and unused by another code except mine can break migration. Any guest-visible behaviour change across the migration boundary is prohibited. In this case, any code that walks a capability list while you migrate will end up in the wrong place in config space. > >> + > >> + if (rc < 0) { > >> + rpc->parent_class.exit(d); > >> + return; > >> + } > >> +} > >> + > >> static const VMStateDescription vmstate_rp_dev = { > >> .name = "pcie-root-port", > >> .version_id = 1, > >> @@ -78,6 +103,10 @@ static const VMStateDescription vmstate_rp_dev = { > >> > >> static Property gen_rp_props[] = { > >> DEFINE_PROP_BOOL("x-migrate-msix", GenPCIERootPort, migrate_msix, true), > >> + DEFINE_PROP_UINT32("bus-reserve", GenPCIERootPort, bus_reserve, -1), > >> + DEFINE_PROP_UINT64("io-reserve", GenPCIERootPort, io_reserve, -1), > >> + DEFINE_PROP_UINT64("mem-reserve", GenPCIERootPort, mem_reserve, -1), > >> + DEFINE_PROP_UINT64("pref-reserve", GenPCIERootPort, pref_reserve, -1), > >> DEFINE_PROP_END_OF_LIST() > >> }; > >> > >> @@ -92,6 +121,10 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data) > >> dc->desc = "PCI Express Root Port"; > >> dc->vmsd = &vmstate_rp_dev; > >> dc->props = gen_rp_props; > >> + > >> + rpc->parent_realize = dc->realize; > >> + dc->realize = gen_rp_realize; > >> + > >> rpc->aer_vector = gen_rp_aer_vector; > >> rpc->interrupts_init = gen_rp_interrupts_init; > >> rpc->interrupts_uninit = gen_rp_interrupts_uninit; > >> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h > >> index 1333266..0736014 100644 > >> --- a/include/hw/pci/pcie_port.h > >> +++ b/include/hw/pci/pcie_port.h > >> @@ -65,6 +65,7 @@ void pcie_chassis_del_slot(PCIESlot *s); > >> > >> typedef struct PCIERootPortClass { > >> PCIDeviceClass parent_class; > >> + DeviceRealize parent_realize; > >> > >> uint8_t (*aer_vector)(const PCIDevice *dev); > >> int (*interrupts_init)(PCIDevice *dev, Error **errp); > >> -- > >> 2.7.4 > > > > -- > Aleksandr Bezzubikov

1 0

[PATCH v4 0/5] Generic PCIE-PCI Bridge
by Aleksandr Bezzubikov 08 Aug '17

08 Aug '17

This series introduces a new device - Generic PCI Express to PCI bridge, and also makes all necessary changes to enable hotplug of the bridge itself and any device into the bridge. Changes v3->v4: 1. PCIE-PCI Bridge device: "msi_enable"->"msi", "shpc"->"shpc_bar", remove local_err, make "msi" property OnOffAuto, shpc_present() is still here to avoid SHPC_VMSTATE refactoring (address Marcel's comments). 2. Change QEMU PCI capability layout (SeaBIOS side has the same changes): - change reservation fields types: bus_res - uint32_t, others - uint64_t - rename 'non_pref' and 'pref' fields - interpret -1 value as 'ignore' 3. Use parent_realize in Generic PCI Express Root Port properly. 4. Fix documentation: fully replace the DMI-PCI bridge references with the new PCIE-PCI bridge, "PCIE"->"PCI Express", small mistakes and typos - address Laszlo's and Marcel's comments. 5. Rename QEMU PCI cap creation fucntion - addresses Marcel's comment. Changes v2->v3: (0). 'do_not_use' capability field flag is still _not_ in here since we haven't come to consesus on it yet. 1. Merge commits 5 (bus_reserve property creation) and 6 (property usage) together - addresses Michael's comment. 2. Add 'bus_reserve' property and QEMU PCI capability only to Generic PCIE Root Port - addresses Michael's and Marcel's comments. 3. Change 'bus_reserve' property's default value to 0 - addresses Michael's comment. 4. Rename QEMU bridge-specific PCI capability creation function - addresses Michael's comment. 5. Init the whole QEMU PCI capability with zeroes - addresses Michael's and Laszlo's comments. 6. Change QEMU PCI capability layout (SeaBIOS side has the same changes) - add 'type' field to distinguish multiple RedHat-specific capabilities - addresses Michael's comment - do not mimiс PCI Config space register layout, but use mutually exclusive differently sized fields for IO and prefetchable memory limits - addresses Laszlo's comment 7. Correct error handling in PCIE-PCI bridge realize function. 8. Replace a '2' constant with PCI_CAP_FLAGS in the capability creation function - addresses Michael's comment. 9. Remove a comment on _OSC which isn't correct anymore - address Marcel's comment. 10. Add documentation for the Generic PCIE-PCI Bridge and QEMU PCI capability - addresses Michael's comment. Changes v1->v2: 1. Enable SHPC for the bridge. 2. Enable SHPC support for the Q35 machine (ACPI stuff). 3. Introduce PCI capability to help firmware on the system init. This allows the bridge to be hotpluggable. Now it's supported only for pcie-root-port. Now it's supposed to used with SeaBIOS only, look at the SeaBIOS corresponding series "Allow RedHat PCI bridges reserve more buses than necessary during init". Aleksandr Bezzubikov (5): hw/i386: allow SHPC for Q35 machine hw/pci: introduce pcie-pci-bridge device hw/pci: introduce bridge-only vendor-specific capability to provide some hints to firmware hw/pci: add QEMU-specific PCI capability to the Generic PCI Express Root Port docs: update documentation considering PCIE-PCI bridge docs/pcie.txt | 49 +++++---- docs/pcie_pci_bridge.txt | 110 +++++++++++++++++++ hw/i386/acpi-build.c | 4 +- hw/pci-bridge/Makefile.objs | 2 +- hw/pci-bridge/gen_pcie_root_port.c | 33 ++++++ hw/pci-bridge/pcie_pci_bridge.c | 212 +++++++++++++++++++++++++++++++++++++ hw/pci/pci_bridge.c | 29 +++++ include/hw/pci/pci.h | 1 + include/hw/pci/pci_bridge.h | 21 ++++ include/hw/pci/pcie_port.h | 1 + 10 files changed, 436 insertions(+), 26 deletions(-) create mode 100644 docs/pcie_pci_bridge.txt create mode 100644 hw/pci-bridge/pcie_pci_bridge.c -- 2.7.4

5 13

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

SeaBIOS August 2017