[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
- To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
- From: "Chen, Tiejun" <tiejun.chen@xxxxxxxxx>
- Date: Tue, 29 Jul 2014 14:59:21 +0800
- Cc: "peter.maydell@xxxxxxxxxx" <peter.maydell@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "anthony@xxxxxxxxxxxxx" <anthony@xxxxxxxxxxxxx>, "Michael S. Tsirkin" <mst@xxxxxxxxxx>, "airlied@xxxxxxxx" <airlied@xxxxxxxx>, "daniel.vetter@xxxxxxxx" <daniel.vetter@xxxxxxxx>, "intel-gfx@xxxxxxxxxxxxxxxxxxxxx" <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>, "Kay, Allen M" <allen.m.kay@xxxxxxxxx>, "Kelly.Zytaruk@xxxxxxx" <Kelly.Zytaruk@xxxxxxx>, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>, "qemu-devel@xxxxxxxxxx" <qemu-devel@xxxxxxxxxx>, "Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxx>, Ross Philipson <ross.philipson@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>, Paolo Bonzini <pbonzini@xxxxxxxxxx>
- Delivery-date: Tue, 29 Jul 2014 06:59:45 +0000
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
On 2014/7/26 1:01, Konrad Rzeszutek Wilk wrote:
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.
Sorry for this delay response.
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.
Yes.
(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?
I'm a bit confused since I guess I'm missing something important in this
long discussion. I guess we just fake a simple PCIe device but just has
PCI_VENDOR_ID_XEN/Real_PCH_Device_ID, and then well probe such a PCIe
device inside intel_detect_pch(), right?
And this means we will not support those existing drivers?
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
Sounds reasonable but I'm not an Windows/Linux GFX driver developer, so
I think we need to wait Allen to double check these points.
Thanks
Tiejun
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
|
