On 2017.01.06 19:58:16 +0000, Chris Wilson wrote:
> set_gma_to_bb_cmd() is completely bogus - it is (incorrectly) applying
> the rules to read a GTT offset from a command as opposed to writing the
> GTT offset. And to cap it all set_gma_to_bb_cmd() is called within a list
> iterator of the most strange construction.
>
Looks fine to me. I'll pick up this after validating with batch buffer scan on.
Thanks.
> Fixes: be1da7070aea ("drm/i915/gvt: vGPU command scanner")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> Cc: Yulei Zhang <yulei.zhang@xxxxxxxxx>
> Cc: <drm-intel-fixes@xxxxxxxxxxxxxxxxxxxxx> # v4.10-rc1+
> ---
> drivers/gpu/drm/i915/gvt/execlist.c | 64 ++++++++++--------------------------
> drivers/gpu/drm/i915/gvt/scheduler.h | 2 +-
> 2 files changed, 19 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
> index 9a98553ad742..4b8454b445cd 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -364,58 +364,30 @@ static void free_workload(struct intel_vgpu_workload *workload)
> #define get_desc_from_elsp_dwords(ed, i) \
> ((struct execlist_ctx_descriptor_format *)&((ed)->data[i * 2]))
>
> -
> -#define BATCH_BUFFER_ADDR_MASK ((1UL << 32) - (1U << 2))
> -#define BATCH_BUFFER_ADDR_HIGH_MASK ((1UL << 16) - (1U))
> -static int set_gma_to_bb_cmd(struct intel_shadow_bb_entry *entry_obj,
> - unsigned long add, int gmadr_bytes)
> -{
> - if (WARN_ON(gmadr_bytes != 4 && gmadr_bytes != 8))
> - return -1;
> -
> - *((u32 *)(entry_obj->bb_start_cmd_va + (1 << 2))) = add &
> - BATCH_BUFFER_ADDR_MASK;
> - if (gmadr_bytes == 8) {
> - *((u32 *)(entry_obj->bb_start_cmd_va + (2 << 2))) =
> - add & BATCH_BUFFER_ADDR_HIGH_MASK;
> - }
> -
> - return 0;
> -}
> -
> static void prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload)
> {
> - int gmadr_bytes = workload->vgpu->gvt->device_info.gmadr_bytes_in_cmd;
> + const int gmadr_bytes = workload->vgpu->gvt->device_info.gmadr_bytes_in_cmd;
> + struct intel_shadow_bb_entry *entry_obj;
>
> /* pin the gem object to ggtt */
> - if (!list_empty(&workload->shadow_bb)) {
> - struct intel_shadow_bb_entry *entry_obj =
> - list_first_entry(&workload->shadow_bb,
> - struct intel_shadow_bb_entry,
> - list);
> - struct intel_shadow_bb_entry *temp;
> + list_for_each_entry(entry_obj, &workload->shadow_bb, list) {
> + struct i915_vma *vma;
>
> - list_for_each_entry_safe(entry_obj, temp, &workload->shadow_bb,
> - list) {
> - struct i915_vma *vma;
> -
> - vma = i915_gem_object_ggtt_pin(entry_obj->obj, NULL, 0,
> - 4, 0);
> - if (IS_ERR(vma)) {
> - gvt_err("Cannot pin\n");
> - return;
> - }
> -
> - /* FIXME: we are not tracking our pinned VMA leaving it
> - * up to the core to fix up the stray pin_count upon
> - * free.
> - */
> -
> - /* update the relocate gma with shadow batch buffer*/
> - set_gma_to_bb_cmd(entry_obj,
> - i915_ggtt_offset(vma),
> - gmadr_bytes);
> + vma = i915_gem_object_ggtt_pin(entry_obj->obj, NULL, 0, 4, 0);
> + if (IS_ERR(vma)) {
> + gvt_err("Cannot pin\n");
> + return;
> }
> +
> + /* FIXME: we are not tracking our pinned VMA leaving it
> + * up to the core to fix up the stray pin_count upon
> + * free.
> + */
> +
> + /* update the relocate gma with shadow batch buffer*/
> + entry_obj->bb_start_cmd_va[1] = i915_ggtt_offset(vma);
> + if (gmadr_bytes == 8)
> + entry_obj->bb_start_cmd_va[2] = 0;
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.h b/drivers/gpu/drm/i915/gvt/scheduler.h
> index 3b30c28bff51..2833dfa8c9ae 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.h
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.h
> @@ -113,7 +113,7 @@ struct intel_shadow_bb_entry {
> struct drm_i915_gem_object *obj;
> void *va;
> unsigned long len;
> - void *bb_start_cmd_va;
> + u32 *bb_start_cmd_va;
> };
>
> #define workload_q_head(vgpu, ring_id) \
> --
> 2.11.0
>
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
