All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO
  2017-02-20 11:17 [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO Xiong Zhang
@ 2017-02-20  3:20 ` no-reply
  0 siblings, 0 replies; 6+ messages in thread
From: no-reply @ 2017-02-20  3:20 UTC (permalink / raw)
  To: xiong.y.zhang; +Cc: famz, qemu-devel, jike.song, zhiyuan.lv, zhi.a.wang

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO
Message-id: 20170220111716.10471-1-xiong.y.zhang@intel.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20170220111716.10471-1-xiong.y.zhang@intel.com -> patchew/20170220111716.10471-1-xiong.y.zhang@intel.com
Switched to a new branch 'test'
f533dc3 vfio/pci-quirks.c: Disable stolen memory for igd VFIO

=== OUTPUT BEGIN ===
Checking PATCH 1/1: vfio/pci-quirks.c: Disable stolen memory for igd VFIO...
ERROR: code indent should never use tabs
#38: FILE: hw/vfio/pci-quirks.c:1370:
+^I/* This must be an Intel VGA device. */$

ERROR: code indent should never use tabs
#39: FILE: hw/vfio/pci-quirks.c:1371:
+^Iif (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||$

ERROR: braces {} are necessary for all arms of this statement
#39: FILE: hw/vfio/pci-quirks.c:1371:
+	if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
[...]

ERROR: code indent should never use tabs
#40: FILE: hw/vfio/pci-quirks.c:1372:
+^I^I!vfio_is_vga(vdev) || nr != 4)$

ERROR: code indent should never use tabs
#41: FILE: hw/vfio/pci-quirks.c:1373:
+^I^Ireturn;$

ERROR: code indent should never use tabs
#43: FILE: hw/vfio/pci-quirks.c:1375:
+^I/*$

ERROR: code indent should never use tabs
#44: FILE: hw/vfio/pci-quirks.c:1376:
+^I * IGD is not a standard, they like to change their specs often.  We$

ERROR: code indent should never use tabs
#45: FILE: hw/vfio/pci-quirks.c:1377:
+^I * only attempt to support back to SandBridge and we hope that newer$

ERROR: code indent should never use tabs
#46: FILE: hw/vfio/pci-quirks.c:1378:
+^I * devices maintain compatibility with generation 8.$

ERROR: code indent should never use tabs
#47: FILE: hw/vfio/pci-quirks.c:1379:
+^I */$

ERROR: code indent should never use tabs
#48: FILE: hw/vfio/pci-quirks.c:1380:
+^Igen = igd_gen(vdev);$

ERROR: code indent should never use tabs
#49: FILE: hw/vfio/pci-quirks.c:1381:
+^Iif (gen != 6 && gen != 8) {$

ERROR: code indent should never use tabs
#50: FILE: hw/vfio/pci-quirks.c:1382:
+^I^Ierror_report("IGD device %s is unsupported in legacy mode, "$

ERROR: code indent should never use tabs
#51: FILE: hw/vfio/pci-quirks.c:1383:
+^I^I^I     "try SandyBridge or newer", vdev->vbasedev.name);$

ERROR: code indent should never use tabs
#52: FILE: hw/vfio/pci-quirks.c:1384:
+^I^Ireturn;$

ERROR: code indent should never use tabs
#53: FILE: hw/vfio/pci-quirks.c:1385:
+^I}$

ERROR: code indent should never use tabs
#54: FILE: hw/vfio/pci-quirks.c:1386:
+^I/*$

ERROR: code indent should never use tabs
#55: FILE: hw/vfio/pci-quirks.c:1387:
+^I * If this isn't at address 00:02.0, bios won't reserv stolen$

ERROR: code indent should never use tabs
#56: FILE: hw/vfio/pci-quirks.c:1388:
+^I * memory in E820, then others could use stolen memory. If guest$

ERROR: code indent should never use tabs
#57: FILE: hw/vfio/pci-quirks.c:1389:
+^I * graphic driver still use stolen memory, system maybe hang.$

ERROR: code indent should never use tabs
#58: FILE: hw/vfio/pci-quirks.c:1390:
+^I * so we set stolen memory size to 0 and guest graphic driver won't$

ERROR: code indent should never use tabs
#59: FILE: hw/vfio/pci-quirks.c:1391:
+^I * use stolen memory.$

ERROR: code indent should never use tabs
#60: FILE: hw/vfio/pci-quirks.c:1392:
+^I */$

ERROR: code indent should never use tabs
#61: FILE: hw/vfio/pci-quirks.c:1393:
+^Igmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);$

ERROR: code indent should never use tabs
#62: FILE: hw/vfio/pci-quirks.c:1394:
+^Igmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));$

ERROR: code indent should never use tabs
#64: FILE: hw/vfio/pci-quirks.c:1396:
+^I/* GMCH is read-only, emulated */$

ERROR: code indent should never use tabs
#65: FILE: hw/vfio/pci-quirks.c:1397:
+^Ipci_set_long(vdev->pdev.config + IGD_GMCH, gmch);$

ERROR: code indent should never use tabs
#66: FILE: hw/vfio/pci-quirks.c:1398:
+^Ipci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);$

ERROR: code indent should never use tabs
#67: FILE: hw/vfio/pci-quirks.c:1399:
+^Ipci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);$

ERROR: code indent should never use tabs
#69: FILE: hw/vfio/pci-quirks.c:1401:
+^I/*$

ERROR: code indent should never use tabs
#70: FILE: hw/vfio/pci-quirks.c:1402:
+^I * This must be at address 00:02.0 for us to even onsider enabling$

ERROR: code indent should never use tabs
#71: FILE: hw/vfio/pci-quirks.c:1403:
+^I * legacy mode.  The vBIOS has dependencies on the PCI bus address.$

ERROR: code indent should never use tabs
#72: FILE: hw/vfio/pci-quirks.c:1404:
+^I */$

ERROR: code indent should never use tabs
#73: FILE: hw/vfio/pci-quirks.c:1405:
+^Iif (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),$

ERROR: braces {} are necessary for all arms of this statement
#73: FILE: hw/vfio/pci-quirks.c:1405:
+	if (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
[...]

ERROR: code indent should never use tabs
#74: FILE: hw/vfio/pci-quirks.c:1406:
+^I^I^I^I^I   0, PCI_DEVFN(0x2, 0)))$

ERROR: code indent should never use tabs
#75: FILE: hw/vfio/pci-quirks.c:1407:
+^I^Ireturn;$

ERROR: code indent should never use tabs
#117: FILE: hw/vfio/pci-quirks.c:1552:
+^I^I^Ipci_set_long(vdev->pdev.config + IGD_GMCH, gmch);$

total: 38 errors, 0 warnings, 105 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO
@ 2017-02-20 11:17 Xiong Zhang
  2017-02-20  3:20 ` no-reply
  0 siblings, 1 reply; 6+ messages in thread
From: Xiong Zhang @ 2017-02-20 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: jike.song, zhi.a.wang, zhiyuan.lv, XiongZhang

From: XiongZhang <xiong.y.zhang@intel.com>

If IGD isn't assigned at 00:02.0 in UPT and host bios enable stolen
memory, seabios won't reseave stolen memory in E820 for guest. Then
both Intel graphic driver and others in guest could use stolen
memory, this will generate system hang. So we should disable stolen
memory in this case.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99028
		https://bugs.freedesktop.org/show_bug.cgi?id=99025

Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
Tested-by: Terrence Xu <terrence.xu@intel.com>
---
 hw/vfio/pci-quirks.c | 71 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 1e97bc4..3c03577 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1364,17 +1364,44 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     uint32_t gmch;
     uint16_t cmd_orig, cmd;
 
-    /*
-     * This must be an Intel VGA device at address 00:02.0 for us to even
-     * consider enabling legacy mode.  The vBIOS has dependencies on the
-     * PCI bus address.
-     */
-    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
-        !vfio_is_vga(vdev) || nr != 4 ||
-        &vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
-                                       0, PCI_DEVFN(0x2, 0))) {
-        return;
-    }
+	/* This must be an Intel VGA device. */
+	if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
+		!vfio_is_vga(vdev) || nr != 4)
+		return;
+
+	/*
+	 * IGD is not a standard, they like to change their specs often.  We
+	 * only attempt to support back to SandBridge and we hope that newer
+	 * devices maintain compatibility with generation 8.
+	 */
+	gen = igd_gen(vdev);
+	if (gen != 6 && gen != 8) {
+		error_report("IGD device %s is unsupported in legacy mode, "
+			     "try SandyBridge or newer", vdev->vbasedev.name);
+		return;
+	}
+	/*
+	 * If this isn't at address 00:02.0, bios won't reserv stolen
+	 * memory in E820, then others could use stolen memory. If guest
+	 * graphic driver still use stolen memory, system maybe hang.
+	 * so we set stolen memory size to 0 and guest graphic driver won't
+	 * use stolen memory.
+	 */
+	gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
+	gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
+
+	/* GMCH is read-only, emulated */
+	pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
+	pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
+	pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
+
+	/*
+	 * This must be at address 00:02.0 for us to even onsider enabling
+	 * legacy mode.  The vBIOS has dependencies on the PCI bus address.
+	 */
+	if (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
+					   0, PCI_DEVFN(0x2, 0)))
+		return;
 
     /*
      * We need to create an LPC/ISA bridge at PCI bus address 00:1f.0 that we
@@ -1391,18 +1418,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     }
 
     /*
-     * IGD is not a standard, they like to change their specs often.  We
-     * only attempt to support back to SandBridge and we hope that newer
-     * devices maintain compatibility with generation 8.
-     */
-    gen = igd_gen(vdev);
-    if (gen != 6 && gen != 8) {
-        error_report("IGD device %s is unsupported in legacy mode, "
-                     "try SandyBridge or newer", vdev->vbasedev.name);
-        return;
-    }
-
-    /*
      * Most of what we're doing here is to enable the ROM to run, so if
      * there's no ROM, there's no point in setting up this quirk.
      * NB. We only seem to get BIOS ROMs, so a UEFI VM would need CSM support.
@@ -1457,8 +1472,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
         goto out;
     }
 
-    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
-
     /*
      * If IGD VGA Disable is clear (expected) and VGA is not already enabled,
      * try to enable it.  Probably shouldn't be using legacy mode without VGA,
@@ -1526,12 +1539,11 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
      * when IVD (IGD VGA Disable) is clear, but the claim is that it's unused,
      * so let's not waste VM memory for it.
      */
