[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocat

From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH 0/4] pci: better support for 64-bit MMIO allocation
Date: Mon, 9 May 2016 21:16:46 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0

On 05/09/2016 07:50 PM, Laszlo Ersek wrote:
Hi Marcel,

On 05/02/16 17:37, Marcel Apfelbaum wrote:

First two patches allocate (max_reserved_ram - max_addr_cpu_addressable) range 
for PCI hotplug
(for PC Machines) instead of the previous 64-bit PCI window that included only
the ranges allocated by the firmware.

The next two patches fix 64-bit CRS computations.

Thank you,

Marcel Apfelbaum (4):
   hw/pc: extract reserved memory end computation to a standalone
   pci: reserve 64 bit MMIO range for PCI hotplug
   acpi: refactor pxb crs computation
   hw/apci: handle 64-bit MMIO regions correctly

  hw/i386/acpi-build.c | 135 ++++++++++++++++++++++++++++++++++++---------------
  hw/i386/pc.c         |  29 ++++++++---
  hw/pci/pci.c         |  16 +++++-
  include/hw/i386/pc.h |   1 +
  4 files changed, 131 insertions(+), 50 deletions(-)

I just tried to test this series, on top of v2.6.0-rc5. (My intent was to dump 
and decompile the DSDT in the guest, and to compare the versions before vs. 

The series doesn't apply.

You had posted it on May 2nd, so I tried to apply the series to v2.6.0-rc4 
(2016-05-02) and v2.6.0-rc3 too (2016-04-21); neither worked.

The error message from "git am --reject" is, for patch #4:

Checking patch hw/i386/acpi-build.c...
error: while searching for:

     if (pci->w64.begin) {
             aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
                              AML_CACHEABLE, AML_READ_WRITE,
                              0, pci->w64.begin, pci->w64.end - 1, 0,
                              pci->w64.end - pci->w64.begin));
     aml_append(scope, aml_name_decl("_CRS", crs));

Looking at the trailing context in your patch #4, in particular

     aml_append(scope, aml_name_decl("_CRS", crs));

I think that the conflict is caused by this intervening commit:

commit 2b1c2e8e5f1990f0a201a8cbf9d366fca60f4aa8
Author: Igor Mammedov <address@hidden>
Date:   Fri Apr 8 13:23:13 2016 +0200

     pc: acpi: tpm: add missing MMIO resource to PCI0._CRS

Which was part of v2.6.0-rc2.

... Actually, that's a pretty small patch, so maybe I can apply your series at 
2b1c2e8e5f19^, then rebase it to v2.6.0-rc5 from there... Yes, that works.

... as far as rebasing is concerned, but when it comes to compiling, the 
patches seem to break the arm-softmmu and aarch64-softmmu targets:

   hw/pci/pci.c:2474: undefined reference to 

This is because patch #2 adds a pc_machine_get_reserved_memory_end() call to 
the generic pci_bus_get_w64_range() function, which is included for other 
targets as well.

Hi Laszlo,

Sorry for all that, I'll rebase and re-send. Next time just ping me and I'll do 
the homework.

Anyway, for now I'm testing with the x86_64-softmmu target. So this is my QEMU 
command line:


   if [ $VIRTIO10 -eq 0 ]; then

   cp $TMPL vars3.fd

   /opt/qemu-installed/bin/qemu-system-x86_64 \
     -m 2048 \
     -M pc \
     -enable-kvm \
     -device qxl-vga \
     -drive if=pflash,readonly,format=raw,file=$CODE \
     -drive if=pflash,format=raw,file=vars3.fd \
     -drive id=cdrom,if=none,readonly,format=raw,cache=writethrough,file=$ISO \
     -debugcon file:debug3.log \
     -global isa-debugcon.iobase=0x402 \
     -chardev stdio,signal=off,mux=on,id=char0 \
     -mon chardev=char0,mode=readline,default \
     -serial chardev:char0 \
     -device pxb,bus=pci.0,id=bridge1,bus_nr=11 \
     -device pxb,bus=pci.0,id=bridge2,bus_nr=7 \
     -device pxb,bus=pci.0,id=bridge3,bus_nr=15 \
     -device virtio-scsi-pci,id=scsi0,bus=bridge1 \
     -device scsi-cd,bus=scsi0.0,drive=cdrom,bootindex=1 \
