diff mbox series

[v7,14/15] drm/i915/gsc: allocate extended operational memory in LMEM

Message ID 20220806122636.43068-15-tomas.winkler@intel.com (mailing list archive)
State New, archived
Headers show
Series GSC support for XeHP SDV and DG2 | expand

Commit Message

Winkler, Tomas Aug. 6, 2022, 12:26 p.m. UTC 
GSC requires more operational memory than available on chip.
Reserve 4M of LMEM for GSC operation. The memory is provided to the
GSC as struct resource to the auxiliary data of the child device.

Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gsc.c | 91 ++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/gt/intel_gsc.h |  3 +
 2 files changed, 87 insertions(+), 7 deletions(-)

Comments

Alan Previn Sept. 1, 2022, 4:31 p.m. UTC | #1 
This patch hasnt changed since v5 and i already provided the R-b then so re-posting rb so patchworks can pick it up:

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>


On Sat, 2022-08-06 at 15:26 +0300, Winkler, Tomas wrote:
> GSC requires more operational memory than available on chip.
> Reserve 4M of LMEM for GSC operation. The memory is provided to the
> GSC as struct resource to the auxiliary data of the child device.
> 
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gsc.c | 91 ++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/gt/intel_gsc.h |  3 +
>  2 files changed, 87 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
> index e1040c8f2fd3..162bea57fbb5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
> @@ -7,6 +7,7 @@
>  #include <linux/mei_aux.h>
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> +#include "gem/i915_gem_region.h"
>  #include "gt/intel_gsc.h"
>  #include "gt/intel_gt.h"
>  
> @@ -36,12 +37,68 @@ static int gsc_irq_init(int irq)
>  	return irq_set_chip_data(irq, NULL);
>  }
>  
> +static int
> +gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t size)
> +{
> +	struct intel_gt *gt = gsc_to_gt(gsc);
> +	struct drm_i915_gem_object *obj;
> +	void *vaddr;
> +	int err;
> +
> +	obj = i915_gem_object_create_lmem(gt->i915, size, I915_BO_ALLOC_CONTIGUOUS);
> +	if (IS_ERR(obj)) {
> +		drm_err(&gt->i915->drm, "Failed to allocate gsc memory\n");
> +		return PTR_ERR(obj);
> +	}
> +
> +	err = i915_gem_object_pin_pages_unlocked(obj);
> +	if (err) {
> +		drm_err(&gt->i915->drm, "Failed to pin pages for gsc memory\n");
> +		goto out_put;
> +	}
> +
> +	vaddr = i915_gem_object_pin_map_unlocked(obj, i915_coherent_map_type(gt->i915, obj, true));
> +	if (IS_ERR(vaddr)) {
> +		err = PTR_ERR(vaddr);
> +		drm_err(&gt->i915->drm, "Failed to map gsc memory\n");
> +		goto out_unpin;
> +	}
> +
> +	memset(vaddr, 0, obj->base.size);
> +
> +	i915_gem_object_unpin_map(obj);
> +
> +	intf->gem_obj = obj;
> +
> +	return 0;
> +
> +out_unpin:
> +	i915_gem_object_unpin_pages(obj);
> +out_put:
> +	i915_gem_object_put(obj);
> +	return err;
> +}
> +
> +static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
> +{
> +	struct drm_i915_gem_object *obj = fetch_and_zero(&intf->gem_obj);
> +
> +	if (!obj)
> +		return;
> +
> +	if (i915_gem_object_has_pinned_pages(obj))
> +		i915_gem_object_unpin_pages(obj);
> +
> +	i915_gem_object_put(obj);
> +}
> +
>  struct gsc_def {
>  	const char *name;
>  	unsigned long bar;
>  	size_t bar_size;
>  	bool use_polling;
>  	bool slow_firmware;
> +	size_t lmem_size;
>  };
>  
>  /* gsc resources and definitions (HECI1 and HECI2) */
> @@ -74,6 +131,7 @@ static const struct gsc_def gsc_def_dg2[] = {
>  		.name = "mei-gsc",
>  		.bar = DG2_GSC_HECI1_BASE,
>  		.bar_size = GSC_BAR_LENGTH,
> +		.lmem_size = SZ_4M,
>  	},
>  	{
>  		.name = "mei-gscfi",
> @@ -90,26 +148,32 @@ static void gsc_release_dev(struct device *dev)
>  	kfree(adev);
>  }
>  
> -static void gsc_destroy_one(struct intel_gsc_intf *intf)
> +static void gsc_destroy_one(struct drm_i915_private *i915,
> +			    struct intel_gsc *gsc, unsigned int intf_id)
>  {
> +	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
> +
>  	if (intf->adev) {
>  		auxiliary_device_delete(&intf->adev->aux_dev);
>  		auxiliary_device_uninit(&intf->adev->aux_dev);
>  		intf->adev = NULL;
>  	}
> +
>  	if (intf->irq >= 0)
>  		irq_free_desc(intf->irq);
>  	intf->irq = -1;
> +
> +	gsc_ext_om_destroy(intf);
>  }
>  
> -static void gsc_init_one(struct drm_i915_private *i915,
> -			 struct intel_gsc_intf *intf,
> +static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
>  			 unsigned int intf_id)
>  {
>  	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>  	struct mei_aux_device *adev;
>  	struct auxiliary_device *aux_dev;
>  	const struct gsc_def *def;
> +	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
>  	int ret;
>  
>  	intf->irq = -1;
> @@ -141,7 +205,7 @@ static void gsc_init_one(struct drm_i915_private *i915,
>  	intf->irq = irq_alloc_desc(0);
>  	if (intf->irq < 0) {
>  		drm_err(&i915->drm, "gsc irq error %d\n", intf->irq);
> -		return;
> +		goto fail;
>  	}
>  
>  	ret = gsc_irq_init(intf->irq);
> @@ -155,6 +219,19 @@ static void gsc_init_one(struct drm_i915_private *i915,
>  	if (!adev)
>  		goto fail;
>  
> +	if (def->lmem_size) {
> +		drm_dbg(&i915->drm, "setting up GSC lmem\n");
> +
> +		if (gsc_ext_om_alloc(gsc, intf, def->lmem_size)) {
> +			drm_err(&i915->drm, "setting up gsc extended operational memory failed\n");
> +			kfree(adev);
> +			goto fail;
> +		}
> +
> +		adev->ext_op_mem.start = i915_gem_object_get_dma_address(intf->gem_obj, 0);
> +		adev->ext_op_mem.end = adev->ext_op_mem.start + def->lmem_size;
> +	}
> +
>  	adev->irq = intf->irq;
>  	adev->bar.parent = &pdev->resource[0];
>  	adev->bar.start = def->bar + pdev->resource[0].start;
> @@ -188,7 +265,7 @@ static void gsc_init_one(struct drm_i915_private *i915,
>  
>  	return;
>  fail:
> -	gsc_destroy_one(intf);
> +	gsc_destroy_one(i915, gsc, intf->id);
>  }
>  
>  static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
> @@ -229,7 +306,7 @@ void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *i915)
>  		return;
>  
>  	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> -		gsc_init_one(i915, &gsc->intf[i], i);
> +		gsc_init_one(i915, gsc, i);
>  }
>  
>  void intel_gsc_fini(struct intel_gsc *gsc)
> @@ -241,5 +318,5 @@ void intel_gsc_fini(struct intel_gsc *gsc)
>  		return;
>  
>  	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> -		gsc_destroy_one(&gsc->intf[i]);
> +		gsc_destroy_one(gt->i915, gsc, i);
>  }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.h b/drivers/gpu/drm/i915/gt/intel_gsc.h
> index 68582f912b21..fcac1775e9c3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.h
> @@ -20,11 +20,14 @@ struct mei_aux_device;
>  
>  /**
>   * struct intel_gsc - graphics security controller
> + *
> + * @gem_obj: scratch memory GSC operations
>   * @intf : gsc interface
>   */
>  struct intel_gsc {
>  	struct intel_gsc_intf {
>  		struct mei_aux_device *adev;
> +		struct drm_i915_gem_object *gem_obj;
>  		int irq;
>  		unsigned int id;
>  	} intf[INTEL_GSC_NUM_INTERFACES];
> -- 
> 2.37.1
>
Matthew Auld Sept. 2, 2022, 8:25 a.m. UTC | #2 
On Sat, 6 Aug 2022 at 13:34, Tomas Winkler <tomas.winkler@intel.com> wrote:
>
> GSC requires more operational memory than available on chip.
> Reserve 4M of LMEM for GSC operation. The memory is provided to the
> GSC as struct resource to the auxiliary data of the child device.
>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gsc.c | 91 ++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/gt/intel_gsc.h |  3 +
>  2 files changed, 87 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
> index e1040c8f2fd3..162bea57fbb5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
> @@ -7,6 +7,7 @@
>  #include <linux/mei_aux.h>
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> +#include "gem/i915_gem_region.h"
>  #include "gt/intel_gsc.h"
>  #include "gt/intel_gt.h"
>
> @@ -36,12 +37,68 @@ static int gsc_irq_init(int irq)
>         return irq_set_chip_data(irq, NULL);
>  }
>
> +static int
> +gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t size)
> +{
> +       struct intel_gt *gt = gsc_to_gt(gsc);
> +       struct drm_i915_gem_object *obj;
> +       void *vaddr;
> +       int err;
> +
> +       obj = i915_gem_object_create_lmem(gt->i915, size, I915_BO_ALLOC_CONTIGUOUS);
> +       if (IS_ERR(obj)) {
> +               drm_err(&gt->i915->drm, "Failed to allocate gsc memory\n");
> +               return PTR_ERR(obj);
> +       }
> +
> +       err = i915_gem_object_pin_pages_unlocked(obj);
> +       if (err) {
> +               drm_err(&gt->i915->drm, "Failed to pin pages for gsc memory\n");
> +               goto out_put;
> +       }
> +
> +       vaddr = i915_gem_object_pin_map_unlocked(obj, i915_coherent_map_type(gt->i915, obj, true));
> +       if (IS_ERR(vaddr)) {
> +               err = PTR_ERR(vaddr);
> +               drm_err(&gt->i915->drm, "Failed to map gsc memory\n");
> +               goto out_unpin;
> +       }
> +
> +       memset(vaddr, 0, obj->base.size);
> +
> +       i915_gem_object_unpin_map(obj);

I think this was mentioned before, here we should rather use:

create_lmem(gt->i915, size,
                      I915_BO_ALLOC_CONTIGUOUS |
                      I915_BO_ALLOC_CPU_CLEAR);

That way we don't need to manually map and clear it here. Instead when
first allocating the pages (like with pin_pages), the clear will be
done for you.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_gsc.c
index e1040c8f2fd3..162bea57fbb5 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.c
+++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
@@ -7,6 +7,7 @@ 
 #include <linux/mei_aux.h>
 #include "i915_drv.h"
 #include "i915_reg.h"
+#include "gem/i915_gem_region.h"
 #include "gt/intel_gsc.h"
 #include "gt/intel_gt.h"
 
@@ -36,12 +37,68 @@  static int gsc_irq_init(int irq)
 	return irq_set_chip_data(irq, NULL);
 }
 