-    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
-
     if (vdev->igd_gms) {
         if (vdev->igd_gms <= 0x10) {
             gms_mb = vdev->igd_gms * 32;
             gmch |= vdev->igd_gms << (gen < 8 ? 3 : 8);
+			pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
         } else {
             error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);
             vdev->igd_gms = 0;
@@ -1551,11 +1563,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
                     bdsm_size, sizeof(*bdsm_size));
 
-    /* GMCH is read-only, emulated */
-    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
-    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
-    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
-
     /* BDSM is read-write, emulated.  The BIOS needs to be able to write it */
     pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
     pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO
  2017-02-20 20:15 ` Alex Williamson
  2017-02-21  4:48   ` Zhang, Xiong Y
@ 2017-02-21  5:14   ` Zhang, Xiong Y
  1 sibling, 0 replies; 6+ messages in thread
From: Zhang, Xiong Y @ 2017-02-21  5:14 UTC (permalink / raw)
  To: Alex Williamson
  Cc: qemu-devel, Song, Jike, Lv, Zhiyuan, Wang, Zhi A, Zhang, Xiong Y

> On Mon, 20 Feb 2017 19:42:54 +0800
> Xiong Zhang <xiong.y.zhang@intel.com> wrote:
> 
> > From: XiongZhang <xiong.y.zhang@intel.com>
> >
> > If IGD isn't assigned at 00:02.0 in UPT and host bios enable stolen
> > memory, seabios won't reseave stolen memory in E820 for guest. Then
> > both Intel graphic driver and others in guest could use stolen
> > memory, this will generate system hang. So we should disable stolen
> > memory in this case.
> 
> Wasn't the intent of UPT mode that it removed all of the BIOS and
> chipset dependencies of IGD such that it could be assigned as just
> another PCI device?  Does this mean that the drivers fail to meet that
> promise by evaluating the size and location of stolen memory as
> programmed on the physical device even in UPT mode?
[Zhang, Xiong Y] The intent of UPT mode is correct. Driver also evaluate
the size and location of stolen memory correctly.
The current problem is: when IGD isn't at 00:02.0, seabios don't create memory
region and reserve memory resource in E820 for stolen memory.
So guest OS maybe assign stolen memory MMIO to other devices, when IGD driver
access stolen memory, it access the wrong device and cause system error. 
If guest OS don't assign stolen memory MMIO to other devices, then there
isn't gpa to hpa translate for stolen memory, guest IGD driver couldn't
access it.
> 
> I'm a little confused by the use of the term "others" here and in the
> comment below.  Can you be more specific what other software beyond the
> graphics driver is evaluating the size or location of stolen memory?
> 
> > Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=99028
> >          https://bugs.freedesktop.org/show_bug.cgi?id=99025
> >
> > Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> > Tested-by: Terrence Xu <terrence.xu@intel.com>
> > ---
> >  hw/vfio/pci-quirks.c | 63
> ++++++++++++++++++++++++++++++----------------------
> >  1 file changed, 36 insertions(+), 27 deletions(-)
> >
> > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> > index 1e97bc4..015d0c2 100644
> > --- a/hw/vfio/pci-quirks.c
> > +++ b/hw/vfio/pci-quirks.c
> > @@ -1364,14 +1364,43 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >      uint32_t gmch;
> >      uint16_t cmd_orig, cmd;
> >
> > +    /* This must be an Intel VGA device. */
> > +    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> > +        !vfio_is_vga(vdev) || nr != 4) {
> > +        return;
> > +    }
> > +
> >      /*
> > -     * This must be an Intel VGA device at address 00:02.0 for us to even
> > -     * consider enabling legacy mode.  The vBIOS has dependencies on
> the
> > -     * PCI bus address.
> > +     * IGD is not a standard, they like to change their specs often.  We
> > +     * only attempt to support back to SandBridge and we hope that
> newer
> > +     * devices maintain compatibility with generation 8.
> >       */
> > -    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> > -        !vfio_is_vga(vdev) || nr != 4 ||
> > -        &vdev->pdev !=
> pci_find_device(pci_device_root_bus(&vdev->pdev),
> > +    gen = igd_gen(vdev);
> > +    if (gen != 6 && gen != 8) {
> > +        error_report("IGD device %s is unsupported in legacy mode, "
> > +                     "try SandyBridge or newer",
> vdev->vbasedev.name);
> 
> This is a little bit misleading now since this is no longer exclusively
> a legacy mode path, a user trying to use UPT mode might disregard this
> as noise.  Perhaps...
> 
>     error_report("IGD device %s is unsupported by IGD quirks, "
>                  "try SandyBridge or newer", vdev->vbasedev.name);
> 
[Zhang, Xiong Y] yes, I will follow it.
> 
> > +        return;
> > +    }
> > +    /*
> > +     * If this isn't at address 00:02.0, bios won't reserv stolen
> 
> s/reserv/reserve/
> 
> > +     * memory in E820, then others could use stolen memory. If guest
> > +     * graphic driver still use stolen memory, system maybe hang.
> > +     * so we set stolen memory size to 0 and guest graphic driver won't
> > +     * use stolen memory.
> 
> Based on my understanding of the bug, I might suggest:
> 
>   Regardless of running in UPT or legacy mode, the guest graphics
>   driver may attempt to use stolen memory, however only legacy mode has
>   BIOS support for reserving stolen memory in the guest VM.  Emulate
>   the GMCH register in all cases and zero out the stolen memory size
>   here.  Legacy mode may request allocation and re-write this below.
> 
[Zhang, Xiong Y] yes, As you comment, things become more clear. I will
Follow it. thanks a lot.
> > +     */
> > +    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> > +    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> > +
> > +    /* GMCH is read-only, emulated */
> > +    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> > +    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> > +    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> > +
> > +    /*
> > +     * This must be at address 00:02.0 for us to even onsider enabling
> > +     * legacy mode.  The vBIOS has dependencies on the PCI bus address.
> > +     */
> > +    if (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
> >                                         0, PCI_DEVFN(0x2, 0))) {
> >          return;
> >      }
> > @@ -1391,18 +1420,6 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >      }
> >
> >      /*
> > -     * IGD is not a standard, they like to change their specs often.  We
> > -     * only attempt to support back to SandBridge and we hope that newer
> > -     * devices maintain compatibility with generation 8.
> > -     */
> > -    gen = igd_gen(vdev);
> > -    if (gen != 6 && gen != 8) {
> > -        error_report("IGD device %s is unsupported in legacy mode, "
> > -                     "try SandyBridge or newer", vdev->vbasedev.name);
> > -        return;
> > -    }
> > -
> > -    /*
> >       * Most of what we're doing here is to enable the ROM to run, so if
> >       * there's no ROM, there's no point in setting up this quirk.
> >       * NB. We only seem to get BIOS ROMs, so a UEFI VM would need
> CSM support.
> > @@ -1457,8 +1474,6 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >          goto out;
> >      }
> >
> > -    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> > -
> >      /*
> >       * If IGD VGA Disable is clear (expected) and VGA is not already
> enabled,
> >       * try to enable it.  Probably shouldn't be using legacy mode without
> VGA,
> > @@ -1526,12 +1541,11 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >       * when IVD (IGD VGA Disable) is clear, but the claim is that it's
> unused,
> >       * so let's not waste VM memory for it.
> >       */
> > -    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> > -
> >      if (vdev->igd_gms) {
> >          if (vdev->igd_gms <= 0x10) {
> >              gms_mb = vdev->igd_gms * 32;
> >              gmch |= vdev->igd_gms << (gen < 8 ? 3 : 8);
> > +            pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> >          } else {
> >              error_report("Unsupported IGD GMS value 0x%x",
> vdev->igd_gms);
> >              vdev->igd_gms = 0;
> > @@ -1551,11 +1565,6 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >      fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
> >                      bdsm_size, sizeof(*bdsm_size));
> >
> > -    /* GMCH is read-only, emulated */
> > -    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> > -    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> > -    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> > -
> >      /* BDSM is read-write, emulated.  The BIOS needs to be able to write
> it */
> >      pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
> >      pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO
  2017-02-20 20:15 ` Alex Williamson