user,id=netdev0,hostfwd=tcp:,tftp=$TFTP,bootfile=$BF \
virtio-net-pci,netdev=netdev0,romfile=,bootindex=2,bus=bridge1,addr=2 \
     -netdev user,id=netdev1,hostfwd=tcp: \
virtio-net-pci,netdev=netdev1,romfile=,$MODERN,bootindex=3,bus=bridge2,addr=3 \
     -netdev user,id=netdev2,hostfwd=tcp: \
virtio-net-pci,netdev=netdev2,romfile=,$MODERN,bootindex=4,bus=bridge3,addr=4 \
     -global PIIX4_PM.disable_s3=0

Without your patches, the guest dmesg contains the messages

   pci 0000:0f:00.0: can't claim BAR 15 [mem 0x800800000-0x800ffffff 64bit 
pref]: no compatible bridge window
   pci 0000:07:00.0: can't claim BAR 15 [mem 0x800000000-0x8007fffff 64bit 
pref]: no compatible bridge window
   pci 0000:10:04.0: can't claim BAR 4 [mem 0x800800000-0x800ffffff 64bit 
pref]: no compatible bridge window
   pci 0000:08:03.0: can't claim BAR 4 [mem 0x800000000-0x8007fffff 64bit 
pref]: no compatible bridge window

With your patches, those messages are gone, so that looks fine.

Comparing the decompiled DSDTs, before and after, in the _CRS objects of the PC0F and PC07 devices 
(see above "id=bridge2,bus_nr=7" and "id=bridge3,bus_nr=15"), the truncation is 
fixed; the DWordMemory entries are replaced with the right QWordMemory entries. Great.


I also see some changes in the _CRS of the PCI0 device:

              DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, 
NonCacheable, ReadWrite,
                  0x00000000,         // Granularity
                  0x88400000,         // Range Minimum
-                0xFFFFFFFF,         // Range Maximum
+                0xFEBFFFFF,         // Range Maximum
                  0x00000000,         // Translation Offset
-                0x77C00000,         // Length
+                0x76800000,         // Length

This change looks correct (the IO-APIC area starts at 0xFEC00000), but I can't 
connect it to your patches. Can you explain what code does this?

The only interesting patch with regard to the above range is Patch 4/4. 
However, it deals only with 64-bit MMIO ranges.
Looking closer, it is possible we had a bug before this patch: The code assumed 
all the ranges assigned to PXBs are 32-bit. Adding a 64-bit
may cause a mismatch beyond the wrong ACPI table.

The way it works is (for each resource type IO/MEM/ and now 64-bit MEM):
1. build_crs is called for each PXB, the firmware assigned ranges are added 
placed temp list CrsRangeSet
2. we call crs_range_merge to try to merge adjacent ranges to shrink the CRS 
(we don't want a resource for each devices)
3. finally we add the ranges to the ACPI tables
4. once all the PXBs are 'done', we call  crs_replace_with_free_ranges to replace the 
ranges "used" by PXBs with free ones
   to be assigned to bus 0.
5. We add the bus 0 resources to ACPI tables.

It is possible that steps 2 and/or 4 may be affected by (not anticipated) 
64-bit ranges, but this patch completely separates them.

+                ,, , AddressRangeMemory, TypeStatic)
+            QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, 
Cacheable, ReadWrite,
+                0x0000000000000000, // Granularity
+                0x0000000100000000, // Range Minimum
+                0x00000007FFFFFFFF, // Range Maximum
+                0x0000000000000000, // Translation Offset
+                0x0000000700000000, // Length
+                ,, , AddressRangeMemory, TypeStatic)
+            QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, 
Cacheable, ReadWrite,
+                0x0000000000000000, // Granularity
+                0x0000000801000000, // Range Minimum
+                0x000000FFFFFFFFFF, // Range Maximum
+                0x0000000000000000, // Translation Offset
+                0x000000F7FF000000, // Length
                  ,, , AddressRangeMemory, TypeStatic)

This looks like the range from 4GB to 1TB, from which the allocated MMIO64 
resources have been punched out. This is the room for the hotplugged devices, 

Exactly. For the moment all this range is allocated to bus 0, but once PXBs 
will support hotplug we'll need to divide it somehow between them.
I tried to hotplug an ivshmem device with a huge BAR (sadly I don't have a real 
device) and after this series it can be hot-plugged like a charm.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]