On Thu, 23 Jan 2020 13:47:35 +0000 Alexandru Elisei <alexandru.elisei@xxxxxxx> 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 >
