From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56112) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUQGl-0004n7-Nz for qemu-devel@nongnu.org; Fri, 12 Feb 2016 21:49:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aUQGg-0006cg-Nl for qemu-devel@nongnu.org; Fri, 12 Feb 2016 21:49:11 -0500 Received: from mail-qk0-x243.google.com ([2607:f8b0:400d:c09::243]:36300) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUQGg-0006cZ-EI for qemu-devel@nongnu.org; Fri, 12 Feb 2016 21:49:06 -0500 Received: by mail-qk0-x243.google.com with SMTP id e124so3397975qkc.3 for ; Fri, 12 Feb 2016 18:49:06 -0800 (PST) Date: Fri, 12 Feb 2016 21:49:04 -0500 From: Kevin O'Connor Message-ID: <20160213024904.GA31040@morn.lan> References: <20160213001835.18456.46422.stgit@gimli.home> <20160213002318.18456.48603.stgit@gimli.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160213002318.18456.48603.stgit@gimli.home> Subject: Re: [Qemu-devel] [RFC PATCH v3 3/3] fw/pci: Allocate IGD stolen memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: seabios@seabios.org, allen.m.kay@intel.com, qemu-devel@nongnu.org, kvm@vger.kernel.org On Fri, Feb 12, 2016 at 05:23:18PM -0700, Alex Williamson wrote: > Intel IGD makes use of memory allocated and marked reserved by the > BIOS as a stolen memory range. For the most part, guest drivers don't > make use of this, but our achilles heel is the vBIOS. The vBIOS > programs the device to use the host stolen memory range and it's used > in the pre-boot environment. Generally the guest won't have access to > the host stolen memory area, so these accesses either land in VM > memory or unassigned space and generate IOMMU faults. By allocating > this range in SeaBIOS and programming it into the device, QEMU (via > vfio) can make sure this guest allocated stolen memory range is used > instead. What does "vBIOS" mean in this context? Is it the video device option rom or something else? > > Signed-off-by: Alex Williamson > --- > src/fw/pciinit.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c > index 92170d5..c1ad5d4 100644 > --- a/src/fw/pciinit.c > +++ b/src/fw/pciinit.c > @@ -260,7 +260,7 @@ static void ich9_smbus_setup(struct pci_device *dev, void *arg) > static void intel_igd_opregion_setup(struct pci_device *dev, void *arg) > { > struct romfile_s *file = romfile_find("etc/igd-opregion"); > - void *opregion; > + void *opregion, *bdsm; > u16 bdf = dev->bdf; > > if (!file || !file->size) > @@ -281,6 +281,17 @@ static void intel_igd_opregion_setup(struct pci_device *dev, void *arg) > > dprintf(1, "Intel IGD OpRegion enabled on %02x:%02x.%x\n", > pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf)); > + > + bdsm = memalign_high(1024 * 1024, 1024 * 1024); > + if (!bdsm) { > + warn_noalloc(); > + return; > + } The "high" memory pool is not a good fit for such a large allocation. For so much space, I'd use memalign_tmphigh() followed by e820_add(addr, size, E820_RESERVED). > + pci_config_writel(bdf, 0x5C, cpu_to_le32((u32)bdsm)); > + > + dprintf(1, "Allocated 1MB reserved memory for Intel IGD stolen memory at " > + "0x%08x\n", (u32)bdsm); > } Does this make sense to do unconditionally in the firmware whenever the device is present? The SeaBIOS release schedule is not in sync with QEMU, so changes in the firmware tend to take longer to deploy if something needs to be done differently in the future. What happens if this register is not set? Does anything go wrong if register 0xFC is set, but 0x5C is not (eg, due to allocation failure). Thanks. -Kevin