@ 2017-02-21  4:48   ` Zhang, Xiong Y
  2017-02-21  5:14   ` Zhang, Xiong Y
  1 sibling, 0 replies; 6+ messages in thread
From: Zhang, Xiong Y @ 2017-02-21  4:48 UTC (permalink / raw)
  To: Alex Williamson
  Cc: qemu-devel, Song, Jike, Lv, Zhiyuan, Wang, Zhi A, Zhang, Xiong Y

> 
> On Mon, 20 Feb 2017 19:42:54 +0800
> Xiong Zhang <xiong.y.zhang@intel.com> wrote:
> 
> > From: XiongZhang <xiong.y.zhang@intel.com>
> >
> > If IGD isn't assigned at 00:02.0 in UPT and host bios enable stolen
> > memory, seabios won't reseave stolen memory in E820 for guest. Then
> > both Intel graphic driver and others in guest could use stolen
> > memory, this will generate system hang. So we should disable stolen
> > memory in this case.
> 
> Wasn't the intent of UPT mode that it removed all of the BIOS and
> chipset dependencies of IGD such that it could be assigned as just
> another PCI device?  Does this mean that the drivers fail to meet that
> promise by evaluating the size and location of stolen memory as
> programmed on the physical device even in UPT mode?
[Zhang, Xiong Y] The intent of UPT mode is correct. Driver also evaluate
the size and location of stolen memory correctly.
The current problem is: when IGD isn't at 00:02.0, seabios don't create memory
region and reserve memory resource in E820 for stolen memory.
So guest OS maybe assign stolen memory MMIO to other devices, when IGD driver
access stolen memory, it access the wrong device and cause system error. 
If guest OS don't assign stolen memory MMIO to other devices, then there
isn't gpa to hpa translate for stolen memory, guest IGD driver couldn't
access it. 
> 
> I'm a little confused by the use of the term "others" here and in the
> comment below.  Can you be more specific what other software beyond the
> graphics driver is evaluating the size or location of stolen memory?
> 
> > Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=99028
> >          https://bugs.freedesktop.org/show_bug.cgi?id=99025
> >
> > Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> > Tested-by: Terrence Xu <terrence.xu@intel.com>
> > ---
> >  hw/vfio/pci-quirks.c | 63
> ++++++++++++++++++++++++++++++----------------------
> >  1 file changed, 36 insertions(+), 27 deletions(-)
> >
> > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> > index 1e97bc4..015d0c2 100644
> > --- a/hw/vfio/pci-quirks.c
> > +++ b/hw/vfio/pci-quirks.c
> > @@ -1364,14 +1364,43 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >      uint32_t gmch;
> >      uint16_t cmd_orig, cmd;
> >
> > +    /* This must be an Intel VGA device. */
> > +    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> > +        !vfio_is_vga(vdev) || nr != 4) {
> > +        return;
> > +    }
> > +
> >      /*
> > -     * This must be an Intel VGA device at address 00:02.0 for us to even
> > -     * consider enabling legacy mode.  The vBIOS has dependencies on
> the
> > -     * PCI bus address.
> > +     * IGD is not a standard, they like to change their specs often.  We
> > +     * only attempt to support back to SandBridge and we hope that
> newer
> > +     * devices maintain compatibility with generation 8.
> >       */
> > -    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> > -        !vfio_is_vga(vdev) || nr != 4 ||
> > -        &vdev->pdev !=
> pci_find_device(pci_device_root_bus(&vdev->pdev),
> > +    gen = igd_gen(vdev);
> > +    if (gen != 6 && gen != 8) {
> > +        error_report("IGD device %s is unsupported in legacy mode, "
> > +                     "try SandyBridge or newer",
> vdev->vbasedev.name);
> 
> This is a little bit misleading now since this is no longer exclusively
> a legacy mode path, a user trying to use UPT mode might disregard this
> as noise.  Perhaps...
> 
>     error_report("IGD device %s is unsupported by IGD quirks, "
>                  "try SandyBridge or newer", vdev->vbasedev.name);
> 
> 
> > +        return;
> > +    }
> > +    /*
> > +     * If this isn't at address 00:02.0, bios won't reserv stolen
> 
> s/reserv/reserve/
> 
> > +     * memory in E820, then others could use stolen memory. If guest
> > +     * graphic driver still use stolen memory, system maybe hang.
> > +     * so we set stolen memory size to 0 and guest graphic driver won't
> > +     * use stolen memory.
> 
> Based on my understanding of the bug, I might suggest:
> 
>   Regardless of running in UPT or legacy mode, the guest graphics
>   driver may attempt to use stolen memory, however only legacy mode has
>   BIOS support for reserving stolen memory in the guest VM.  Emulate
>   the GMCH register in all cases and zero out the stolen memory size
>   here.  Legacy mode may request allocation and re-write this below.
> 
> > +     */
> > +    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> > +    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> > +
> > +    /* GMCH is read-only, emulated */
> > +    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> > +    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> > +    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> > +
> > +    /*
> > +     * This must be at address 00:02.0 for us to even onsider enabling
> > +     * legacy mode.  The vBIOS has dependencies on the PCI bus address.
> > +     */
> > +    if (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
> >                                         0, PCI_DEVFN(0x2, 0))) {
> >          return;
> >      }
> > @@ -1391,18 +1420,6 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >      }
> >
> >      /*
> > -     * IGD is not a standard, they like to change their specs often.  We
> > -     * only attempt to support back to SandBridge and we hope that newer
> > -     * devices maintain compatibility with generation 8.
> > -     */
> > -    gen = igd_gen(vdev);
> > -    if (gen != 6 && gen != 8) {
> > -        error_report("IGD device %s is unsupported in legacy mode, "
> > -                     "try SandyBridge or newer", vdev->vbasedev.name);
> > -        return;
> > -    }
> > -
> > -    /*
> >       * Most of what we're doing here is to enable the ROM to run, so if
> >       * there's no ROM, there's no point in setting up this quirk.
> >       * NB. We only seem to get BIOS ROMs, so a UEFI VM would need
> CSM support.
> > @@ -1457,8 +1474,6 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >          goto out;
> >      }
> >
> > -    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> > -
> >      /*
> >       * If IGD VGA Disable is clear (expected) and VGA is not already
> enabled,
> >       * try to enable it.  Probably shouldn't be using legacy mode without
> VGA,
> > @@ -1526,12 +1541,11 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >       * when IVD (IGD VGA Disable) is clear, but the claim is that it's
> unused,
> >       * so let's not waste VM memory for it.
> >       */
> > -    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> > -
> >      if (vdev->igd_gms) {
> >          if (vdev->igd_gms <= 0x10) {
> >              gms_mb = vdev->igd_gms * 32;
> >              gmch |= vdev->igd_gms << (gen < 8 ? 3 : 8);
> > +            pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> >          } else {
> >              error_report("Unsupported IGD GMS value 0x%x",
> vdev->igd_gms);
> >              vdev->igd_gms = 0;
> > @@ -1551,11 +1565,6 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >      fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
> >                      bdsm_size, sizeof(*bdsm_size));
> >
> > -    /* GMCH is read-only, emulated */
> > -    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> > -    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> > -    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> > -
> >      /* BDSM is read-write, emulated.  The BIOS needs to be able to write
> it */
> >      pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
> >      pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO
  2017-02-20 11:42 Xiong Zhang
@ 2017-02-20 20:15 ` Alex Williamson
  2017-02-21  4:48   ` Zhang, Xiong Y
  2017-02-21  5:14   ` Zhang, Xiong Y
  0 siblings, 2 replies; 6+ messages in thread
From: Alex Williamson @ 2017-02-20 20:15 UTC (permalink / raw)
  To: Xiong Zhang; +Cc: qemu-devel, jike.song, zhiyuan.lv, zhi.a.wang

On Mon, 20 Feb 2017 19:42:54 +0800
Xiong Zhang <xiong.y.zhang@intel.com> wrote:

> From: XiongZhang <xiong.y.zhang@intel.com>
> 
> If IGD isn't assigned at 00:02.0 in UPT and host bios enable stolen
> memory, seabios won't reseave stolen memory in E820 for guest. Then
> both Intel graphic driver and others in guest could use stolen
> memory, this will generate system hang. So we should disable stolen
> memory in this case.

Wasn't the intent of UPT mode that it removed all of the BIOS and
chipset dependencies of IGD such that it could be assigned as just
another PCI device?  Does this mean that the drivers fail to meet that
promise by evaluating the size and location of stolen memory as
programmed on the physical device even in UPT mode?

I'm a little confused by the use of the term "others" here and in the
comment below.  Can you be more specific what other software beyond the
graphics driver is evaluating the size or location of stolen memory?
 
> Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=99028
>          https://bugs.freedesktop.org/show_bug.cgi?id=99025
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> Tested-by: Terrence Xu <terrence.xu@intel.com>
> ---
>  hw/vfio/pci-quirks.c | 63 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 1e97bc4..015d0c2 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1364,14 +1364,43 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>      uint32_t gmch;
>      uint16_t cmd_orig, cmd;
>  
> +    /* This must be an Intel VGA device. */
> +    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> +        !vfio_is_vga(vdev) || nr != 4) {
> +        return;
> +    }
> +
>      /*
> -     * This must be an Intel VGA device at address 00:02.0 for us to even
> -     * consider enabling legacy mode.  The vBIOS has dependencies on the
> -     * PCI bus address.
> +     * IGD is not a standard, they like to change their specs often.  We
> +     * only attempt to support back to SandBridge and we hope that newer
> +     * devices maintain compatibility with generation 8.
>       */
> -    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> -        !vfio_is_vga(vdev) || nr != 4 ||
> -        &vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
> +    gen = igd_gen(vdev);
> +    if (gen != 6 && gen != 8) {
> +        error_report("IGD device %s is unsupported in legacy mode, "
> +                     "try SandyBridge or newer", vdev->vbasedev.name);

This is a little bit misleading now since this is no longer exclusively
a legacy mode path, a user trying to use UPT mode might disregard this
as noise.  Perhaps...

    error_report("IGD device %s is unsupported by IGD quirks, "
                 "try SandyBridge or newer", vdev->vbasedev.name);


> +        return;
> +    }
> +    /*
> +     * If this isn't at address 00:02.0, bios won't reserv stolen

s/reserv/reserve/

> +     * memory in E820, then others could use stolen memory. If guest
> +     * graphic driver still use stolen memory, system maybe hang.
> +     * so we set stolen memory size to 0 and guest graphic driver won't
> +     * use stolen memory.

Based on my understanding of the bug, I might suggest:

  Regardless of running in UPT or legacy mode, the guest graphics
  driver may attempt to use stolen memory, however only legacy mode has
  BIOS support for reserving stolen memory in the guest VM.  Emulate
  the GMCH register in all cases and zero out the stolen memory size
  here.  Legacy mode may request allocation and re-write this below.

> +     */
> +    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> +    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> +
> +    /* GMCH is read-only, emulated */
> +    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> +    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> +    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> +
> +    /*
> +     * This must be at address 00:02.0 for us to even onsider enabling
> +     * legacy mode.  The vBIOS has dependencies on the PCI bus address.
> +     */
> +    if (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
>                                         0, PCI_DEVFN(0x2, 0))) {
>          return;
>      }
> @@ -1391,18 +1420,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>      }
>  
>      /*
> -     * IGD is not a standard, they like to change their specs often.  We
> -     * only attempt to support back to SandBridge and we hope that newer
> -     * devices maintain compatibility with generation 8.
> -     */
> -    gen = igd_gen(vdev);
> -    if (gen != 6 && gen != 8) {
> -        error_report("IGD device %s is unsupported in legacy mode, "
> -                     "try SandyBridge or newer", vdev->vbasedev.name);
> -        return;
> -    }
> -
> -    /*
>       * Most of what we're doing here is to enable the ROM to run, so if
>       * there's no ROM, there's no point in setting up this quirk.
>       * NB. We only seem to get BIOS ROMs, so a UEFI VM would need CSM support.
> @@ -1457,8 +1474,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>          goto out;
>      }
>  
> -    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> -
>      /*
>       * If IGD VGA Disable is clear (expected) and VGA is not already enabled,
>       * try to enable it.  Probably shouldn't be using legacy mode without VGA,
> @@ -1526,12 +1541,11 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       * when IVD (IGD VGA Disable) is clear, but the claim is that it's unused,
>       * so let's not waste VM memory for it.
>       */
> -    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> -
>      if (vdev->igd_gms) {
>          if (vdev->igd_gms <= 0x10) {
>              gms_mb = vdev->igd_gms * 32;
>              gmch |= vdev->igd_gms << (gen < 8 ? 3 : 8);
> +            pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
>          } else {
>              error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);
>              vdev->igd_gms = 0;
> @@ -1551,11 +1565,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>      fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
>                      bdsm_size, sizeof(*bdsm_size));
>  
> -    /* GMCH is read-only, emulated */
> -    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> -    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> -    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> -
>      /* BDSM is read-write, emulated.  The BIOS needs to be able to write it */
>      pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
>      pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO
@ 2017-02-20 11:42 Xiong Zhang
  2017-02-20 20:15 ` Alex Williamson
  0 siblings, 1 reply; 6+ messages in thread
From: Xiong Zhang @ 2017-02-20 11:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: jike.song, zhi.a.wang, zhiyuan.lv, XiongZhang

From: XiongZhang <xiong.y.zhang@intel.com>

If IGD isn't assigned at 00:02.0 in UPT and host bios enable stolen
memory, seabios won't reseave stolen memory in E820 for guest. Then
both Intel graphic driver and others in guest could use stolen
memory, this will generate system hang. So we should disable stolen
memory in this case.

Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=99028
         https://bugs.freedesktop.org/show_bug.cgi?id=99025

Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
Tested-by: Terrence Xu <terrence.xu@intel.com>
---
 hw/vfio/pci-quirks.c | 63 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 1e97bc4..015d0c2 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1364,14 +1364,43 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     uint32_t gmch;
     uint16_t cmd_orig, cmd;
 
+    /* This must be an Intel VGA device. */
+    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
+        !vfio_is_vga(vdev) || nr != 4) {
+        return;
+    }
+
     /*
-     * This must be an Intel VGA device at address 00:02.0 for us to even
-     * consider enabling legacy mode.  The vBIOS has dependencies on the
-     * PCI bus address.
+     * IGD is not a standard, they like to change their specs often.  We
+     * only attempt to support back to SandBridge and we hope that newer
+     * devices maintain compatibility with generation 8.
      */
