Re: [Xen-devel] [Intel-gfx] ResettRe: [v5][PATCH 0/5] xen: add Intel IGD passthrough support

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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