[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [Intel-gfx] ResettRe: [v5][PATCH 0/5] xen: add Intel IGD passthrough support
On Thu, Jul 24, 2014 at 09:44:41AM +0800, Chen, Tiejun wrote: > On 2014/7/24 4:54, Konrad Rzeszutek Wilk wrote: > >On Sat, Jul 19, 2014 at 12:27:21AM +0000, Kay, Allen M wrote: > >>>For the MCH PCI registers that do need to be read - can you tell us which > >>>ones those are? > >> > >>In qemu/hw/xen_pt_igd.c/igd_pci_read(), following MCH PCI config register > >>reads are passthrough to the host HW. Some of the registers are needed by > >>Ironlake GFX driver which we probably can remove. I did a trace recently > >>on Broadwell, the number of register accessed are even smaller (0, 2, 2c, > >>2e, 50, 52, a0, a4). Given that we now have integrated MCH and GPU in the > >>same socket, looks like driver can easily remove reads for offsets 0 - 0x2e. > >> > >> case 0x00: /* vendor id */ > >> case 0x02: /* device id */ > >> case 0x08: /* revision id */ > >> case 0x2c: /* sybsystem vendor id */ > >> case 0x2e: /* sybsystem id */ > > > >Right. We can fix the i915 to use the mechanism that Michael mentioned. > >(see attached RFC patches) > > > >> case 0x50: /* SNB: processor graphics control register */ > >> case 0x52: /* processor graphics control register */ > >> case 0xa0: /* top of memory */ > >> case 0xb0: /* ILK: BSM: should read from dev 2 offset > >> 0x5c */ > >> case 0x58: /* SNB: PAVPC Offset */ > >> case 0xa4: /* SNB: graphics base of stolen memory */ > >> case 0xa8: /* SNB: base of GTT stolen memory */ > > > >I dug in the intel-gtt.c (see ironlake_gtt_driver) to figure out this > >a bit more. As in, I speculated, what if we returned 0 (and not implement > >any support for reading from these registers). What would happen? > > > >0x52 for Ironlake (g5): > >------------------ > >It looks like intel_gmch_probe is called when i915_gem_gtt_init > >starts (there is a lot of code that looks to be used between > >intel-gtt.c and i915.c). > > > >Anyhow the interesting parts are that i9xx_setup ends up calling > >ioremap the i915 BAR to setup some of these registers for older generations. > > > >Then i965_gtt_total_entries gets which reads 0x52, but it is only > >needed for v5 generation. For other (v4 and G33) it reads it from the GPU's > >0x2020 register. > > > >If there is a mismatch, it writes to the GPU at 0x2020 to update the > >the size based on the bridge. And then it reads from 0x2020 and that > >is returned and stuck in intel_private.gtt_total_entries. > > > >So 0x52 in the emulated bridge could be populated with what the > >GPU has at 0x2020. And the writes go in the emulated area. > > > >0x52 for v6 -> v8: > >----------------- > >We seem to go to gen6_gmch_probe which just figures out the > >the GTT based on the GPU's BAR sizes. The stolen values > >are read from 0x50 from the GPU. So no need to touch the bridge > >(see gen6_gmch_probe) > > > >OK, so no need to have 0x52 or 0x50 in the bridge. > > > >0xA0: > >--------- > >Could not find any reference in the Linux code. Why would > >Windows driver need this? If we returned the _real_ TOM would > >it matter? Is it used to figure out the device should use 32-bit > >DMA operations or 40-bit? > > > >0xb0 or 0x5c: > >------------- > >No mention of them in the Linux code. > > > >0x58, 0xa4, 0xa8: > >----------------- > >No usage of them in the Linux code. We seem to be using the 0x52 > >from the bridge and 0x2020 from the GPU for this functionality. > > > > > >So in theory*, if using Ironlake we need to have a proper value > >in 0x52 in the bridge. But for the later chipsets we do not need > >these values (I am assuming that intel_setup_mchbar can safely > >return as it does that for Ironlake and could very well for later > >generations). > > > >Can this be reflected in the Windows driver as well? > > > >P.S. > >*theory: That is assuming we modify the Linux i915_drv.c:intel_detect_pch > >to pick up the id as suggested earlier. See the RFC patches attached. > >(Not compile tested at all!) > > I take a look these patches, looks we still scan all PCI Bridge to walk all > PCHs. So this means we still need to fake a PCI bridge, right? Or maybe you > don't cover this problem this time. I totally forgot. Feel free to fix that. > > I prefer we should check dev slot to get that PCH like my previous patch, Uh? Your patch was checking for 0:1f.0, not the slot of the device. (see https://lkml.org/lkml/2014/6/19/121) > "gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class > type". Because Windows always use this way, so I think this point should be > same between Linux and Windows. Didn't we discuss that we did not want to pass in PCH at all if we can? And from the observation I made above it seems that we can safely do it under Linux. With Windows I don't know - but I presume the answer is yes too. > > Or we need anther better way to unify all OSs. Yes. The observation is that a lot of these registers are aliased in the GPU. As such we can skip some of this bridge poking. Hm. I should have gotten further and also done this on baremetal, but figured as an RFC it would paint a picture of what we had in mind? > > Thanks > Tiejun > > >> > >>Allen > >> > >>-----Original Message----- > >>From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx] > >>Sent: Friday, July 18, 2014 6:45 AM > >>To: Kay, Allen M > >>Cc: Michael S. Tsirkin; Jesse Barnes; peter.maydell@xxxxxxxxxx; > >>xen-devel@xxxxxxxxxxxxxxxxxxx; Ross Philipson; airlied@xxxxxxxx; > >>daniel.vetter@xxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > >>Kelly.Zytaruk@xxxxxxx; qemu-devel@xxxxxxxxxx; Anthony Perard; Stefano > >>Stabellini; anthony@xxxxxxxxxxxxx; Paolo Bonzini; Zhang, Yang Z; Chen, > >>Tiejun > >>Subject: Re: [Intel-gfx] ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: add > >>Intel IGD passthrough support > >> > >>On Thu, Jul 17, 2014 at 05:37:12PM +0000, Kay, Allen M wrote: > >>>>That sounds great. Tiejun could you confirm that with windows driver guys > >>>>too? > >>> > >>>I believe windows driver can also assume specific CPU/PCH combos. I will > >>>discuss this with native Windows driver guys. Preferably, the same code > >>>path can be used for both native and virtualization cases to avoid > >>>frequent breakage as most developers and QA do not test new code changes > >>>in virtualization environment. > >>> > >>>I have verified that Windows driver do not need to write to any MCH PCI > >>>registers on HSW/BDW so the PCI write function can be removed. The MCH > >>>PCI registers that need to be read, we are working with HW team to get > >>>them mirrored in GPU MMIO registers in future HW. > >>> > >> > >>For the MCH PCI registers that do need to be read - can you tell us which > >>ones those are? > >> > >>Thank you! > >>>Allen > >>> > >>>-----Original Message----- > >>>From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On > >>>Behalf Of Michael S. Tsirkin > >>>Sent: Thursday, July 03, 2014 1:28 PM > >>>To: Jesse Barnes > >>>Cc: peter.maydell@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxx; Ross > >>>Philipson; Konrad Rzeszutek Wilk; airlied@xxxxxxxx; > >>>daniel.vetter@xxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > >>>Kelly.Zytaruk@xxxxxxx; qemu-devel@xxxxxxxxxx; Anthony Perard; Stefano > >>>Stabellini; anthony@xxxxxxxxxxxxx; Paolo Bonzini; Zhang, Yang Z; Chen, > >>>Tiejun > >>>Subject: Re: [Intel-gfx] ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: > >>>add Intel IGD passthrough support > >>> > >>>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 <konrad.wilk@xxxxxxxxxx> 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? > >>> > >>>>-- > >>>>Jesse Barnes, Intel Open Source Technology Center > >>>_______________________________________________ > >>>Intel-gfx mailing list > >>>Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |