diff mbox

[V4] drm/i915: Enhanced disable access to stolen memory as a guest

Message ID 1491358106-26329-1-git-send-email-xiong.y.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang, Xiong Y April 5, 2017, 2:08 a.m. UTC
Stolen memory is in RMRR and has identity mapping in iommu, so
gt could access stolen memory in host OS. While RMRR isn't supported
by kvm, both EPT and guest iommu domain lack of maaping for stolen memory,
so both vcpu and gt couldn't access stolen memory in guest.

commit "04a68a3 drm/i915/gvt: Disable access to stolen memory as a guest"
isn't enough in IGD passthrough environment where vgt code doesn't run. This
patch detects i915 running as a guest through the existence of emulated ISA
bridge.

v2:GVT-g may run in non qemu (Zhenyu)
v3:Make commit message clear (Daniel)
v4:Fix typo	

References: c875d2c iommu/vt-d: Exclude devices using RMRRs from IOMMU API domains
Link: https://access.redhat.com/sites/default/files/attachments/rmrr-wp1.pdf
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99028

Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_drv.c        | 1 +
 drivers/gpu/drm/i915/i915_drv.h        | 1 +
 drivers/gpu/drm/i915/i915_gem_stolen.c | 4 ++--
 3 files changed, 4 insertions(+), 2 deletions(-)

Comments

Daniel Vetter April 5, 2017, 6:50 a.m. UTC | #1
On Wed, Apr 05, 2017 at 10:08:26AM +0800, Xiong Zhang wrote:
> Stolen memory is in RMRR and has identity mapping in iommu, so
> gt could access stolen memory in host OS. While RMRR isn't supported
> by kvm, both EPT and guest iommu domain lack of maaping for stolen memory,
> so both vcpu and gt couldn't access stolen memory in guest.
> 
> commit "04a68a3 drm/i915/gvt: Disable access to stolen memory as a guest"
> isn't enough in IGD passthrough environment where vgt code doesn't run. This
> patch detects i915 running as a guest through the existence of emulated ISA
> bridge.
> 
> v2:GVT-g may run in non qemu (Zhenyu)
> v3:Make commit message clear (Daniel)
> v4:Fix typo	
> 
> References: c875d2c iommu/vt-d: Exclude devices using RMRRs from IOMMU API domains
> Link: https://access.redhat.com/sites/default/files/attachments/rmrr-wp1.pdf
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99028
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: stable@vger.kernel.org

