Re: [PATCH V6] drm/i915: Disable stolen memory when i915 runs in guest vm

From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: "Zhang, Xiong Y" <xiong.y.zhang@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"Vetter, Daniel" <daniel.vetter@intel.com>,
	"zhenyuw@linux.intel.com" <zhenyuw@linux.intel.com>,
	"jani.nikula@linux.intel.com" <jani.nikula@linux.intel.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	"Bloomfield, Jon" <jon.bloomfield@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"intel-gvt-dev@lists.freedesktop.org"
	<intel-gvt-dev@lists.freedesktop.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH V6] drm/i915: Disable stolen memory when i915 runs in guest vm
Date: Fri, 05 May 2017 12:14:30 +0300	[thread overview]
Message-ID: <1493975670.3551.16.camel@linux.intel.com> (raw)
In-Reply-To: <8082FF9BCB2B054996454E47167FF4EC1C4D19EB@SHSMSX104.ccr.corp.intel.com>

On to, 2017-04-27 at 05:54 +0000, Zhang, Xiong Y wrote:
> > 
> > Also, was fixing the IGD driver loading with zero stolen memory
> > considered instead? All this information should exist in the commit
> > message.
> [Zhang, Xiong Y] IGD and i915 driver read pci config register 0x50 to get 
> the size of stolen memory. When guest read this register, qemu could trap
> it and return one value to guest.
> So in order to  " fixing the IGD driver loading with zero stolen memory ",
> We have to modify both Qemu and IGD driver:
> 1) QEMU: trap read from pci cfg 0x50 register, then return zero to guest
> 2) IGD driver: when IGD driver see zero size of stolen memory, don't exit loading
> and continue.
> This doesn't give any benefit to i915, i915 will still disable stolen memory as i915
> see zero size stolen memory . So I prefer to disable stolen memory in i915 directly 
> and keep Qemu and IGD driver unchanged. 

If due to lack of code in QEMU, RMRR is not available. It'd sound to me
they should carry the change to report stolen as zero, because they're
in best position to track if that changes in future.

Also, if the IGD driver requires a special treatment where stolen is
reported to exist but is not functional, that logic should be fixed
where the flaw is. i915 driver is not the go-to for fixing any quirks
and lack of code related to virtualization.

The driver performs perfectly logically, if stolen is reported to
exist, it is expected to work, if not, it's not touched. Adding special
cases to satisfy conditions outside of i915 will make driver
maintenance a burden.

And, if all other options fail and we end up having to fix the issue in
i915, the fix should be robust, which this currently is not (see my
previous messages).

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation