On Wed, 19 Dec 2012 01:43:22 +0800
"G.R." <firemeteor at users.sourceforge.net> wrote:
> On Wed, Dec 19, 2012 at 12:53 AM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > On Wed, 19 Dec 2012 00:49:28 +0800
> > "G.R." <firemeteor at users.sourceforge.net> wrote:
> >
> >> Hi guys,
> >>
> >> In XEN HVM guest, there is always an emulated PIIX3 ISA bridge on slot 01.0.
> >> This shadows the PCH ISA bridge on 1f.0 with the current
> >> intel_detect_pch() implementation.
> >> The issue can be easily solved by looping through all the ISA bridges
> >> until the first match is found, instead of just check against the
> >> first one.
> >>
> >> Here I attach the patch I used locally. It's created on Torvalds's git.
> >> Looking forward to your comments.
> >>
> >> Thanks,
> >> Timothy
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index 530db83..3f7e5fb 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -408,9 +408,11 @@ void intel_detect_pch(struct drm_device *dev)
> >> * underneath. This is a requirement from virtualization team.
> >> */
> >> pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
> >> - if (pch) {
> >> + while (pch) {
> >> + struct pci_dev * curr = pch;
> >> if (pch->vendor == PCI_VENDOR_ID_INTEL) {
> >> unsigned short id;
> >> + unsigned found = 1;
> >> id = pch->device & INTEL_PCH_DEVICE_ID_MASK;
> >> dev_priv->pch_id = id;
> >>
> >> @@ -440,10 +442,20 @@ void intel_detect_pch(struct drm_device *dev)
> >> dev_priv->num_pch_pll = 0;
> >> DRM_DEBUG_KMS("Found LynxPoint LP PCH\n");
> >> WARN_ON(!IS_HASWELL(dev));
> >> + } else {
> >> + found = 0;
> >> + }
> >> + if (found) {
> >> + BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
> >> + pci_dev_put(pch);
> >> + break;
> >> }
> >> - BUG_ON(dev_priv->num_pch_pll > I915_NUM_PLLS);
> >> }
> >> - pci_dev_put(pch);
> >> + pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, curr);
> >> + pci_dev_put(curr);
> >> + }
> >> + if (!pch) {
> >> + DRM_DEBUG_KMS("No PCH found?\n");
> >> }
> >> }
> >>
> >
> > I'd like to see a comment about this being for Xen in here, and I
> > wonder if there are other places where we might have to worry about the
> > Xen implementation. In that case, setting a flag in dev_priv when we
> > don't find the PCH where we expect would be a good idea too.
> >
>
> I can add a comment here if the overall idea is acceptable to you.
> But there is already a comment mentioning that the ISA bridge check is
> for virtualization:
>
> 404 /*
> 405 * The reason to probe ISA bridge instead of Dev31:Fun0 is to
> 406 * make graphics device passthrough work easy for VMM, that only
> 407 * need to expose ISA bridge to let driver know the real hardware
> 408 * underneath. This is a requirement from virtualization team.
> 409 */
>
> > Ack on the general idea though; I'd like us to be able to run under Xen
> > without modification.
> >
>
> Stefano may be able to comment if it's feasible to achieve zero
> modification in this case.
> I believe this has something to do with getting rid of the PIIX3
> device provided by qemu.
>
> But generally I think it's very hard to achieve perfect emulation.
> You can't always foresee what assumption a guest driver would make.
> Maybe we need some compromise.
I meant that I'd like to see any other patches required for Xen get
merged, that way people won't have to patch their kernels for i915
under Xen.
--
Jesse Barnes, Intel Open Source Technology Center