+static int
+gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t size)
+{
+	struct intel_gt *gt = gsc_to_gt(gsc);
+	struct drm_i915_gem_object *obj;
+	void *vaddr;
+	int err;
+
+	obj = i915_gem_object_create_lmem(gt->i915, size, I915_BO_ALLOC_CONTIGUOUS);
+	if (IS_ERR(obj)) {
+		drm_err(&gt->i915->drm, "Failed to allocate gsc memory\n");
+		return PTR_ERR(obj);
+	}
+
+	err = i915_gem_object_pin_pages_unlocked(obj);
+	if (err) {
+		drm_err(&gt->i915->drm, "Failed to pin pages for gsc memory\n");
+		goto out_put;
+	}
+
+	vaddr = i915_gem_object_pin_map_unlocked(obj, i915_coherent_map_type(gt->i915, obj, true));
+	if (IS_ERR(vaddr)) {
+		err = PTR_ERR(vaddr);
+		drm_err(&gt->i915->drm, "Failed to map gsc memory\n");
+		goto out_unpin;
+	}
+
+	memset(vaddr, 0, obj->base.size);
+
+	i915_gem_object_unpin_map(obj);
+
+	intf->gem_obj = obj;
+
+	return 0;
+
+out_unpin:
+	i915_gem_object_unpin_pages(obj);
+out_put:
+	i915_gem_object_put(obj);
+	return err;
+}
+
+static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
+{
+	struct drm_i915_gem_object *obj = fetch_and_zero(&intf->gem_obj);
+
+	if (!obj)
+		return;
+
+	if (i915_gem_object_has_pinned_pages(obj))
+		i915_gem_object_unpin_pages(obj);
+
+	i915_gem_object_put(obj);
+}
+
 struct gsc_def {
 	const char *name;
 	unsigned long bar;
 	size_t bar_size;
 	bool use_polling;
 	bool slow_firmware;
+	size_t lmem_size;
 };
 
 /* gsc resources and definitions (HECI1 and HECI2) */