Yeah, commit message is now much improved.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_drv.c        | 1 +
>  drivers/gpu/drm/i915/i915_drv.h        | 1 +
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 4 ++--
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 03d9e45..8b807a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -223,6 +223,7 @@ static void intel_detect_pch(struct drm_i915_private *dev_priv)
>  					    PCI_SUBVENDOR_ID_REDHAT_QUMRANET &&
>  				    pch->subsystem_device ==
>  					    PCI_SUBDEVICE_ID_QEMU)) {
> +				dev_priv->run_on_qemu = true;
>  				dev_priv->pch_type =
>  					intel_virt_detect_pch(dev_priv);
>  			} else
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a5947a4..ad95c87 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2145,6 +2145,7 @@ struct drm_i915_private {
>  	struct intel_uncore uncore;
>  
>  	struct i915_virtual_gpu vgpu;
> +	bool run_on_qemu;
>  
>  	struct intel_gvt *gvt;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index f3abdc2..6a011b0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -409,8 +409,8 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>  
>  	mutex_init(&dev_priv->mm.stolen_lock);
>  
> -	if (intel_vgpu_active(dev_priv)) {
> -		DRM_INFO("iGVT-g active, disabling use of stolen memory\n");
> +	if (dev_priv->run_on_qemu || intel_vgpu_active(dev_priv)) {
> +		DRM_INFO("Running in guest, disabling use of stolen memory\n");
>  		return 0;
>  	}
>  
> -- 
> 1.9.1
>
Jani Nikula April 5, 2017, 7:44 a.m. UTC | #2
On Wed, 05 Apr 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Apr 05, 2017 at 10:08:26AM +0800, Xiong Zhang wrote:
>> Stolen memory is in RMRR and has identity mapping in iommu, so
>> gt could access stolen memory in host OS. While RMRR isn't supported
>> by kvm, both EPT and guest iommu domain lack of maaping for stolen memory,
>> so both vcpu and gt couldn't access stolen memory in guest.
>> 
>> commit "04a68a3 drm/i915/gvt: Disable access to stolen memory as a guest"
>> isn't enough in IGD passthrough environment where vgt code doesn't run. This
>> patch detects i915 running as a guest through the existence of emulated ISA
>> bridge.
>> 
>> v2:GVT-g may run in non qemu (Zhenyu)
>> v3:Make commit message clear (Daniel)
>> v4:Fix typo	
>> 
>> References: c875d2c iommu/vt-d: Exclude devices using RMRRs from IOMMU API domains

What does it mean to References: a commit? The message text doesn't say
anything about this commit.

Does this patch fix commit 04a68a3 or c875d2c? If so, there should be a
Fixes: tag.

The canonical format for commit references is

04a68a35ce6d ("drm/i915/gvt: Disable access to stolen memory as a guest")
c875d2c1b808 ("iommu/vt-d: Exclude devices using RMRRs from IOMMU API domains")

In particular, note that 12 digits is preferred for sha1 references to
avoid ambiguity in the kernel git repo.

>> Link: https://access.redhat.com/sites/default/files/attachments/rmrr-wp1.pdf

Please use References: for stuff like this. Link: is reserved for the
backlink to the patch and discussion.

>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99028
>> 
>> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
>> Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: stable@vger.kernel.org

With a Fixes: tag it would be possible to decide which stable kernels to
backport to.

> Yeah, commit message is now much improved.

I'm sorry, but from the fixes backports POV it still lacks clarity. I
wouldn't know if and where this should be backported.


BR,
Jani.


>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c        | 1 +
>>  drivers/gpu/drm/i915/i915_drv.h        | 1 +
>>  drivers/gpu/drm/i915/i915_gem_stolen.c | 4 ++--
>>  3 files changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 03d9e45..8b807a9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -223,6 +223,7 @@ static void intel_detect_pch(struct drm_i915_private *dev_priv)
>>  					    PCI_SUBVENDOR_ID_REDHAT_QUMRANET &&
>>  				    pch->subsystem_device ==
>>  					    PCI_SUBDEVICE_ID_QEMU)) {
>> +				dev_priv->run_on_qemu = true;
>>  				dev_priv->pch_type =
>>  					intel_virt_detect_pch(dev_priv);
>>  			} else
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index a5947a4..ad95c87 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2145,6 +2145,7 @@ struct drm_i915_private {
>>  	struct intel_uncore uncore;
>>  
>>  	struct i915_virtual_gpu vgpu;
>> +	bool run_on_qemu;
>>  
>>  	struct intel_gvt *gvt;
>>  
>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> index f3abdc2..6a011b0 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -409,8 +409,8 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>>  
>>  	mutex_init(&dev_priv->mm.stolen_lock);
>>  
>> -	if (intel_vgpu_active(dev_priv)) {
>> -		DRM_INFO("iGVT-g active, disabling use of stolen memory\n");
>> +	if (dev_priv->run_on_qemu || intel_vgpu_active(dev_priv)) {
>> +		DRM_INFO("Running in guest, disabling use of stolen memory\n");
>>  		return 0;
>>  	}
>>  
>> -- 
>> 1.9.1
>>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 03d9e45..8b807a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -223,6 +223,7 @@  static void intel_detect_pch(struct drm_i915_private *dev_priv)
 					    PCI_SUBVENDOR_ID_REDHAT_QUMRANET &&
 				    pch->subsystem_device ==
 					    PCI_SUBDEVICE_ID_QEMU)) {
+				dev_priv->run_on_qemu = true;
 				dev_priv->pch_type =
 					intel_virt_detect_pch(dev_priv);
 			} else
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a5947a4..ad95c87 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2145,6 +2145,7 @@  struct drm_i915_private {
 	struct intel_uncore uncore;
 
 	struct i915_virtual_gpu vgpu;
+	bool run_on_qemu;
 
 	struct intel_gvt *gvt;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index f3abdc2..6a011b0 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -409,8 +409,8 @@  int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 
 	mutex_init(&dev_priv->mm.stolen_lock);
 
-	if (intel_vgpu_active(dev_priv)) {
-		DRM_INFO("iGVT-g active, disabling use of stolen memory\n");
+	if (dev_priv->run_on_qemu || intel_vgpu_active(dev_priv)) {
+		DRM_INFO("Running in guest, disabling use of stolen memory\n");
 		return 0;
 	}