[v4,04/14] drm/i915/gvt: Detect 64K gtt entry by IPS bit of PDE

Submitted by changbin.du@intel.com on May 4, 2018, 9:50 a.m.

Details

Message ID 1525427440-6255-5-git-send-email-changbin.du@intel.com
State New
Headers show
Series "drm/i915/gvt: Add huge gtt shadowing" ( rev: 4 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

changbin.du@intel.com May 4, 2018, 9:50 a.m.
From: Changbin Du <changbin.du@intel.com>

This change help us detect the real entry type per PSE and IPS setting.
For 64K entry, we also need to check reg GEN8_GAMW_ECO_DEV_RW_IA.

Signed-off-by: Changbin Du <changbin.du@intel.com>
---
 drivers/gpu/drm/i915/gvt/gtt.c | 68 +++++++++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/gvt/gtt.h |  2 ++
 2 files changed, 49 insertions(+), 21 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index cd2a227..7a80518 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -384,20 +384,7 @@  static void gen8_gtt_set_pfn(struct intel_gvt_gtt_entry *e, unsigned long pfn)
 
 static bool gen8_gtt_test_pse(struct intel_gvt_gtt_entry *e)
 {
-	/* Entry doesn't have PSE bit. */
-	if (get_pse_type(e->type) == GTT_TYPE_INVALID)
-		return false;
-
-	e->type = get_entry_type(e->type);
-	if (!(e->val64 & _PAGE_PSE))
-		return false;
-
-	/* We don't support 64K entry yet, will remove this later. */
-	if (get_pse_type(e->type) == GTT_TYPE_PPGTT_PTE_64K_ENTRY)
-		return false;
-
-	e->type = get_pse_type(e->type);
-	return true;
+	return !!(e->val64 & _PAGE_PSE);
 }
 
 static bool gen8_gtt_test_ips(struct intel_gvt_gtt_entry *e)
@@ -487,6 +474,27 @@  static struct intel_gvt_gtt_gma_ops gen8_gtt_gma_ops = {
 	.gma_to_pml4_index = gen8_gma_to_pml4_index,
 };
 
+/* Update entry type per pse and ips bit. */
+static void update_entry_type_for_real(struct intel_gvt_gtt_pte_ops *pte_ops,
+	struct intel_gvt_gtt_entry *entry, bool ips)
+{
+	switch (entry->type) {
+	case GTT_TYPE_PPGTT_PDE_ENTRY:
+	case GTT_TYPE_PPGTT_PDP_ENTRY:
+		if (pte_ops->test_pse(entry))
+			entry->type = get_pse_type(entry->type);
+		break;
+	case GTT_TYPE_PPGTT_PTE_4K_ENTRY:
+		if (ips)
+			entry->type = get_pse_type(entry->type);
+		break;
+	default:
+		GEM_BUG_ON(!gtt_type_is_entry(entry->type));
+	}
+
+	GEM_BUG_ON(entry->type == GTT_TYPE_INVALID);
+}
+
 /*
  * MM helpers.
  */
@@ -502,8 +510,7 @@  static void _ppgtt_get_root_entry(struct intel_vgpu_mm *mm,
 	pte_ops->get_entry(guest ? mm->ppgtt_mm.guest_pdps :
 			   mm->ppgtt_mm.shadow_pdps,
 			   entry, index, false, 0, mm->vgpu);
-
-	pte_ops->test_pse(entry);
+	update_entry_type_for_real(pte_ops, entry, false);
 }
 
 static inline void ppgtt_get_guest_root_entry(struct intel_vgpu_mm *mm,
@@ -608,7 +615,8 @@  static inline int ppgtt_spt_get_entry(
 	if (ret)
 		return ret;
 
-	ops->test_pse(e);
+	update_entry_type_for_real(ops, e, guest ?
+				   spt->guest_page.pde_ips : false);
 
 	gvt_vdbg_mm("read ppgtt entry, spt type %d, entry type %d, index %lu, value %llx\n",
 		    type, e->type, index, e->val64);
@@ -752,7 +760,8 @@  static inline struct intel_vgpu_ppgtt_spt *intel_vgpu_find_spt_by_mfn(
 static int reclaim_one_ppgtt_mm(struct intel_gvt *gvt);
 
 static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
-		struct intel_vgpu *vgpu, int type, unsigned long gfn)
+		struct intel_vgpu *vgpu, int type, unsigned long gfn,
+		bool guest_pde_ips)
 {
 	struct device *kdev = &vgpu->gvt->dev_priv->drm.pdev->dev;
 	struct intel_vgpu_ppgtt_spt *spt = NULL;
@@ -792,6 +801,7 @@  static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
 	 */
 	spt->guest_page.type = type;
 	spt->guest_page.gfn = gfn;
+	spt->guest_page.pde_ips = guest_pde_ips;
 
 	ret = intel_vgpu_register_page_track(vgpu, spt->guest_page.gfn,
 					ppgtt_write_protection_handler, spt);
@@ -934,6 +944,20 @@  static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt)
 	return ret;
 }
 
+static bool vgpu_ips_enabled(struct intel_vgpu *vgpu)
+{
+	if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) {
+		u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) &
+			GAMW_ECO_ENABLE_64K_IPS_FIELD;
+
+		return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD;
+	} else if (INTEL_GEN(vgpu->gvt->dev_priv) >= 10) {
+		/* 64K paging only controlled by IPS bit in PTE now. */
+		return true;
+	} else
+		return false;
+}
+
 static int ppgtt_populate_spt(struct intel_vgpu_ppgtt_spt *spt);
 
 static struct intel_vgpu_ppgtt_spt *ppgtt_populate_spt_by_guest_entry(
@@ -941,6 +965,7 @@  static struct intel_vgpu_ppgtt_spt *ppgtt_populate_spt_by_guest_entry(
 {
 	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
 	struct intel_vgpu_ppgtt_spt *spt = NULL;
+	bool ips = false;
 	int ret;
 
 	GEM_BUG_ON(!gtt_type_is_pt(get_next_pt_type(we->type)));
@@ -951,7 +976,10 @@  static struct intel_vgpu_ppgtt_spt *ppgtt_populate_spt_by_guest_entry(
 	else {
 		int type = get_next_pt_type(we->type);
 
-		spt = ppgtt_alloc_spt(vgpu, type, ops->get_pfn(we));
+		if (we->type == GTT_TYPE_PPGTT_PDE_ENTRY)
+			ips = vgpu_ips_enabled(vgpu) && ops->test_ips(we);
+
+		spt = ppgtt_alloc_spt(vgpu, type, ops->get_pfn(we), ips);
 		if (IS_ERR(spt)) {
 			ret = PTR_ERR(spt);
 			goto fail;
@@ -1427,8 +1455,6 @@  static int ppgtt_handle_guest_write_page_table_bytes(
 
 	ppgtt_get_guest_entry(spt, &we, index);
 
-	ops->test_pse(&we);
-
 	if (bytes == info->gtt_entry_size) {
 		ret = ppgtt_handle_guest_write_page_table(spt, &we, index);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index 9257b74..c11284b 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -223,6 +223,7 @@  struct intel_vgpu_ppgtt_spt {
 
 	struct {
 		intel_gvt_gtt_type_t type;
+		bool pde_ips; /* for 64KB PTEs */
 		void *vaddr;
 		struct page *page;
 		unsigned long mfn;
@@ -230,6 +231,7 @@  struct intel_vgpu_ppgtt_spt {
 
 	struct {
 		intel_gvt_gtt_type_t type;
+		bool pde_ips; /* for 64KB PTEs */
 		unsigned long gfn;
 		unsigned long write_cnt;
 		struct intel_vgpu_oos_page *oos_page;

Comments

On 2018.05.04 17:50:30 +0800, changbin.du@intel.com wrote:
>
> @@ -934,6 +944,20 @@ static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt)
>  	return ret;
>  }
>  
> +static bool vgpu_ips_enabled(struct intel_vgpu *vgpu)
> +{
> +	if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) {
> +		u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) &
> +			GAMW_ECO_ENABLE_64K_IPS_FIELD;
> +
> +		return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD;

Should 64K will always be required to enable for all engines or we
should only check if any engine has 64K ppgtt enabled?


> +	} else if (INTEL_GEN(vgpu->gvt->dev_priv) >= 10) {
> +		/* 64K paging only controlled by IPS bit in PTE now. */
> +		return true;
> +	} else
> +		return false;
> +}
> +
>  static int ppgtt_populate_spt(struct intel_vgpu_ppgtt_spt *spt);
>  
>  static struct intel_vgpu_ppgtt_spt *ppgtt_populate_spt_by_guest_entry(
On Mon, May 07, 2018 at 10:56:27AM +0800, Zhenyu Wang wrote:
> On 2018.05.04 17:50:30 +0800, changbin.du@intel.com wrote:
> >
> > @@ -934,6 +944,20 @@ static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt)
> >  	return ret;
> >  }
> >  
> > +static bool vgpu_ips_enabled(struct intel_vgpu *vgpu)
> > +{
> > +	if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) {
> > +		u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) &
> > +			GAMW_ECO_ENABLE_64K_IPS_FIELD;
> > +
> > +		return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD;
> 
> Should 64K will always be required to enable for all engines or we
> should only check if any engine has 64K ppgtt enabled?
>
64K ppgtt is not controlled by special per-engine flag. It is controlled by a global
flag and IPS bit in PDE.

> 
> > +	} else if (INTEL_GEN(vgpu->gvt->dev_priv) >= 10) {
> > +		/* 64K paging only controlled by IPS bit in PTE now. */
> > +		return true;
> > +	} else
> > +		return false;
> > +}
> > +
> >  static int ppgtt_populate_spt(struct intel_vgpu_ppgtt_spt *spt);
> >  
> >  static struct intel_vgpu_ppgtt_spt *ppgtt_populate_spt_by_guest_entry(
> 
> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
On 2018.05.07 11:03:55 +0800, Du, Changbin wrote:
> On Mon, May 07, 2018 at 10:56:27AM +0800, Zhenyu Wang wrote:
> > On 2018.05.04 17:50:30 +0800, changbin.du@intel.com wrote:
> > >
> > > @@ -934,6 +944,20 @@ static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt)
> > >  	return ret;
> > >  }
> > >  
> > > +static bool vgpu_ips_enabled(struct intel_vgpu *vgpu)
> > > +{
> > > +	if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) {
> > > +		u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) &
> > > +			GAMW_ECO_ENABLE_64K_IPS_FIELD;
> > > +
> > > +		return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD;
> > 
> > Should 64K will always be required to enable for all engines or we
> > should only check if any engine has 64K ppgtt enabled?
> >
> 64K ppgtt is not controlled by special per-engine flag. It is controlled by a global
> flag and IPS bit in PDE.
>
GEN8_GAMW_ECO_DEV_RW_IA has control of 64k ppgtt for each engine right?
So need to check all of them or any of them?
On Mon, May 07, 2018 at 11:09:17AM +0800, Zhenyu Wang wrote:
> On 2018.05.07 11:03:55 +0800, Du, Changbin wrote:
> > On Mon, May 07, 2018 at 10:56:27AM +0800, Zhenyu Wang wrote:
> > > On 2018.05.04 17:50:30 +0800, changbin.du@intel.com wrote:
> > > >
> > > > @@ -934,6 +944,20 @@ static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt)
> > > >  	return ret;
> > > >  }
> > > >  
> > > > +static bool vgpu_ips_enabled(struct intel_vgpu *vgpu)
> > > > +{
> > > > +	if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) {
> > > > +		u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) &
> > > > +			GAMW_ECO_ENABLE_64K_IPS_FIELD;
> > > > +
> > > > +		return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD;
> > > 
> > > Should 64K will always be required to enable for all engines or we
> > > should only check if any engine has 64K ppgtt enabled?
> > >
> > 64K ppgtt is not controlled by special per-engine flag. It is controlled by a global
> > flag and IPS bit in PDE.
> >
> GEN8_GAMW_ECO_DEV_RW_IA has control of 64k ppgtt for each engine right?
> So need to check all of them or any of them?
>
Yes, you are right after checking bspec. I was curious why this is a multi-bits
flag!

> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
On Mon, May 07, 2018 at 11:09:17AM +0800, Zhenyu Wang wrote:
> On 2018.05.07 11:03:55 +0800, Du, Changbin wrote:
> > On Mon, May 07, 2018 at 10:56:27AM +0800, Zhenyu Wang wrote:
> > > On 2018.05.04 17:50:30 +0800, changbin.du@intel.com wrote:
> > > >
> > > > @@ -934,6 +944,20 @@ static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt)
> > > >  	return ret;
> > > >  }
> > > >  
> > > > +static bool vgpu_ips_enabled(struct intel_vgpu *vgpu)
> > > > +{
> > > > +	if (INTEL_GEN(vgpu->gvt->dev_priv) == 9) {
> > > > +		u32 ips = vgpu_vreg_t(vgpu, GEN8_GAMW_ECO_DEV_RW_IA) &
> > > > +			GAMW_ECO_ENABLE_64K_IPS_FIELD;
> > > > +
> > > > +		return ips == GAMW_ECO_ENABLE_64K_IPS_FIELD;
> > > 
> > > Should 64K will always be required to enable for all engines or we
> > > should only check if any engine has 64K ppgtt enabled?
> > >
> > 64K ppgtt is not controlled by special per-engine flag. It is controlled by a global
> > flag and IPS bit in PDE.
> >
> GEN8_GAMW_ECO_DEV_RW_IA has control of 64k ppgtt for each engine right?
> So need to check all of them or any of them?
> 
It is a problem now. We cannot know which engine the ppgtt will bind to! A good
news is no guest does partial IPS enabling. So we can warn user for unsupported
setting.

> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827