-    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
-        !vfio_is_vga(vdev) || nr != 4 ||
-        &vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
+    gen = igd_gen(vdev);
+    if (gen != 6 && gen != 8) {
+        error_report("IGD device %s is unsupported in legacy mode, "
+                     "try SandyBridge or newer", vdev->vbasedev.name);
+        return;
+    }
+    /*
+     * If this isn't at address 00:02.0, bios won't reserv stolen
+     * memory in E820, then others could use stolen memory. If guest
+     * graphic driver still use stolen memory, system maybe hang.
+     * so we set stolen memory size to 0 and guest graphic driver won't
+     * use stolen memory.
+     */
+    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
+    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
+
+    /* GMCH is read-only, emulated */
+    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
+    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
+    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
+
+    /*
+     * This must be at address 00:02.0 for us to even onsider enabling
+     * legacy mode.  The vBIOS has dependencies on the PCI bus address.
+     */
+    if (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
                                        0, PCI_DEVFN(0x2, 0))) {
         return;
     }
@@ -1391,18 +1420,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     }
 
     /*
-     * IGD is not a standard, they like to change their specs often.  We
-     * only attempt to support back to SandBridge and we hope that newer
-     * devices maintain compatibility with generation 8.
-     */
-    gen = igd_gen(vdev);
-    if (gen != 6 && gen != 8) {
-        error_report("IGD device %s is unsupported in legacy mode, "
-                     "try SandyBridge or newer", vdev->vbasedev.name);
-        return;
-    }
-
-    /*
      * Most of what we're doing here is to enable the ROM to run, so if
      * there's no ROM, there's no point in setting up this quirk.
      * NB. We only seem to get BIOS ROMs, so a UEFI VM would need CSM support.
@@ -1457,8 +1474,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
         goto out;
     }
 
-    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
-
     /*
      * If IGD VGA Disable is clear (expected) and VGA is not already enabled,
      * try to enable it.  Probably shouldn't be using legacy mode without VGA,
@@ -1526,12 +1541,11 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
      * when IVD (IGD VGA Disable) is clear, but the claim is that it's unused,
      * so let's not waste VM memory for it.
      */
-    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
-
     if (vdev->igd_gms) {
         if (vdev->igd_gms <= 0x10) {
             gms_mb = vdev->igd_gms * 32;
             gmch |= vdev->igd_gms << (gen < 8 ? 3 : 8);
+            pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
         } else {
             error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);
             vdev->igd_gms = 0;
@@ -1551,11 +1565,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
                     bdsm_size, sizeof(*bdsm_size));
 
-    /* GMCH is read-only, emulated */
-    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
-    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
-    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
-
     /* BDSM is read-write, emulated.  The BIOS needs to be able to write it */
     pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
     pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-21  5:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-20 11:17 [Qemu-devel] [PATCH] vfio/pci-quirks.c: Disable stolen memory for igd VFIO Xiong Zhang
2017-02-20  3:20 ` no-reply
2017-02-20 11:42 Xiong Zhang
2017-02-20 20:15 ` Alex Williamson
2017-02-21  4:48   ` Zhang, Xiong Y
2017-02-21  5:14   ` Zhang, Xiong Y

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.