On Wed, 18 May 2016 17:22:43 +0300 Marcel Apfelbaum <mar...@redhat.com> wrote:
> On 05/18/2016 04:53 PM, 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 > > > > 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. > > > I'll add the test, thanks, and also I'll look for the PXB warning. > > > > > 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() > > > > Their PRT are not exactly the same, please see build_prt in > hw/i386/acpi-build.c . build_prt() can generate shared AML if is_pci0_prt would be checked in AML, something like this: @@@ -667,13 -684,13 +667,13 @@@ static Aml *build_prt(bool is_pci0_prt /* route[2] = "LNK[D|A|B|C]", selection based on pin % 3 */ aml_append(while_ctx, initialize_route(route, "LNKD", lnk_idx, 0)); -- if (is_pci0_prt) { ++ { Aml *if_device_1, *if_pin_4, *else_pin_4; /* device 1 is the power-management device, needs SCI */ if_device_1 = aml_if(aml_equal(lnk_idx, aml_int(1))); { -- if_pin_4 = aml_if(aml_equal(pin, aml_int(4))); ++ if_pin_4 = aml_if(aml_and(aml_equal(pin, aml_int(4)), is_pci0_prt, NULL)); { aml_append(if_pin_4, aml_store(build_prt_entry("LNKS"), route)); @@@ -687,8 -704,8 +687,6 @@@ aml_append(if_device_1, else_pin_4); } aml_append(while_ctx, if_device_1); -- } else { -- aml_append(while_ctx, initialize_route(route, "LNKA", lnk_idx, 1)); } aml_append(while_ctx, initialize_route(route, "LNKB", lnk_idx, 2)); aml_append(while_ctx, initialize_route(route, "LNKC", lnk_idx, 3)); as is_pci0_prt key, it's probably possible to use _UID == 0 condition or make a wrapper method per bus that explicitly will tell common_prt() if it's root bus or not. > > Thanks, > Marcel > > >> 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(-) > >> > > > >