| Message ID | 20161224193146.4402-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/24/2016 11:31 AM, Chris Wilson wrote: > Add an assertion to the plain i915_ggtt_offset() to double check that > any offset we hand to the GuC is outside of its unmappable ranges. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 14 +++++++------- > drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++--- > drivers/gpu/drm/i915/intel_uc.h | 9 +++++++++ > 3 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index 3e20fe2be811..30e012b9e93c 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -270,11 +270,11 @@ static void guc_ctx_desc_init(struct intel_guc *guc, > > /* The state page is after PPHWSP */ > lrc->ring_lcra = > - i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; > + guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; > lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) | > (guc_engine_id << GUC_ELC_ENGINE_OFFSET); > > - lrc->ring_begin = i915_ggtt_offset(ce->ring->vma); > + lrc->ring_begin = guc_ggtt_offset(ce->ring->vma); > lrc->ring_end = lrc->ring_begin + ce->ring->size - 1; > lrc->ring_next_free_location = lrc->ring_begin; > lrc->ring_current_tail_pointer_value = 0; > @@ -290,7 +290,7 @@ static void guc_ctx_desc_init(struct intel_guc *guc, > * The doorbell, process descriptor, and workqueue are all parts > * of the client object, which the GuC will reference via the GGTT > */ > - gfx_addr = i915_ggtt_offset(client->vma); > + gfx_addr = guc_ggtt_offset(client->vma); > desc.db_trigger_phy = sg_dma_address(client->vma->pages->sgl) + > client->doorbell_offset; > desc.db_trigger_cpu = > @@ -1226,7 +1226,7 @@ static void guc_log_create(struct intel_guc *guc) > (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) | > (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT); > > - offset = i915_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ > + offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ > guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags; > } > > @@ -1329,7 +1329,7 @@ static void guc_addon_create(struct intel_guc *guc) > guc_policies_init(policies); > > ads->scheduler_policies = > - i915_ggtt_offset(vma) + sizeof(struct guc_ads); > + guc_ggtt_offset(vma) + sizeof(struct guc_ads); > > /* MMIO reg state */ > reg_state = (void *)policies + sizeof(struct guc_policies); > @@ -1495,7 +1495,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv) > /* any value greater than GUC_POWER_D0 */ > data[1] = GUC_POWER_D1; > /* first page is shared data with GuC */ > - data[2] = i915_ggtt_offset(ctx->engine[RCS].state); > + data[2] = guc_ggtt_offset(ctx->engine[RCS].state); > > return intel_guc_send(guc, data, ARRAY_SIZE(data)); > } > @@ -1522,7 +1522,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv) > data[0] = INTEL_GUC_ACTION_EXIT_S_STATE; > data[1] = GUC_POWER_D0; > /* first page is shared data with GuC */ > - data[2] = i915_ggtt_offset(ctx->engine[RCS].state); > + data[2] = guc_ggtt_offset(ctx->engine[RCS].state); > > return intel_guc_send(guc, data, ARRAY_SIZE(data)); > } > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 21db69705458..35d5690f47a2 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -220,14 +220,14 @@ static void guc_params_init(struct drm_i915_private *dev_priv) > params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED; > > if (guc->ads_vma) { > - u32 ads = i915_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; > + u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; > params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT; > params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED; > } > > /* If GuC submission is enabled, set up additional parameters here */ > if (i915.enable_guc_submission) { > - u32 pgs = i915_ggtt_offset(dev_priv->guc.ctx_pool_vma); > + u32 pgs = guc_ggtt_offset(dev_priv->guc.ctx_pool_vma); > u32 ctx_in_16 = GUC_MAX_GPU_CONTEXTS / 16; > > pgs >>= PAGE_SHIFT; > @@ -297,7 +297,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, > I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size); > > /* Set the source address for the new blob */ > - offset = i915_ggtt_offset(vma) + guc_fw->header_offset; > + offset = guc_ggtt_offset(vma) + guc_fw->header_offset; > I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset)); > I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF); > > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > index 11f56082b363..d556215e691f 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -28,6 +28,8 @@ > #include "i915_guc_reg.h" > #include "intel_ringbuffer.h" > > +#include "i915_vma.h" > + > struct drm_i915_gem_request; > > /* > @@ -198,4 +200,11 @@ void i915_guc_register(struct drm_i915_private *dev_priv); > void i915_guc_unregister(struct drm_i915_private *dev_priv); > int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val); > > +static inline u32 guc_ggtt_offset(struct i915_vma *vma) > +{ > + u32 offset = i915_ggtt_offset(vma); > + GEM_BUG_ON(offset < GUC_WOPCM_TOP); GEM_BUG_ON(offset < GUC_WOPCM_TOP || offset > 0xFEE00000); Maybe we could add a define for 0xFEE00000 as we might have to re-use it elsewhere. With the change for the GEM_BUG_ON: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> BR, Daniele > + return offset; > +} > + > #endif
On Sat, Dec 24, 2016 at 12:52:37PM -0800, Ceraolo Spurio, Daniele wrote: > > > On 12/24/2016 11:31 AM, Chris Wilson wrote: > >Add an assertion to the plain i915_ggtt_offset() to double check that > >any offset we hand to the GuC is outside of its unmappable ranges. > > > >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >--- > > drivers/gpu/drm/i915/i915_guc_submission.c | 14 +++++++------- > > drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++--- > > drivers/gpu/drm/i915/intel_uc.h | 9 +++++++++ > > 3 files changed, 19 insertions(+), 10 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > >index 3e20fe2be811..30e012b9e93c 100644 > >--- a/drivers/gpu/drm/i915/i915_guc_submission.c > >+++ b/drivers/gpu/drm/i915/i915_guc_submission.c > >@@ -270,11 +270,11 @@ static void guc_ctx_desc_init(struct intel_guc *guc, > > /* The state page is after PPHWSP */ > > lrc->ring_lcra = > >- i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; > >+ guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; > > lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) | > > (guc_engine_id << GUC_ELC_ENGINE_OFFSET); > >- lrc->ring_begin = i915_ggtt_offset(ce->ring->vma); > >+ lrc->ring_begin = guc_ggtt_offset(ce->ring->vma); > > lrc->ring_end = lrc->ring_begin + ce->ring->size - 1; > > lrc->ring_next_free_location = lrc->ring_begin; > > lrc->ring_current_tail_pointer_value = 0; > >@@ -290,7 +290,7 @@ static void guc_ctx_desc_init(struct intel_guc *guc, > > * The doorbell, process descriptor, and workqueue are all parts > > * of the client object, which the GuC will reference via the GGTT > > */ > >- gfx_addr = i915_ggtt_offset(client->vma); > >+ gfx_addr = guc_ggtt_offset(client->vma); > > desc.db_trigger_phy = sg_dma_address(client->vma->pages->sgl) + > > client->doorbell_offset; > > desc.db_trigger_cpu = > >@@ -1226,7 +1226,7 @@ static void guc_log_create(struct intel_guc *guc) > > (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) | > > (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT); > >- offset = i915_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ > >+ offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ > > guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags; > > } > >@@ -1329,7 +1329,7 @@ static void guc_addon_create(struct intel_guc *guc) > > guc_policies_init(policies); > > ads->scheduler_policies = > >- i915_ggtt_offset(vma) + sizeof(struct guc_ads); > >+ guc_ggtt_offset(vma) + sizeof(struct guc_ads); > > /* MMIO reg state */ > > reg_state = (void *)policies + sizeof(struct guc_policies); > >@@ -1495,7 +1495,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv) > > /* any value greater than GUC_POWER_D0 */ > > data[1] = GUC_POWER_D1; > > /* first page is shared data with GuC */ > >- data[2] = i915_ggtt_offset(ctx->engine[RCS].state); > >+ data[2] = guc_ggtt_offset(ctx->engine[RCS].state); > > return intel_guc_send(guc, data, ARRAY_SIZE(data)); > > } > >@@ -1522,7 +1522,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv) > > data[0] = INTEL_GUC_ACTION_EXIT_S_STATE; > > data[1] = GUC_POWER_D0; > > /* first page is shared data with GuC */ > >- data[2] = i915_ggtt_offset(ctx->engine[RCS].state); > >+ data[2] = guc_ggtt_offset(ctx->engine[RCS].state); > > return intel_guc_send(guc, data, ARRAY_SIZE(data)); > > } > >diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > >index 21db69705458..35d5690f47a2 100644 > >--- a/drivers/gpu/drm/i915/intel_guc_loader.c > >+++ b/drivers/gpu/drm/i915/intel_guc_loader.c > >@@ -220,14 +220,14 @@ static void guc_params_init(struct drm_i915_private *dev_priv) > > params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED; > > if (guc->ads_vma) { > >- u32 ads = i915_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; > >+ u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; > > params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT; > > params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED; > > } > > /* If GuC submission is enabled, set up additional parameters here */ > > if (i915.enable_guc_submission) { > >- u32 pgs = i915_ggtt_offset(dev_priv->guc.ctx_pool_vma); > >+ u32 pgs = guc_ggtt_offset(dev_priv->guc.ctx_pool_vma); > > u32 ctx_in_16 = GUC_MAX_GPU_CONTEXTS / 16; > > pgs >>= PAGE_SHIFT; > >@@ -297,7 +297,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, > > I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size); > > /* Set the source address for the new blob */ > >- offset = i915_ggtt_offset(vma) + guc_fw->header_offset; > >+ offset = guc_ggtt_offset(vma) + guc_fw->header_offset; > > I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset)); > > I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF); > >diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > >index 11f56082b363..d556215e691f 100644 > >--- a/drivers/gpu/drm/i915/intel_uc.h > >+++ b/drivers/gpu/drm/i915/intel_uc.h > >@@ -28,6 +28,8 @@ > > #include "i915_guc_reg.h" > > #include "intel_ringbuffer.h" > >+#include "i915_vma.h" > >+ > > struct drm_i915_gem_request; > > /* > >@@ -198,4 +200,11 @@ void i915_guc_register(struct drm_i915_private *dev_priv); > > void i915_guc_unregister(struct drm_i915_private *dev_priv); > > int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val); > >+static inline u32 guc_ggtt_offset(struct i915_vma *vma) > >+{ > >+ u32 offset = i915_ggtt_offset(vma); > >+ GEM_BUG_ON(offset < GUC_WOPCM_TOP); > > GEM_BUG_ON(offset < GUC_WOPCM_TOP || offset > 0xFEE00000); > > Maybe we could add a define for 0xFEE00000 as we might have to > re-use it elsewhere. > > With the change for the GEM_BUG_ON: > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> I wasn't going to do the upper check until we have the fix in place (i.e. that patch to apply the limit would add the assertion as well). No point in intentionally panicking the kernel, just hang the hw instead! ;) -Chris
On 12/24/2016 1:22 PM, Chris Wilson wrote: > On Sat, Dec 24, 2016 at 12:52:37PM -0800, Ceraolo Spurio, Daniele wrote: >> >> On 12/24/2016 11:31 AM, Chris Wilson wrote: >>> Add an assertion to the plain i915_ggtt_offset() to double check that >>> any offset we hand to the GuC is outside of its unmappable ranges. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_guc_submission.c | 14 +++++++------- >>> drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++--- >>> drivers/gpu/drm/i915/intel_uc.h | 9 +++++++++ >>> 3 files changed, 19 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c >>> index 3e20fe2be811..30e012b9e93c 100644 >>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c >>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c >>> @@ -270,11 +270,11 @@ static void guc_ctx_desc_init(struct intel_guc *guc, >>> /* The state page is after PPHWSP */ >>> lrc->ring_lcra = >>> - i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; >>> + guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; >>> lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) | >>> (guc_engine_id << GUC_ELC_ENGINE_OFFSET); >>> - lrc->ring_begin = i915_ggtt_offset(ce->ring->vma); >>> + lrc->ring_begin = guc_ggtt_offset(ce->ring->vma); >>> lrc->ring_end = lrc->ring_begin + ce->ring->size - 1; >>> lrc->ring_next_free_location = lrc->ring_begin; >>> lrc->ring_current_tail_pointer_value = 0; >>> @@ -290,7 +290,7 @@ static void guc_ctx_desc_init(struct intel_guc *guc, >>> * The doorbell, process descriptor, and workqueue are all parts >>> * of the client object, which the GuC will reference via the GGTT >>> */ >>> - gfx_addr = i915_ggtt_offset(client->vma); >>> + gfx_addr = guc_ggtt_offset(client->vma); >>> desc.db_trigger_phy = sg_dma_address(client->vma->pages->sgl) + >>> client->doorbell_offset; >>> desc.db_trigger_cpu = >>> @@ -1226,7 +1226,7 @@ static void guc_log_create(struct intel_guc *guc) >>> (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) | >>> (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT); >>> - offset = i915_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ >>> + offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ >>> guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags; >>> } >>> @@ -1329,7 +1329,7 @@ static void guc_addon_create(struct intel_guc *guc) >>> guc_policies_init(policies); >>> ads->scheduler_policies = >>> - i915_ggtt_offset(vma) + sizeof(struct guc_ads); >>> + guc_ggtt_offset(vma) + sizeof(struct guc_ads); >>> /* MMIO reg state */ >>> reg_state = (void *)policies + sizeof(struct guc_policies); >>> @@ -1495,7 +1495,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv) >>> /* any value greater than GUC_POWER_D0 */ >>> data[1] = GUC_POWER_D1; >>> /* first page is shared data with GuC */ >>> - data[2] = i915_ggtt_offset(ctx->engine[RCS].state); >>> + data[2] = guc_ggtt_offset(ctx->engine[RCS].state); >>> return intel_guc_send(guc, data, ARRAY_SIZE(data)); >>> } >>> @@ -1522,7 +1522,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv) >>> data[0] = INTEL_GUC_ACTION_EXIT_S_STATE; >>> data[1] = GUC_POWER_D0; >>> /* first page is shared data with GuC */ >>> - data[2] = i915_ggtt_offset(ctx->engine[RCS].state); >>> + data[2] = guc_ggtt_offset(ctx->engine[RCS].state); >>> return intel_guc_send(guc, data, ARRAY_SIZE(data)); >>> } >>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c >>> index 21db69705458..35d5690f47a2 100644 >>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c >>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c >>> @@ -220,14 +220,14 @@ static void guc_params_init(struct drm_i915_private *dev_priv) >>> params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED; >>> if (guc->ads_vma) { >>> - u32 ads = i915_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; >>> + u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; >>> params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT; >>> params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED; >>> } >>> /* If GuC submission is enabled, set up additional parameters here */ >>> if (i915.enable_guc_submission) { >>> - u32 pgs = i915_ggtt_offset(dev_priv->guc.ctx_pool_vma); >>> + u32 pgs = guc_ggtt_offset(dev_priv->guc.ctx_pool_vma); >>> u32 ctx_in_16 = GUC_MAX_GPU_CONTEXTS / 16; >>> pgs >>= PAGE_SHIFT; >>> @@ -297,7 +297,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, >>> I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size); >>> /* Set the source address for the new blob */ >>> - offset = i915_ggtt_offset(vma) + guc_fw->header_offset; >>> + offset = guc_ggtt_offset(vma) + guc_fw->header_offset; >>> I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset)); >>> I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF); >>> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h >>> index 11f56082b363..d556215e691f 100644 >>> --- a/drivers/gpu/drm/i915/intel_uc.h >>> +++ b/drivers/gpu/drm/i915/intel_uc.h >>> @@ -28,6 +28,8 @@ >>> #include "i915_guc_reg.h" >>> #include "intel_ringbuffer.h" >>> +#include "i915_vma.h" >>> + >>> struct drm_i915_gem_request; >>> /* >>> @@ -198,4 +200,11 @@ void i915_guc_register(struct drm_i915_private *dev_priv); >>> void i915_guc_unregister(struct drm_i915_private *dev_priv); >>> int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val); >>> +static inline u32 guc_ggtt_offset(struct i915_vma *vma) >>> +{ >>> + u32 offset = i915_ggtt_offset(vma); >>> + GEM_BUG_ON(offset < GUC_WOPCM_TOP); >> GEM_BUG_ON(offset < GUC_WOPCM_TOP || offset > 0xFEE00000); >> >> Maybe we could add a define for 0xFEE00000 as we might have to >> re-use it elsewhere. >> >> With the change for the GEM_BUG_ON: >> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > I wasn't going to do the upper check until we have the fix in place > (i.e. that patch to apply the limit would add the assertion as well). No > point in intentionally panicking the kernel, just hang the hw instead! > ;) > -Chris > Oh well, in that case I'll trust you to update the check after the fix is merged ;-) You can apply my r-b to this version. Thanks, Daniele
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 3e20fe2be811..30e012b9e93c 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -270,11 +270,11 @@ static void guc_ctx_desc_init(struct intel_guc *guc, /* The state page is after PPHWSP */ lrc->ring_lcra = - i915_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; + guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE; lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) | (guc_engine_id << GUC_ELC_ENGINE_OFFSET); - lrc->ring_begin = i915_ggtt_offset(ce->ring->vma); + lrc->ring_begin = guc_ggtt_offset(ce->ring->vma); lrc->ring_end = lrc->ring_begin + ce->ring->size - 1; lrc->ring_next_free_location = lrc->ring_begin; lrc->ring_current_tail_pointer_value = 0; @@ -290,7 +290,7 @@ static void guc_ctx_desc_init(struct intel_guc *guc, * The doorbell, process descriptor, and workqueue are all parts * of the client object, which the GuC will reference via the GGTT */ - gfx_addr = i915_ggtt_offset(client->vma); + gfx_addr = guc_ggtt_offset(client->vma); desc.db_trigger_phy = sg_dma_address(client->vma->pages->sgl) + client->doorbell_offset; desc.db_trigger_cpu = @@ -1226,7 +1226,7 @@ static void guc_log_create(struct intel_guc *guc) (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) | (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT); - offset = i915_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ + offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags; } @@ -1329,7 +1329,7 @@ static void guc_addon_create(struct intel_guc *guc) guc_policies_init(policies); ads->scheduler_policies = - i915_ggtt_offset(vma) + sizeof(struct guc_ads); + guc_ggtt_offset(vma) + sizeof(struct guc_ads); /* MMIO reg state */ reg_state = (void *)policies + sizeof(struct guc_policies); @@ -1495,7 +1495,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv) /* any value greater than GUC_POWER_D0 */ data[1] = GUC_POWER_D1; /* first page is shared data with GuC */ - data[2] = i915_ggtt_offset(ctx->engine[RCS].state); + data[2] = guc_ggtt_offset(ctx->engine[RCS].state); return intel_guc_send(guc, data, ARRAY_SIZE(data)); } @@ -1522,7 +1522,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv) data[0] = INTEL_GUC_ACTION_EXIT_S_STATE; data[1] = GUC_POWER_D0; /* first page is shared data with GuC */ - data[2] = i915_ggtt_offset(ctx->engine[RCS].state); + data[2] = guc_ggtt_offset(ctx->engine[RCS].state); return intel_guc_send(guc, data, ARRAY_SIZE(data)); } diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 21db69705458..35d5690f47a2 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -220,14 +220,14 @@ static void guc_params_init(struct drm_i915_private *dev_priv) params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED; if (guc->ads_vma) { - u32 ads = i915_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; + u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT; params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED; } /* If GuC submission is enabled, set up additional parameters here */ if (i915.enable_guc_submission) { - u32 pgs = i915_ggtt_offset(dev_priv->guc.ctx_pool_vma); + u32 pgs = guc_ggtt_offset(dev_priv->guc.ctx_pool_vma); u32 ctx_in_16 = GUC_MAX_GPU_CONTEXTS / 16; pgs >>= PAGE_SHIFT; @@ -297,7 +297,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size); /* Set the source address for the new blob */ - offset = i915_ggtt_offset(vma) + guc_fw->header_offset; + offset = guc_ggtt_offset(vma) + guc_fw->header_offset; I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset)); I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF); diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 11f56082b363..d556215e691f 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -28,6 +28,8 @@ #include "i915_guc_reg.h" #include "intel_ringbuffer.h" +#include "i915_vma.h" + struct drm_i915_gem_request; /* @@ -198,4 +200,11 @@ void i915_guc_register(struct drm_i915_private *dev_priv); void i915_guc_unregister(struct drm_i915_private *dev_priv); int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val); +static inline u32 guc_ggtt_offset(struct i915_vma *vma) +{ + u32 offset = i915_ggtt_offset(vma); + GEM_BUG_ON(offset < GUC_WOPCM_TOP); + return offset; +} + #endif
Add an assertion to the plain i915_ggtt_offset() to double check that any offset we hand to the GuC is outside of its unmappable ranges. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> --- drivers/gpu/drm/i915/i915_guc_submission.c | 14 +++++++------- drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++--- drivers/gpu/drm/i915/intel_uc.h | 9 +++++++++ 3 files changed, 19 insertions(+), 10 deletions(-)