@@ -74,6 +131,7 @@  static const struct gsc_def gsc_def_dg2[] = {
 		.name = "mei-gsc",
 		.bar = DG2_GSC_HECI1_BASE,
 		.bar_size = GSC_BAR_LENGTH,
+		.lmem_size = SZ_4M,
 	},
 	{
 		.name = "mei-gscfi",
@@ -90,26 +148,32 @@  static void gsc_release_dev(struct device *dev)
 	kfree(adev);
 }
 
-static void gsc_destroy_one(struct intel_gsc_intf *intf)
+static void gsc_destroy_one(struct drm_i915_private *i915,
+			    struct intel_gsc *gsc, unsigned int intf_id)
 {
+	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
+
 	if (intf->adev) {
 		auxiliary_device_delete(&intf->adev->aux_dev);
 		auxiliary_device_uninit(&intf->adev->aux_dev);
 		intf->adev = NULL;
 	}
+
 	if (intf->irq >= 0)
 		irq_free_desc(intf->irq);
 	intf->irq = -1;
+
+	gsc_ext_om_destroy(intf);
 }
 
-static void gsc_init_one(struct drm_i915_private *i915,
-			 struct intel_gsc_intf *intf,
+static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
 			 unsigned int intf_id)
 {
 	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
 	struct mei_aux_device *adev;
 	struct auxiliary_device *aux_dev;
 	const struct gsc_def *def;
+	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
 	int ret;
 
 	intf->irq = -1;
@@ -141,7 +205,7 @@  static void gsc_init_one(struct drm_i915_private *i915,
 	intf->irq = irq_alloc_desc(0);
 	if (intf->irq < 0) {
 		drm_err(&i915->drm, "gsc irq error %d\n", intf->irq);
-		return;
+		goto fail;
 	}
 
 	ret = gsc_irq_init(intf->irq);
@@ -155,6 +219,19 @@  static void gsc_init_one(struct drm_i915_private *i915,
 	if (!adev)
 		goto fail;
 
+	if (def->lmem_size) {
+		drm_dbg(&i915->drm, "setting up GSC lmem\n");
+
+		if (gsc_ext_om_alloc(gsc, intf, def->lmem_size)) {
+			drm_err(&i915->drm, "setting up gsc extended operational memory failed\n");
+			kfree(adev);
+			goto fail;
+		}
+
+		adev->ext_op_mem.start = i915_gem_object_get_dma_address(intf->gem_obj, 0);
+		adev->ext_op_mem.end = adev->ext_op_mem.start + def->lmem_size;
+	}
+
 	adev->irq = intf->irq;
 	adev->bar.parent = &pdev->resource[0];
 	adev->bar.start = def->bar + pdev->resource[0].start;
@@ -188,7 +265,7 @@  static void gsc_init_one(struct drm_i915_private *i915,
 
 	return;
 fail:
-	gsc_destroy_one(intf);
+	gsc_destroy_one(i915, gsc, intf->id);
 }
 
 static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
@@ -229,7 +306,7 @@  void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *i915)
 		return;
 
 	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
-		gsc_init_one(i915, &gsc->intf[i], i);
+		gsc_init_one(i915, gsc, i);
 }
 
 void intel_gsc_fini(struct intel_gsc *gsc)
@@ -241,5 +318,5 @@  void intel_gsc_fini(struct intel_gsc *gsc)
 		return;
 
 	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
-		gsc_destroy_one(&gsc->intf[i]);
+		gsc_destroy_one(gt->i915, gsc, i);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.h b/drivers/gpu/drm/i915/gt/intel_gsc.h
index 68582f912b21..fcac1775e9c3 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.h
+++ b/drivers/gpu/drm/i915/gt/intel_gsc.h
@@ -20,11 +20,14 @@  struct mei_aux_device;
 
 /**
  * struct intel_gsc - graphics security controller
+ *
+ * @gem_obj: scratch memory GSC operations
  * @intf : gsc interface
  */
 struct intel_gsc {
 	struct intel_gsc_intf {
 		struct mei_aux_device *adev;
+		struct drm_i915_gem_object *gem_obj;
 		int irq;
 		unsigned int id;
 	} intf[INTEL_GSC_NUM_INTERFACES];