From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55094) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X7iCx-0003cd-Lk for qemu-devel@nongnu.org; Thu, 17 Jul 2014 05:42:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X7iCt-0005SA-1g for qemu-devel@nongnu.org; Thu, 17 Jul 2014 05:42:35 -0400 Received: from mga03.intel.com ([143.182.124.21]:3701) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X7iCs-0005Ra-Jw for qemu-devel@nongnu.org; Thu, 17 Jul 2014 05:42:30 -0400 Message-ID: <53C79A7A.4080904@intel.com> Date: Thu, 17 Jul 2014 17:42:18 +0800 From: "Chen, Tiejun" MIME-Version: 1.0 References: <20140701170206.GB7640@redhat.com> <53B2F238.7000009@citrix.com> <53B3EDF5.4000802@redhat.com> <20140702140033.GG19068@laptop.dumpdata.com> <53B41C27.4030706@redhat.com> <20140702162337.GB32380@laptop.dumpdata.com> <20140703073212.GA20828@redhat.com> <20140703182611.GH13710@konrad-lan.dumpdata.com> <20140703120928.3a1be7d4@jbarnes-desktop> <20140703202740.GC30281@redhat.com> <20140716142008.GA25938@laptop.dumpdata.com> In-Reply-To: <20140716142008.GA25938@laptop.dumpdata.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Intel-gfx] ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konrad Rzeszutek Wilk , "Michael S. Tsirkin" , "Kay, Allen M" Cc: "peter.maydell@linaro.org" , "xen-devel@lists.xensource.com" , Ross Philipson , airlied@linux.ie, daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org, "Kelly.Zytaruk@amd.com" , Jesse Barnes , "qemu-devel@nongnu.org" , Anthony Perard , Stefano Stabellini , "anthony@codemonkey.ws" , "yang.z.zhang@intel.com" , Paolo Bonzini On 2014/7/16 22:20, Konrad Rzeszutek Wilk wrote: > On Thu, Jul 03, 2014 at 11:27:40PM +0300, Michael S. Tsirkin wrote: >> On Thu, Jul 03, 2014 at 12:09:28PM -0700, Jesse Barnes wrote: >>> On Thu, 3 Jul 2014 14:26:12 -0400 >>> Konrad Rzeszutek Wilk wrote: >>> >>>> On Thu, Jul 03, 2014 at 10:32:12AM +0300, Michael S. Tsirkin wrote: >>>>> On Wed, Jul 02, 2014 at 12:23:37PM -0400, Konrad Rzeszutek Wilk wrote: >>>>>> On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote: >>>>>>> Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto: >>>>>>>> With this long thread I lost a bit context about the challenges >>>>>>>> that exists. But let me try summarizing it here - which will hopefully >>>>>>>> get some consensus. >>>>>>>> >>>>>>>> 1). Fix IGD hardware to not use Southbridge magic addresses. >>>>>>>> We can moan and moan but I doubt it is going to change. >>>>>>> >>>>>>> There are two problems: >>>>>>> >>>>>>> - Northbridge (i.e. MCH i.e. PCI host bridge) configuration space addresses >>>>>> >>>>>> Right. So in drivers/gpu/drm/i915/i915_dma.c: >>>>>> 1135 #define MCHBAR_I915 0x44 >>>>>> 1136 #define MCHBAR_I965 0x48 >>>>>> >>>>>> 1147 int reg = INTEL_INFO(dev)->gen >= 4 ? MCHBAR_I965 : MCHBAR_I915; >>>>>> 1152 if (INTEL_INFO(dev)->gen >= 4) >>>>>> 1153 pci_read_config_dword(dev_priv->bridge_dev, reg + 4, &temp_hi); >>>>>> 1154 pci_read_config_dword(dev_priv->bridge_dev, reg, &temp_lo); >>>>>> 1155 mchbar_addr = ((u64)temp_hi << 32) | temp_lo; >>>>>> >>>>>> and >>>>>> >>>>>> 1139 #define DEVEN_REG 0x54 >>>>>> >>>>>> 1193 int mchbar_reg = INTEL_INFO(dev)->gen >= 4 ? MCHBAR_I965 : MCHBAR_I915; >>>>>> 1202 if (IS_I915G(dev) || IS_I915GM(dev)) { >>>>>> 1203 pci_read_config_dword(dev_priv->bridge_dev, DEVEN_REG, &temp); >>>>>> 1204 enabled = !!(temp & DEVEN_MCHBAR_EN); >>>>>> 1205 } else { >>>>>> 1206 pci_read_config_dword(dev_priv->bridge_dev, mchbar_reg, &temp); >>>>>> 1207 enabled = temp & 1; >>>>>> 1208 } >>>>>>> >>>>>>> - Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some versions of >>>>>>> the driver identify it by class, some versions identify it by slot (1f.0). >>>>>> >>>>>> Right, So in drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch >>>>>> which sets the pch_type based on : >>>>>> >>>>>> 432 if (pch->vendor == PCI_VENDOR_ID_INTEL) { >>>>>> 433 unsigned short id = pch->device & INTEL_PCH_DEVICE_ID_MASK; >>>>>> 434 dev_priv->pch_id = id; >>>>>> 435 >>>>>> 436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) { >>>>>> >>>>>> It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00. >>>>>> The INTEL_PCH_DEVICE_ID_MASK is 0xff00 >>>>>>> >>>>>>> To solve the first, make a new machine type, PIIX4-based, and pass through >>>>>>> the registers you need. The patch must document _exactly_ why the registers >>>>>>> are safe to pass. If they are not reserved on PIIX4, the patch must >>>>>>> document what the same offsets mean on PIIX4, and why it's sensible to >>>>>>> assume that firmware for virtual machine will not read/write them. Bonus >>>>>>> point for also documenting the same for Q35. >>>>>> >>>>>> OK. They look to be related to setting up an MBAR , but I don't understand >>>>>> why it is needed. Hopefully some of the i915 folks CC-ed here can answer. >>>>> >>>>> In particular, I think setting up MBAR should move out of i915 and become >>>>> the bridge driver tweak on linux and windows. >>>> >>>> That is an excellent idea. >>>> >>>> However I am still curious to _why_ it has to be done in the first place. >>> >>> The display part of the GPU is partly on the PCH, and it's possible to >>> mix & match them a bit, so we have this detection code to figure things >>> out. In some cases, the PCH display portion may not even be present, >>> so we look for that too. >>> >>> Practically speaking, we could probably assume specific CPU/PCH combos, >>> as I don't think they're generally mixed across generations, though SNB >>> and IVB did have compatible sockets, so there is the possibility of >>> mixing CPT and PPT PCHs, but those are register identical as far as the >>> graphics driver is concerned, so even that should be safe. >>> >>> Beyond that, the other MCH data we need to look at is mirrored into the >>> GPU's MMIO space on current gens. On older gens, we do need to poke at >>> the memory controller a bit to get some info (see >>> intel_setup_mchbar()), but that's not true of newer stuff. Looks like >>> we only short circuit that on VLV though; we could probably do it on >>> SNB+. >> >> That sounds great. Tiejun could you confirm that with >> windows driver guys too? > > ping? Allen, Could you reply this? Thanks Tiejun From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support Date: Thu, 17 Jul 2014 17:42:18 +0800 Message-ID: <53C79A7A.4080904@intel.com> References: <20140701170206.GB7640@redhat.com> <53B2F238.7000009@citrix.com> <53B3EDF5.4000802@redhat.com> <20140702140033.GG19068@laptop.dumpdata.com> <53B41C27.4030706@redhat.com> <20140702162337.GB32380@laptop.dumpdata.com> <20140703073212.GA20828@redhat.com> <20140703182611.GH13710@konrad-lan.dumpdata.com> <20140703120928.3a1be7d4@jbarnes-desktop> <20140703202740.GC30281@redhat.com> <20140716142008.GA25938@laptop.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 160B589FCC for ; Thu, 17 Jul 2014 02:42:28 -0700 (PDT) In-Reply-To: <20140716142008.GA25938@laptop.dumpdata.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Konrad Rzeszutek Wilk , "Michael S. Tsirkin" , "Kay, Allen M" Cc: "peter.maydell@linaro.org" , "xen-devel@lists.xensource.com" , Ross Philipson , airlied@linux.ie, daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org, "Kelly.Zytaruk@amd.com" , "qemu-devel@nongnu.org" , Anthony Perard , Stefano Stabellini , "anthony@codemonkey.ws" , "yang.z.zhang@intel.com" , Paolo Bonzini List-Id: intel-gfx@lists.freedesktop.org On 2014/7/16 22:20, Konrad Rzeszutek Wilk wrote: > On Thu, Jul 03, 2014 at 11:27:40PM +0300, Michael S. Tsirkin wrote: >> On Thu, Jul 03, 2014 at 12:09:28PM -0700, Jesse Barnes wrote: >>> On Thu, 3 Jul 2014 14:26:12 -0400 >>> Konrad Rzeszutek Wilk wrote: >>> >>>> On Thu, Jul 03, 2014 at 10:32:12AM +0300, Michael S. Tsirkin wrote: >>>>> On Wed, Jul 02, 2014 at 12:23:37PM -0400, Konrad Rzeszutek Wilk wrote: >>>>>> On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote: >>>>>>> Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto: >>>>>>>> With this long thread I lost a bit context about the challenges >>>>>>>> that exists. But let me try summarizing it here - which will hopefully >>>>>>>> get some consensus. >>>>>>>> >>>>>>>> 1). Fix IGD hardware to not use Southbridge magic addresses. >>>>>>>> We can moan and moan but I doubt it is going to change. >>>>>>> >>>>>>> There are two problems: >>>>>>> >>>>>>> - Northbridge (i.e. MCH i.e. PCI host bridge) configuration space addresses >>>>>> >>>>>> Right. So in drivers/gpu/drm/i915/i915_dma.c: >>>>>> 1135 #define MCHBAR_I915 0x44 >>>>>> 1136 #define MCHBAR_I965 0x48 >>>>>> >>>>>> 1147 int reg = INTEL_INFO(dev)->gen >= 4 ? MCHBAR_I965 : MCHBAR_I915; >>>>>> 1152 if (INTEL_INFO(dev)->gen >= 4) >>>>>> 1153 pci_read_config_dword(dev_priv->bridge_dev, reg + 4, &temp_hi); >>>>>> 1154 pci_read_config_dword(dev_priv->bridge_dev, reg, &temp_lo); >>>>>> 1155 mchbar_addr = ((u64)temp_hi << 32) | temp_lo; >>>>>> >>>>>> and >>>>>> >>>>>> 1139 #define DEVEN_REG 0x54 >>>>>> >>>>>> 1193 int mchbar_reg = INTEL_INFO(dev)->gen >= 4 ? MCHBAR_I965 : MCHBAR_I915; >>>>>> 1202 if (IS_I915G(dev) || IS_I915GM(dev)) { >>>>>> 1203 pci_read_config_dword(dev_priv->bridge_dev, DEVEN_REG, &temp); >>>>>> 1204 enabled = !!(temp & DEVEN_MCHBAR_EN); >>>>>> 1205 } else { >>>>>> 1206 pci_read_config_dword(dev_priv->bridge_dev, mchbar_reg, &temp); >>>>>> 1207 enabled = temp & 1; >>>>>> 1208 } >>>>>>> >>>>>>> - Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some versions of >>>>>>> the driver identify it by class, some versions identify it by slot (1f.0). >>>>>> >>>>>> Right, So in drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch >>>>>> which sets the pch_type based on : >>>>>> >>>>>> 432 if (pch->vendor == PCI_VENDOR_ID_INTEL) { >>>>>> 433 unsigned short id = pch->device & INTEL_PCH_DEVICE_ID_MASK; >>>>>> 434 dev_priv->pch_id = id; >>>>>> 435 >>>>>> 436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) { >>>>>> >>>>>> It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00. >>>>>> The INTEL_PCH_DEVICE_ID_MASK is 0xff00 >>>>>>> >>>>>>> To solve the first, make a new machine type, PIIX4-based, and pass through >>>>>>> the registers you need. The patch must document _exactly_ why the registers >>>>>>> are safe to pass. If they are not reserved on PIIX4, the patch must >>>>>>> document what the same offsets mean on PIIX4, and why it's sensible to >>>>>>> assume that firmware for virtual machine will not read/write them. Bonus >>>>>>> point for also documenting the same for Q35. >>>>>> >>>>>> OK. They look to be related to setting up an MBAR , but I don't understand >>>>>> why it is needed. Hopefully some of the i915 folks CC-ed here can answer. >>>>> >>>>> In particular, I think setting up MBAR should move out of i915 and become >>>>> the bridge driver tweak on linux and windows. >>>> >>>> That is an excellent idea. >>>> >>>> However I am still curious to _why_ it has to be done in the first place. >>> >>> The display part of the GPU is partly on the PCH, and it's possible to >>> mix & match them a bit, so we have this detection code to figure things >>> out. In some cases, the PCH display portion may not even be present, >>> so we look for that too. >>> >>> Practically speaking, we could probably assume specific CPU/PCH combos, >>> as I don't think they're generally mixed across generations, though SNB >>> and IVB did have compatible sockets, so there is the possibility of >>> mixing CPT and PPT PCHs, but those are register identical as far as the >>> graphics driver is concerned, so even that should be safe. >>> >>> Beyond that, the other MCH data we need to look at is mirrored into the >>> GPU's MMIO space on current gens. On older gens, we do need to poke at >>> the memory controller a bit to get some info (see >>> intel_setup_mchbar()), but that's not true of newer stuff. Looks like >>> we only short circuit that on VLV though; we could probably do it on >>> SNB+. >> >> That sounds great. Tiejun could you confirm that with >> windows driver guys too? > > ping? Allen, Could you reply this? Thanks Tiejun