| Message ID | 20200123134805.1993-1-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add reassignable BARs and PCIE 1.1 support | expand |
On Thu, 23 Jan 2020 13:47:35 +0000 Alexandru Elisei <alexandru.elisei@arm.com> wrote: Hi Will, I am done with reviewing this series. The first patches up to and including 07/30 are good to go, since they are fixes, I would be delighted to see them merged ASAP. You might want to add my R-b: tags to those. For the rest of the series I had some comments, but apart from two locking issues I don't see any showstoppers. If I understand correctly, Alex would send a new version soonish. Merging the first fixes now would help to make v3 less daunting regarding the number of patches ;-) Thanks, Andre > kvmtool uses the Linux-only dt property 'linux,pci-probe-only' to prevent > it from trying to reassign the BARs. Let's make the BARs reassignable so > we can get rid of this band-aid. > > Let's also extend the legacy PCI emulation, which came out in 1992, so we > can properly emulate the PCI Express version 1.1 protocol, which is > relatively new, being published in 2005. > > For this iteration, I have completely reworked the way BARs are > reassigned. As I was adding support for reassignable BARs to more devices, > it became clear to me that I was duplicating the same code over and over > again. Furthermore, during device configuration, Linux can assign a region > resource to a BAR that temporarily overlaps with another device. With my > original approach, that meant that every device must be aware of the BAR > values for all the other devices. > > With this new approach, the algorithm for activating/deactivating emulation > as BAR addresses change lives completely inside the PCI code. Each device > registers two callback functions which are called when device emulation is > activated (for example, to activate emulation for a newly assigned BAR > address), respectively, when device emulation is deactivated (a previous > BAR address is changed, and emulation for that region must be deactivated). > > I also tried to do better at testing the patches. I have tested VFIO with > virtio-pci on an arm64 and a x86 machine: > > 1. AMD Seattle: Intel 82574L Gigabit Ethernet card, Samsung 970 Pro NVME > (controller registers are in the same BAR region as the MSIX table and PBA, > I wrote a nasty hack to make it work, will try to upstream something after > this series), Realtek 8168 Gigabit Ethernet card, NVIDIA Quadro P400 (only > device detection), AMD Firepro W2100 (amdgpu driver fails probing > because of missing expansion ROM emulation in kvmtool, I will send patches > for this too), Myricom 10 Gigabit Ethernet card, Seagate Barracuda 1000GB > drive. > > 2. Ryzen 3900x + Gigabyte x570 Aorus Master (bios F10): Realtek 8168 > Gigabit Ethernet card, AMD Firepro W2100 (same issue as on Seattle). > > Using the CFI flash emulation for kvmtool [1] and a hacked version of EDK2 > as the firmware for the virtual machine, I was able download an official > debian arm64 installation iso, install debian and then run it. EDK2 patches > for kvmtool will be posted soon. > > You will notice from the changelog that there are a lot of new patches > (17!), but most of them are fixes for stuff that I found while testing. > > Patches 1-18 are fixes and cleanups, and can be merged independently. They > are pretty straightforward, so if the size of the series looks off-putting, > please review these first. I am aware that the series has grown quite a > lot, I am willing to split the fixes from the rest of the patches, or > whatever else can make reviewing easier. > > Changes in v2: > * Patches 2, 11-18, 20, 22-27, 29 are new. > * Patches 11, 13, and 14 have been dropped. > * Reworked the way BAR reassignment is implemented. > * The patch "Add PCI Express 1.1 support" has been reworked to apply only > to arm64. For x86 we would need ACPI support in order to advertise the > location of the ECAM space. > * Gathered Reviewed-by tags. > * Implemented review comments. > > [1] https://www.spinics.net/lists/arm-kernel/msg778623.html > > Alexandru Elisei (24): > Makefile: Use correct objcopy binary when cross-compiling for x86_64 > hw/i8042: Compile only for x86 > Remove pci-shmem device > Check that a PCI device's memory size is power of two > arm/pci: Advertise only PCI bus 0 in the DT > vfio/pci: Allocate correct size for MSIX table and PBA BARs > vfio/pci: Don't assume that only even numbered BARs are 64bit > vfio/pci: Ignore expansion ROM BAR writes > vfio/pci: Don't access potentially unallocated regions > virtio: Don't ignore initialization failures > Don't ignore errors registering a device, ioport or mmio emulation > hw/vesa: Don't ignore fatal errors > hw/vesa: Set the size for BAR 0 > Use independent read/write locks for ioport and mmio > pci: Add helpers for BAR values and memory/IO space access > virtio/pci: Get emulated region address from BARs > vfio: Destroy memslot when unmapping the associated VAs > vfio: Reserve ioports when configuring the BAR > vfio/pci: Don't write configuration value twice > pci: Implement callbacks for toggling BAR emulation > pci: Toggle BAR I/O and memory space emulation > pci: Implement reassignable BARs > vfio: Trap MMIO access to BAR addresses which aren't page aligned > arm/arm64: Add PCI Express 1.1 support > > Julien Thierry (5): > ioport: pci: Move port allocations to PCI devices > pci: Fix ioport allocation size > arm/pci: Fix PCI IO region > virtio/pci: Make memory and IO BARs independent > arm/fdt: Remove 'linux,pci-probe-only' property > > Sami Mujawar (1): > pci: Fix BAR resource sizing arbitration > > Makefile | 6 +- > arm/fdt.c | 1 - > arm/include/arm-common/kvm-arch.h | 4 +- > arm/include/arm-common/pci.h | 1 + > arm/ioport.c | 3 +- > arm/kvm.c | 3 + > arm/pci.c | 25 +- > builtin-run.c | 6 +- > hw/i8042.c | 14 +- > hw/pci-shmem.c | 400 ------------------------------ > hw/vesa.c | 132 +++++++--- > include/kvm/devices.h | 3 +- > include/kvm/ioport.h | 10 +- > include/kvm/kvm-config.h | 2 +- > include/kvm/kvm.h | 9 +- > include/kvm/pci-shmem.h | 32 --- > include/kvm/pci.h | 168 ++++++++++++- > include/kvm/util.h | 2 + > include/kvm/vesa.h | 6 +- > include/kvm/virtio-pci.h | 3 - > include/kvm/virtio.h | 7 +- > include/linux/compiler.h | 2 +- > ioport.c | 57 ++--- > kvm.c | 65 ++++- > mips/kvm.c | 3 +- > mmio.c | 26 +- > pci.c | 320 ++++++++++++++++++++++-- > powerpc/include/kvm/kvm-arch.h | 2 +- > powerpc/ioport.c | 3 +- > powerpc/spapr_pci.c | 2 +- > vfio/core.c | 22 +- > vfio/pci.c | 231 +++++++++++++---- > virtio/9p.c | 9 +- > virtio/balloon.c | 10 +- > virtio/blk.c | 14 +- > virtio/console.c | 11 +- > virtio/core.c | 9 +- > virtio/mmio.c | 13 +- > virtio/net.c | 32 +-- > virtio/pci.c | 220 +++++++++++----- > virtio/scsi.c | 14 +- > x86/include/kvm/kvm-arch.h | 2 +- > x86/ioport.c | 66 +++-- > 43 files changed, 1217 insertions(+), 753 deletions(-) > delete mode 100644 hw/pci-shmem.c > delete mode 100644 include/kvm/pci-shmem.h >
Hi all, On 2020-01-23 13:47, Alexandru Elisei wrote: > kvmtool uses the Linux-only dt property 'linux,pci-probe-only' to > prevent > it from trying to reassign the BARs. Let's make the BARs reassignable > so > we can get rid of this band-aid. Is there anything holding up this series? I'd really like to see it merged in mainline kvmtool, as the EDK2 port seem to have surfaced (and there are environments where running QEMU is just overkill). It'd be good if it could be rebased and reposted. Thanks, M.
Hi, On 5/13/20 3:56 PM, Marc Zyngier wrote: > Hi all, > > On 2020-01-23 13:47, Alexandru Elisei wrote: >> kvmtool uses the Linux-only dt property 'linux,pci-probe-only' to prevent >> it from trying to reassign the BARs. Let's make the BARs reassignable so >> we can get rid of this band-aid. > > Is there anything holding up this series? I'd really like to see it > merged in mainline kvmtool, as the EDK2 port seem to have surfaced > (and there are environments where running QEMU is just overkill). > > It'd be good if it could be rebased and reposted. Thank you for the interest, v3 is already out there, by the way, and the first 18 patches are already merged. I finished working on v4 and I was just getting ready to run the finally battery of tests. If I don't discover any bugs (fingers crossed!), I'll send v4 tomorrow. Thanks, Alex
Hi, On 5/13/20 4:15 PM, Alexandru Elisei wrote: > Hi, > > On 5/13/20 3:56 PM, Marc Zyngier wrote: >> Hi all, >> >> On 2020-01-23 13:47, Alexandru Elisei wrote: >>> kvmtool uses the Linux-only dt property 'linux,pci-probe-only' to prevent >>> it from trying to reassign the BARs. Let's make the BARs reassignable so >>> we can get rid of this band-aid. >> Is there anything holding up this series? I'd really like to see it >> merged in mainline kvmtool, as the EDK2 port seem to have surfaced >> (and there are environments where running QEMU is just overkill). >> >> It'd be good if it could be rebased and reposted. > Thank you for the interest, v3 is already out there, by the way, and the first 18 > patches are already merged. v3 can be found at link [1]. The cover letter mentions that I had to drop Julien Thierry's patch that fixed the UART overlapping with the PCI I/O region because it broke guests that used 64k pages. Which means that EDK2 + PCI still doesn't work with kvmtool, even if the series gets merged. On the plus side, CFI flash emulation is merged and EDK2 works right now with kvmtool, as long as you stick to virtio-mmio (which unfortunately means no passthrough as well). I tested this with the EDK2 firmware posted by Ard [2]. [1] https://www.spinics.net/lists/kvm/msg211272.html [2] https://www.spinics.net/lists/kvm/msg213842.html > > I finished working on v4 and I was just getting ready to run the finally battery > of tests. If I don't discover any bugs (fingers crossed!), I'll send v4 tomorrow. Just as I feared, the last patch in the series, the one that adds PCIe support, breaks EDK2. EDK2 doesn't know about legacy PCI so the aforementioned overlap is not an issue. But as soon as I advertise support for PCIe EDK2 breaks because of it. I think I'll just drop the PCIe support patch from the series (so I don't regress EDK2 + virtio-mmio support) and re-send it after this entire issue gets sorted. Thanks, Alex