From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5JOK-0000tn-Ma for qemu-devel@nongnu.org; Wed, 17 Jun 2015 15:52:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5JOH-0002XY-EB for qemu-devel@nongnu.org; Wed, 17 Jun 2015 15:52:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54593) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5JOH-0002XH-6m for qemu-devel@nongnu.org; Wed, 17 Jun 2015 15:52:53 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id A4F87C4652 for ; Wed, 17 Jun 2015 19:52:51 +0000 (UTC) Message-ID: <5581D011.3060205@redhat.com> Date: Wed, 17 Jun 2015 22:52:49 +0300 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1434545105-5811-1-git-send-email-lersek@redhat.com> <1434545105-5811-5-git-send-email-lersek@redhat.com> <20150617154222-mutt-send-email-mst@redhat.com> In-Reply-To: <20150617154222-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 4/7] hw/pci-bridge: disable hotplug in PXB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Laszlo Ersek Cc: qemu-devel@nongnu.org On 06/17/2015 04:45 PM, Michael S. Tsirkin wrote: > On Wed, Jun 17, 2015 at 02:45:02PM +0200, Laszlo Ersek wrote: >> OVMF downloads the ACPI linker/loader script from QEMU when the edk2 PCI >> Bus driver globally signals the firmware that PCI enumeration and resource >> allocation have completed. At this point QEMU regenerates the ACPI payload >> in an fw_cfg read callback, and this is when the PXB's _CRS gets >> populated. >> >> Unfortunately, when this happens, the PCI_COMMAND_MEMORY bit is clear in >> the root bus's command register, *unlike* under SeaBIOS. The consequences >> unfold as follows: >> >> - When build_crs() fetches dev->io_regions[i].addr, it is all-bits-one, >> because pci_update_mappings() --> pci_bar_address() calculated it as >> PCI_BAR_UNMAPPED, due to the PCI_COMMAND_MEMORY bit being clear. >> >> - Consequently, the SHPC MMIO BAR (bar 0) of the bridge is not added to >> the _CRS, *despite* having been programmed in PCI config space. >> >> - Similarly, the SHPC MMIO BAR of the PXB is not removed from the main >> root bus's DWordMemory descriptor. >> >> - Guest OSes (Linux and Windows alike) notice the pre-programmed SHPC BAR >> within the PXB's config space, and notice that it conflicts with the >> main root bus's memory resource descriptors. Linux reports >> >> pci 0000:04:00.0: BAR 0: can't assign mem (size 0x100) >> pci 0000:04:00.0: BAR 0: trying firmware assignment [mem >> 0x88200000-0x882000ff 64bit] >> pci 0000:04:00.0: BAR 0: [mem 0x88200000-0x882000ff 64bit] conflicts >> with PCI Bus 0000:00 [mem >> 0x88200000-0xfebfffff] >> >> While Windows Server 2012 R2 reports >> >> https://technet.microsoft.com/en-us/library/cc732199%28v=ws.10%29.aspx >> >> This device cannot find enough free resources that it can use. If you >> want to use this device, you will need to disable one of the other >> devices on this system. (Code 12) >> >> This issue was apparently encountered earlier, see the "hack" in: >> >> https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02983.html >> >> and the current hole-punching logic in build_crs() and build_ssdt() is >> probably supposed to remedy exactly that problem -- however, for OVMF they >> don't work, because at the end of the PCI enumeration and resource >> allocation, which cues the ACPI linker/loader client, the command register >> is clear. >> >> The "hotplug" property of "pci-bridge", introduced in the previous >> patches, enables us to disable the standard hotplug controller cleanly, >> eliminating the SHPC bar and the conflict. >> >> Cc: Michael S. Tsirkin >> Cc: Marcel Apfelbaum >> Signed-off-by: Laszlo Ersek >> --- >> >> Notes: >> v6: >> - new in v6 (replaces "hw/pci-bridge: create interrupt-less, >> hotplug-less bridge for PXB" from v5) [Michael] >> >> hw/pci-bridge/pci_expander_bridge.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c >> index ec2bb45..4398d98 100644 >> --- a/hw/pci-bridge/pci_expander_bridge.c >> +++ b/hw/pci-bridge/pci_expander_bridge.c >> @@ -176,6 +176,7 @@ static int pxb_dev_initfn(PCIDevice *dev) >> bds = qdev_create(BUS(bus), "pci-bridge"); >> bds->id = dev_name; >> qdev_prop_set_uint8(bds, "chassis_nr", pxb->bus_nr); > > BTW this is a bug: it will conflict with chassis_nr > on another bridge if user set it. Not related to this > patch of course. I will take care of this. Thanks, Marcel > >> + qdev_prop_set_bit(bds, "hotplug", false); >> >> PCI_HOST_BRIDGE(ds)->bus = bus; >> >> -- >> 1.8.3.1 >>