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

[Chen, Tiejun]

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 <konrad.wilk at oracle.com> 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