From patchwork Wed May 28 01:31:36 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 353201 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 424211400DA for ; Wed, 28 May 2014 11:32:09 +1000 (EST) Received: from localhost ([::1]:39103 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WpSis-0006jE-Mu for incoming@patchwork.ozlabs.org; Tue, 27 May 2014 21:32:06 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54854) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WpSiY-0006Ri-8j for qemu-devel@nongnu.org; Tue, 27 May 2014 21:31:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WpSiT-0002Sk-Vk for qemu-devel@nongnu.org; Tue, 27 May 2014 21:31:46 -0400 Received: from mga01.intel.com ([192.55.52.88]:48440) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WpSiT-0002Se-Na for qemu-devel@nongnu.org; Tue, 27 May 2014 21:31:41 -0400 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 27 May 2014 18:31:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.98,924,1392192000"; d="scan'208";a="545888655" Received: from fmsmsx107.amr.corp.intel.com ([10.19.9.54]) by fmsmga002.fm.intel.com with ESMTP; 27 May 2014 18:31:39 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.19.17.221) by FMSMSX107.amr.corp.intel.com (10.19.9.54) with Microsoft SMTP Server (TLS) id 14.3.123.3; Tue, 27 May 2014 18:31:39 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx152.amr.corp.intel.com (10.19.17.221) with Microsoft SMTP Server (TLS) id 14.3.123.3; Tue, 27 May 2014 18:31:39 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.7]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.7]) with mapi id 14.03.0123.003; Wed, 28 May 2014 09:31:38 +0800 From: "Chen, Tiejun" To: Stefano Stabellini Thread-Topic: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping Thread-Index: AQHPedIZM7JjYMukY0CkXlcSvMUxHptVMpfA Date: Wed, 28 May 2014 01:31:36 +0000 Message-ID: References: <1401097389-4255-1-git-send-email-tiejun.chen@intel.com> <1401097389-4255-6-git-send-email-tiejun.chen@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 192.55.52.88 Cc: "peter.maydell@linaro.org" , "xen-devel@lists.xensource.com" , "mst@redhat.com" , "Kay, Allen M" , "qemu-devel@nongnu.org" , "Kelly.Zytaruk@amd.com" , "Zhang, Yang Z" , "anthony@codemonkey.ws" , "anthony.perard@citrix.com" Subject: Re: [Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Wednesday, May 28, 2014 1:36 AM > To: Chen, Tiejun > Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com; > mst@redhat.com; Kelly.Zytaruk@amd.com; qemu-devel@nongnu.org; > xen-devel@lists.xensource.com; peter.maydell@linaro.org; > anthony@codemonkey.ws; Kay, Allen M; Zhang, Yang Z > Subject: Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping > > On Mon, 26 May 2014, Tiejun Chen wrote: > > The OpRegion shouldn't be mapped 1:1 because the address in the host > > can't be used in the guest directly. > > > > This patch traps read and write access to the opregion of the Intel > > GPU config space (offset 0xfc). > > > > The original patch is from Jean Guyader > > > > Signed-off-by: Yang Zhang > > Signed-off-by: Tiejun Chen > > Cc: Jean Guyader > > --- > > v3: > > > > * Fix some typos. > > * Add more comments to make that readable. > > * To unmap igd_opregion when call xen_pt_unregister_vga_regions(). > > * Improve some return paths. > > * We need to map 3 pages for opregion as hvmloader set. > > * Force to convert igd_guest/host_opoegion as an unsigned long type > > while calling xc_domain_memory_mapping(). > > > > v2: > > > > * We should return zero as an invalid address value while calling > > igd_read_opregion(). > > [snip] > > + > > +uint32_t igd_read_opregion(XenPCIPassthroughState *s) { > > + uint32_t val = 0; > > + > > + if (igd_guest_opregion == 0) { > > + return val; > > Sorry for not having spotted it before, but isn't this supposed to return an error? > The old code returns -1 in this case. I already commented this in v2 log above. We shouldn't return "-1" to indicate an invalid address since we often think "!0" is a valid address value. In Linux instance, drivers/gpu/drm/i915/intel_opregion.c: int intel_opregion_setup(struct drm_device *dev) { ... pci_read_config_dword(dev->pdev, PCI_ASLS, &asls); DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls); if (asls == 0) { DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n"); return -ENOTSUPP; } ... > > > > + } > > + > > + val = igd_guest_opregion; > > + > > + XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val); > > + return val; > > +} > > + > > +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val) { > > + int ret; > > + > > + if (igd_guest_opregion) { > > + XEN_PT_LOG(&s->dev, "opregion register already been set, > ignoring %x\n", > > + val); > > + return; > > + } > > + > > + xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION, > > + (uint8_t *)&igd_host_opregion, 4); > > + igd_guest_opregion = (unsigned long)(val & ~0xfff) > > + | (igd_host_opregion & 0xfff); > > + > > + ret = xc_domain_memory_mapping(xen_xc, xen_domid, > > + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT), > > + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), > > + 3, > > Why 3 pages instead of 2? commit 408a9e56343b006c9e58a334f0b97dd2deedf9ac Author: Keir Fraser Date: Thu Jan 10 17:26:24 2013 +0000 hvmloader: Allocate 3 pages for Intel GPU OpRegion passthrough. The 8kB region may not be page aligned, hence requiring 3 pages to be mapped through. Signed-off-by: Keir Fraser Tiejun diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h index 3a4e145..7f8a90f 100644 --- a/tools/firmware/hvmloader/config.h +++ b/tools/firmware/hvmloader/config.h @@ -5,7 +5,9 @@ enum virtual_vga { VGA_none, VGA_std, VGA_cirrus, VGA_pt }; extern enum virtual_vga virtual_vga; + extern unsigned long igd_opregion_pgbase; +#define IGD_OPREGION_PAGES 3 ... Thanks