On Wed, May 18, 2016 at 03:53:08PM +0200, Igor Mammedov wrote:
> On Sun, 15 May 2016 22:23:30 +0300
> Marcel Apfelbaum <mar...@redhat.com> wrote:
>
> > Hi,
> >
> > 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.
> I'd would add test case + expected tables as the first 2 patches
> and then finish series with expected tables update with fixed 64bit range
I don't think we necessarily need this noise when running tests.
Adding tests last is OK I think, but I prefer updating expected acpi tables
myself as these can't be merged.
> as experiment I've hacked existing piix4 case:
>
> @@ -744,7 +744,9 @@ static void test_acpi_piix4_tcg(void)
> */
> memset(&data, 0, sizeof(data));
> data.machine = MACHINE_PC;
> - test_acpi_one("-machine accel=tcg", &data);
> + test_acpi_one("-machine accel=tcg"
> + " -device pxb,id=bridge1,bus=pci.0,bus_nr=4"
> + " -device ivshmem,bus=bridge1,size=4G,shm", &data);
> free_test_data(&data);
> }
>
> And it shows not related to this series, but another pxb issue
>
> + External (_SB_.PCI0.S18_.PCNT, MethodObj) // Warning: Unresolved
> method, guessing 0 arguments
> ...
> @@ -1197,8 +1322,8 @@ DefinitionBlock ("tests/acpi-test-data/pc/DSDT.aml",
> "DSDT", 1, "BOCHS ", "BXPCD
>
> Device (S18)
> {
> - Name (_SUN, 0x03) // _SUN: Slot User Number
> Name (_ADR, 0x00030000) // _ADR: Address
> + Name (_SUN, 0x03) // _SUN: Slot User Number
> Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device
> {
> PCEJ (BSEL, _SUN)
> @@ -1638,6 +1763,7 @@ DefinitionBlock ("tests/acpi-test-data/pc/DSDT.aml",
> "DSDT", 1, "BOCHS ", "BXPCD
> BNUM = Zero
> DVNT (PCIU, One)
> DVNT (PCID, 0x03)
> + ^S18.PCNT ()
> }
> }
> }
>
> so it's better to have test case in place so that changes to pxb
> parts wouldn't go unnoticed and would be observable.
>
>
> Also from above experiment I see that pxb duplicates and uses
> the same _PRT method as PCI0, probably should be changed to
> something like:
>
> Method(_PRT)
> return ^PCI0._PRT()
>
> > v1 -> v2:
> > - resolved some styling issues (Laszlo)
> > - rebase on latest master (Laszlo)
> >
> > Thank you,
> > Marcel
> >
> > Marcel Apfelbaum (4):
> > hw/pc: extract reserved memory end computation to a standalone
> > function
> > 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 | 127
> > ++++++++++++++++++++++++++++++++++++---------------
> > hw/i386/pc.c | 29 ++++++++----
> > hw/pci/pci.c | 16 ++++++-
> > include/hw/i386/pc.h | 1 +
> > 4 files changed, 127 insertions(+), 46 deletions(-)
> >
