diff mbox

vfio/pci-quirks: Set non-zero GMS memory size for IGD

Message ID 20170724120729.24350-1-dmitry@daynix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dmitry Fleytman July 24, 2017, 12:07 p.m. UTC
There is a claim that GMS memory is unused however
Intel Windows 10 drivers starting from V.4534 (10/7/2016)
allocate extra ~4G memory when GMS size set to 0.

This patch fixes this issue by seting IGD GMS memory
size to minimum by changing default value of x-igd-gms
device parameter.

Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
---
 hw/vfio/pci-quirks.c | 13 +++++++++----
 hw/vfio/pci.c        |  2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

Comments

Sameeh Jubran July 24, 2017, 12:59 p.m. UTC | #1
Just noticed a small typo, other than LGTM +1 :)
On Mon, Jul 24, 2017 at 3:07 PM, Dmitry Fleytman <dmitry@daynix.com> wrote:

> There is a claim that GMS memory is unused however
> Intel Windows 10 drivers starting from V.4534 (10/7/2016)
> allocate extra ~4G memory when GMS size set to 0.
>
> This patch fixes this issue by seting IGD GMS memory
>
* setting

> size to minimum by changing default value of x-igd-gms
> device parameter.
>
> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
> ---
>  hw/vfio/pci-quirks.c | 13 +++++++++----
>  hw/vfio/pci.c        |  2 +-
>  2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 349085e..197cb90 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1527,10 +1527,15 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice
> *vdev, int nr)
>      }
>
>      /*
> -     * Assume we have no GMS memory, but allow it to be overrided by
> device
> -     * option (experimental).  The spec doesn't actually allow zero GMS
> when
> -     * when IVD (IGD VGA Disable) is clear, but the claim is that it's
> unused,
> -     * so let's not waste VM memory for it.
> +     * There is a claim that GMS memory is unused and we want to waste
> for it
> +     * as less VM memory as possible, however Intel Windows 10 drivers
> starting
> +     * from V.4534 (10/7/2016) allocate extra ~4G memory when GMS size
> set to 0.
> +     * The spec as well doesn't actually allow zero GMS when IVD
> +     * (IGD VGA Disable) is clear.
> +     *
> +     * Therefore we set GMS memory size to minimal by default via device
> +     * option x-igd-gms (experimental) and allow further tweaking of this
> +     * parameter.
>       */
>      gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index d4051cb..bda07b7 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2982,7 +2982,7 @@ static Property vfio_pci_dev_properties[] = {
>                         sub_vendor_id, PCI_ANY_ID),
>      DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
>                         sub_device_id, PCI_ANY_ID),
> -    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
> +    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 1),
>      /*
>       * TODO - support passed fds... is this necessary?
>       * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
> --
> 2.9.4
>
>
>
Alex Williamson July 25, 2017, 3:31 p.m. UTC | #2
[cc +Zhang, Xiong Y]


Intel, any comments?


On Mon, 24 Jul 2017 15:07:29 +0300
Dmitry Fleytman <dmitry@daynix.com> wrote:

> There is a claim that GMS memory is unused however
> Intel Windows 10 drivers starting from V.4534 (10/7/2016)
> allocate extra ~4G memory when GMS size set to 0.
> 
> This patch fixes this issue by seting IGD GMS memory
> size to minimum by changing default value of x-igd-gms
> device parameter.
> 
> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
> ---
>  hw/vfio/pci-quirks.c | 13 +++++++++----
>  hw/vfio/pci.c        |  2 +-
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 349085e..197cb90 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1527,10 +1527,15 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>      }
>  
>      /*
> -     * Assume we have no GMS memory, but allow it to be overrided by device
> -     * option (experimental).  The spec doesn't actually allow zero GMS when
> -     * when IVD (IGD VGA Disable) is clear, but the claim is that it's unused,
> -     * so let's not waste VM memory for it.
> +     * There is a claim that GMS memory is unused and we want to waste for it
> +     * as less VM memory as possible, however Intel Windows 10 drivers starting
> +     * from V.4534 (10/7/2016) allocate extra ~4G memory when GMS size set to 0.
> +     * The spec as well doesn't actually allow zero GMS when IVD
> +     * (IGD VGA Disable) is clear.
> +     *
> +     * Therefore we set GMS memory size to minimal by default via device
> +     * option x-igd-gms (experimental) and allow further tweaking of this
> +     * parameter.
>       */
>      gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
>  
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index d4051cb..bda07b7 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2982,7 +2982,7 @@ static Property vfio_pci_dev_properties[] = {
>                         sub_vendor_id, PCI_ANY_ID),
>      DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
>                         sub_device_id, PCI_ANY_ID),
> -    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
> +    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 1),
>      /*
>       * TODO - support passed fds... is this necessary?
>       * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
Zhang, Xiong Y July 26, 2017, 5:22 a.m. UTC | #3
Sorry, we indeed found Intel windows guest graphic driver couldn't be bind when GMS memory size is zero. And we have fixed it and the next intel windows driver release will contain this fix.
So currently please use x-igd-gms in legacy mode to work around this. 

BTW, how did you know window driver allocate extra ~4G memory when GMS size set to 0 ?

thanks
> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Tuesday, July 25, 2017 11:32 PM
> To: Dmitry Fleytman <dmitry@daynix.com>
> Cc: qemu-devel@nongnu.org; Zhang, Xiong Y <xiong.y.zhang@intel.com>
> Subject: Re: [PATCH] vfio/pci-quirks: Set non-zero GMS memory size for IGD
> 
> [cc +Zhang, Xiong Y]
> 
> 
> Intel, any comments?
> 
> 
> On Mon, 24 Jul 2017 15:07:29 +0300
> Dmitry Fleytman <dmitry@daynix.com> wrote:
> 
> > There is a claim that GMS memory is unused however
> > Intel Windows 10 drivers starting from V.4534 (10/7/2016)
> > allocate extra ~4G memory when GMS size set to 0.
> >
> > This patch fixes this issue by seting IGD GMS memory
> > size to minimum by changing default value of x-igd-gms
> > device parameter.
> >
> > Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
> > ---
> >  hw/vfio/pci-quirks.c | 13 +++++++++----
> >  hw/vfio/pci.c        |  2 +-
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> > index 349085e..197cb90 100644
> > --- a/hw/vfio/pci-quirks.c
> > +++ b/hw/vfio/pci-quirks.c
> > @@ -1527,10 +1527,15 @@ static void
> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
> >      }
> >
> >      /*
> > -     * Assume we have no GMS memory, but allow it to be overrided by
> device
> > -     * option (experimental).  The spec doesn't actually allow zero GMS
> when
> > -     * when IVD (IGD VGA Disable) is clear, but the claim is that it's unused,
> > -     * so let's not waste VM memory for it.
> > +     * There is a claim that GMS memory is unused and we want to waste
> for it
> > +     * as less VM memory as possible, however Intel Windows 10 drivers
> starting
> > +     * from V.4534 (10/7/2016) allocate extra ~4G memory when GMS
> size set to 0.
> > +     * The spec as well doesn't actually allow zero GMS when IVD
> > +     * (IGD VGA Disable) is clear.
> > +     *
> > +     * Therefore we set GMS memory size to minimal by default via device
> > +     * option x-igd-gms (experimental) and allow further tweaking of this
> > +     * parameter.
> >       */
> >      gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index d4051cb..bda07b7 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -2982,7 +2982,7 @@ static Property vfio_pci_dev_properties[] = {
> >                         sub_vendor_id, PCI_ANY_ID),
> >      DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
> >                         sub_device_id, PCI_ANY_ID),
> > -    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
> > +    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 1),
> >      /*
> >       * TODO - support passed fds... is this necessary?
> >       * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
Dmitry Fleytman July 26, 2017, 9:40 a.m. UTC | #4
> On 26 Jul 2017, at 08:22 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com> wrote:
> 
> Sorry, we indeed found Intel windows guest graphic driver couldn't be bind when GMS memory size is zero. And we have fixed it and the next intel windows driver release will contain this fix.
> So currently please use x-igd-gms in legacy mode to work around this. 
> 
> BTW, how did you know window driver allocate extra ~4G memory when GMS size set to 0 ?

We noticed that with new IGD driver memory usage on VMs raised by around 4G.

Then we examined memory allocations with poolmon (https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/poolmon)
and saw that around 4G of memory is allocated with IGD driver pool tag.

Additionally we noticed that on VMs with less than 5G of memory IGD driver does not load and system does fallback to standard VGA driver.

> 
> thanks
>> -----Original Message-----
>> From: Alex Williamson [mailto:alex.williamson@redhat.com]
>> Sent: Tuesday, July 25, 2017 11:32 PM
>> To: Dmitry Fleytman <dmitry@daynix.com>
>> Cc: qemu-devel@nongnu.org; Zhang, Xiong Y <xiong.y.zhang@intel.com>
>> Subject: Re: [PATCH] vfio/pci-quirks: Set non-zero GMS memory size for IGD
>> 
>> [cc +Zhang, Xiong Y]
>> 
>> 
>> Intel, any comments?
>> 
>> 
>> On Mon, 24 Jul 2017 15:07:29 +0300
>> Dmitry Fleytman <dmitry@daynix.com> wrote:
>> 
>>> There is a claim that GMS memory is unused however
>>> Intel Windows 10 drivers starting from V.4534 (10/7/2016)
>>> allocate extra ~4G memory when GMS size set to 0.
>>> 
>>> This patch fixes this issue by seting IGD GMS memory
>>> size to minimum by changing default value of x-igd-gms
>>> device parameter.
>>> 
>>> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
>>> ---
>>> hw/vfio/pci-quirks.c | 13 +++++++++----
>>> hw/vfio/pci.c        |  2 +-
>>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>>> index 349085e..197cb90 100644
>>> --- a/hw/vfio/pci-quirks.c
>>> +++ b/hw/vfio/pci-quirks.c
>>> @@ -1527,10 +1527,15 @@ static void
>> vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>>>     }
>>> 
>>>     /*
>>> -     * Assume we have no GMS memory, but allow it to be overrided by
>> device
>>> -     * option (experimental).  The spec doesn't actually allow zero GMS
>> when
>>> -     * when IVD (IGD VGA Disable) is clear, but the claim is that it's unused,
>>> -     * so let's not waste VM memory for it.
>>> +     * There is a claim that GMS memory is unused and we want to waste
>> for it
>>> +     * as less VM memory as possible, however Intel Windows 10 drivers
>> starting
>>> +     * from V.4534 (10/7/2016) allocate extra ~4G memory when GMS
>> size set to 0.
>>> +     * The spec as well doesn't actually allow zero GMS when IVD
>>> +     * (IGD VGA Disable) is clear.
>>> +     *
>>> +     * Therefore we set GMS memory size to minimal by default via device
>>> +     * option x-igd-gms (experimental) and allow further tweaking of this
>>> +     * parameter.
>>>      */
>>>     gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
>>> 
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index d4051cb..bda07b7 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -2982,7 +2982,7 @@ static Property vfio_pci_dev_properties[] = {
>>>                        sub_vendor_id, PCI_ANY_ID),
>>>     DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
>>>                        sub_device_id, PCI_ANY_ID),
>>> -    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
>>> +    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 1),
>>>     /*
>>>      * TODO - support passed fds... is this necessary?
>>>      * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
>
Zhang, Xiong Y July 28, 2017, 4:51 a.m. UTC | #5
> > On 26 Jul 2017, at 08:22 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com>
> wrote:
> >
> > Sorry, we indeed found Intel windows guest graphic driver couldn't be bind
> when GMS memory size is zero. And we have fixed it and the next intel
> windows driver release will contain this fix.
> > So currently please use x-igd-gms in legacy mode to work around this.
> >
> > BTW, how did you know window driver allocate extra ~4G memory when
> GMS size set to 0 ?
> 
> We noticed that with new IGD driver memory usage on VMs raised by around
> 4G.
> 
> Then we examined memory allocations with poolmon
> (https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/poolm
> on)
> and saw that around 4G of memory is allocated with IGD driver pool tag.
> Additionally we noticed that on VMs with less than 5G of memory IGD driver
> does not load and system does fallback to standard VGA driver.
> 
[Zhang, Xiong Y]  I tried our internal driver and didn't found the above two issues, please wait for the next intel graphic driver release.

thanks
Dmitry Fleytman July 28, 2017, 5:22 a.m. UTC | #6
> On 28 Jul 2017, at 07:51 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com> wrote:
> 
>>> On 26 Jul 2017, at 08:22 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com>
>> wrote:
>>> 
>>> Sorry, we indeed found Intel windows guest graphic driver couldn't be bind
>> when GMS memory size is zero. And we have fixed it and the next intel
>> windows driver release will contain this fix.
>>> So currently please use x-igd-gms in legacy mode to work around this.
>>> 
>>> BTW, how did you know window driver allocate extra ~4G memory when
>> GMS size set to 0 ?
>> 
>> We noticed that with new IGD driver memory usage on VMs raised by around
>> 4G.
>> 
>> Then we examined memory allocations with poolmon
>> (https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/poolm
>> on)
>> and saw that around 4G of memory is allocated with IGD driver pool tag.
>> Additionally we noticed that on VMs with less than 5G of memory IGD driver
>> does not load and system does fallback to standard VGA driver.
>> 
> [Zhang, Xiong Y]  I tried our internal driver and didn't found the above two issues, please wait for the next intel graphic driver release.

Thanks!

> 
> thanks
Alex Williamson Aug. 1, 2017, 9:24 p.m. UTC | #7
On Fri, 28 Jul 2017 08:22:45 +0300
Dmitry Fleytman <dmitry@daynix.com> wrote:

> > On 28 Jul 2017, at 07:51 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com> wrote:
> >   
> >>> On 26 Jul 2017, at 08:22 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com>  
> >> wrote:  
> >>> 
> >>> Sorry, we indeed found Intel windows guest graphic driver couldn't be bind  
> >> when GMS memory size is zero. And we have fixed it and the next intel
> >> windows driver release will contain this fix.  
> >>> So currently please use x-igd-gms in legacy mode to work around this.
> >>> 
> >>> BTW, how did you know window driver allocate extra ~4G memory when  
> >> GMS size set to 0 ?
> >> 
> >> We noticed that with new IGD driver memory usage on VMs raised by around
> >> 4G.
> >> 
> >> Then we examined memory allocations with poolmon
> >> (https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/poolm
> >> on)
> >> and saw that around 4G of memory is allocated with IGD driver pool tag.
> >> Additionally we noticed that on VMs with less than 5G of memory IGD driver
> >> does not load and system does fallback to standard VGA driver.
> >>   
> > [Zhang, Xiong Y]  I tried our internal driver and didn't found the above two issues, please wait for the next intel graphic driver release.  
> 
> Thanks!

So to close on this, we should just be recommending that users
encountering this problem add the x-igd-gms=1 option to the device
until the next Intel graphics release.  I won't plan to push this
change.  Thanks all,

Alex
Dmitry Fleytman Aug. 2, 2017, 7:46 a.m. UTC | #8
> On 2 Aug 2017, at 24:24 AM, Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> On Fri, 28 Jul 2017 08:22:45 +0300
> Dmitry Fleytman <dmitry@daynix.com <mailto:dmitry@daynix.com>> wrote:
> 
>>> On 28 Jul 2017, at 07:51 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com> wrote:
>>> 
>>>>> On 26 Jul 2017, at 08:22 AM, Zhang, Xiong Y <xiong.y.zhang@intel.com>  
>>>> wrote:  
>>>>> 
>>>>> Sorry, we indeed found Intel windows guest graphic driver couldn't be bind  
>>>> when GMS memory size is zero. And we have fixed it and the next intel
>>>> windows driver release will contain this fix.  
>>>>> So currently please use x-igd-gms in legacy mode to work around this.
>>>>> 
>>>>> BTW, how did you know window driver allocate extra ~4G memory when  
>>>> GMS size set to 0 ?
>>>> 
>>>> We noticed that with new IGD driver memory usage on VMs raised by around
>>>> 4G.
>>>> 
>>>> Then we examined memory allocations with poolmon
>>>> (https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/poolm
>>>> on)
>>>> and saw that around 4G of memory is allocated with IGD driver pool tag.
>>>> Additionally we noticed that on VMs with less than 5G of memory IGD driver
>>>> does not load and system does fallback to standard VGA driver.
>>>> 
>>> [Zhang, Xiong Y]  I tried our internal driver and didn't found the above two issues, please wait for the next intel graphic driver release.  
>> 
>> Thanks!
> 
> So to close on this, we should just be recommending that users
> encountering this problem add the x-igd-gms=1 option to the device
> until the next Intel graphics release.  I won't plan to push this
> change.  Thanks all,

Hi Alex,

Agree, that makes sense.

Thanks you,
Dmitry

> 
> Alex
diff mbox

Patch

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 349085e..197cb90 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1527,10 +1527,15 @@  static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     }
 
     /*
-     * Assume we have no GMS memory, but allow it to be overrided by device
-     * option (experimental).  The spec doesn't actually allow zero GMS when
-     * when IVD (IGD VGA Disable) is clear, but the claim is that it's unused,
-     * so let's not waste VM memory for it.
+     * There is a claim that GMS memory is unused and we want to waste for it
+     * as less VM memory as possible, however Intel Windows 10 drivers starting
+     * from V.4534 (10/7/2016) allocate extra ~4G memory when GMS size set to 0.
+     * The spec as well doesn't actually allow zero GMS when IVD
+     * (IGD VGA Disable) is clear.
+     *
+     * Therefore we set GMS memory size to minimal by default via device
+     * option x-igd-gms (experimental) and allow further tweaking of this
+     * parameter.
      */
     gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d4051cb..bda07b7 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2982,7 +2982,7 @@  static Property vfio_pci_dev_properties[] = {
                        sub_vendor_id, PCI_ANY_ID),
     DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
                        sub_device_id, PCI_ANY_ID),
-    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
+    DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 1),
     /*
      * TODO - support passed fds... is this necessary?
      * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),