Re: [PATCH v7 4/7] drm/xe/guc: Check sizing of guc_capture output

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

On Wed, 2024-03-27 at 13:40 -0700, Zhanjun Dong wrote:
> Add capture output size check function to provide a reasonable
> minimum size for error capture region before allocating the shared
> buffer.
> 
alan:snip

> +static int
> +guc_capture_output_min_size_est(struct xe_guc *guc)
> +{
> +       struct xe_gt *gt = guc_to_gt(guc);
> +       struct xe_hw_engine *hwe;
> +       enum xe_hw_engine_id id;
alan:snip

> +       for_each_hw_engine(hwe, gt, id) {
alan: please verify that for xe-kmd this code actually produces the full list of
engines and not an empty mask (i think i915 worked differently, we had an
initial full mask).
alan:snip

> +/*
> + * Add on a 3x multiplier to allow for multiple back-to-back captures occurring
> + * before the i915 can read the data out and process it
alan: %s/i915/xe/g
alan:snip

> diff --git a/drivers/gpu/drm/xe/xe_guc_capture_fwif.h b/drivers/gpu/drm/xe/xe_guc_capture_fwif.h
> index 4bb94ac1ff48..b975a65b64e7 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture_fwif.h
> +++ b/drivers/gpu/drm/xe/xe_guc_capture_fwif.h
> @@ -10,6 +10,51 @@
>  #include "regs/xe_reg_defs.h"
>  #include "xe_guc_fwif.h"
alan: all of the changes in this file should be moved to patch #5.
 
> +/*
> + * struct __guc_capture_bufstate
> + *
> + * Book-keeping structure used to track read and write pointers
> + * as we extract error capture data from the GuC-log-buffer's
> + * error-capture region as a stream of dwords.
> + */
> +struct __guc_capture_bufstate {
> 
alan:snip

> +/*
> + * struct __guc_capture_parsed_output - extracted error capture node
> + *
> + * A single unit of extracted error-capture output data grouped together
> + * at an engine-instance level. We keep these nodes in a linked list.
> + * See cachelist and outlist below.
> + */
> +struct __guc_capture_parsed_output {
alan:snip