[PATCH v12 3/5] drm/xe/guc: Add capture size check in GuC log buffer

Teres Alexis, Alan Previn alan.previn.teres.alexis at intel.com
Mon Jul 15 21:40:46 UTC 2024


On Fri, 2024-07-12 at 09:47 -0700, Zhanjun Dong wrote:
> The capture-nodes is included in GuC log buffer, add the size check
> for capture region in the whole GuC log buffer.
> Add capture output size check before allocating the shared buffer.
> 
> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
> ---
>  drivers/gpu/drm/xe/abi/guc_log_abi.h  |  20 ++++
> 
alan:snip
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index a37ee3419428..8fecb528b92c 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -9,9 +9,64 @@
>  
>  #include "xe_bo.h"
>  #include "xe_gt.h"
> +#include "xe_gt_printk.h"
> +#include "xe_guc.h"
>  #include "xe_map.h"
>  #include "xe_module.h"
>  
> +#define GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE      CRASH_BUFFER_SIZE
> +#define GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE      DEBUG_BUFFER_SIZE
> +#define GUC_LOG_DEFAULT_CAPTURE_BUFFER_SIZE    CAPTURE_BUFFER_SIZE
> +
> +#define GUC_LOG_CALC_UNIT(size)                (((size) % SZ_1M) == 0 ? SZ_1M : SZ_4K)
> +#define GUC_LOG_CALC_FLAG(size)                (((size) % SZ_1M) == 0 ? GUC_LOG_LOG_ALLOC_UNITS : 0)
> +
> +/* Size is +1 unit */
> +#define GUC_LOG_CALC_COUNT(size)       ((size) / GUC_LOG_CALC_UNIT(size) - 1)
> +
> +static const struct xe_log_sizes_type log_sizes[GUC_LOG_BUFFER_TYPE_MAX] = {
> +       {
> +               GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE,
> +               GUC_LOG_CALC_UNIT(GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE),
> +               GUC_LOG_CALC_COUNT(GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE),
> +               GUC_LOG_CALC_FLAG(GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE)
> +       },
> +       {
> +               GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE,
> +               GUC_LOG_CALC_UNIT(GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE),
> +               GUC_LOG_CALC_COUNT(GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE),
> +               GUC_LOG_CALC_FLAG(GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE)
> +       },
> +       {
> +               GUC_LOG_DEFAULT_CAPTURE_BUFFER_SIZE,
> +               GUC_LOG_CALC_UNIT(GUC_LOG_DEFAULT_CAPTURE_BUFFER_SIZE),
> +               GUC_LOG_CALC_COUNT(GUC_LOG_DEFAULT_CAPTURE_BUFFER_SIZE),
> +               GUC_LOG_CALC_FLAG(GUC_LOG_DEFAULT_CAPTURE_BUFFER_SIZE)
> +       }
> +};
> +
> +/* Zero is not allowed for crash/debug/capture size */
> +static_assert(GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE);
> +static_assert(GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE);
> +static_assert(GUC_LOG_DEFAULT_CAPTURE_BUFFER_SIZE);
> +
> +/* crash unit must equal to debug unit */
> +static_assert(GUC_LOG_CALC_UNIT(GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE) ==
> +             GUC_LOG_CALC_UNIT(GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE));
> +
> +/* crash/debug/capture count must less than mask range */
> +static_assert(GUC_LOG_CALC_COUNT(GUC_LOG_DEFAULT_CRASH_BUFFER_SIZE) <=
> +             GUC_LOG_CRASH_MASK >> GUC_LOG_CRASH_SHIFT);
> +static_assert(GUC_LOG_CALC_COUNT(GUC_LOG_DEFAULT_DEBUG_BUFFER_SIZE) <=
> +             GUC_LOG_DEBUG_MASK >> GUC_LOG_DEBUG_SHIFT);
> +static_assert(GUC_LOG_CALC_COUNT(GUC_LOG_DEFAULT_CAPTURE_BUFFER_SIZE) <=
> +             GUC_LOG_CAPTURE_MASK >> GUC_LOG_CAPTURE_SHIFT);
> +
alan: as per offline conversation and in line with my review comments from v11,
you don't need to add any of these build time checks because guc_ctl_log_params_flags
already does this - i just wanted you to check that existing code is still
good for the increased capture region size.


> +static inline struct xe_guc *log_to_guc(struct xe_guc_log *log)
> +{
> +       return container_of(log, struct xe_guc, log);
> +}
> +
>  static struct xe_gt *
>  log_to_gt(struct xe_guc_log *log)
>  {
> @@ -80,7 +135,8 @@ void xe_guc_log_print(struct xe_guc_log *log, struct drm_printer *p)
>  int xe_guc_log_init(struct xe_guc_log *log)
>  {
>         struct xe_device *xe = log_to_xe(log);
> -       struct xe_tile *tile = gt_to_tile(log_to_gt(log));
> +       struct xe_gt *gt = log_to_gt(log);
> +       struct xe_tile *tile = gt_to_tile(gt);
>         struct xe_bo *bo;
>  
>         bo = xe_managed_bo_create_pin_map(xe, tile, guc_log_size(),
> @@ -96,3 +152,86 @@ int xe_guc_log_init(struct xe_guc_log *log)
>  
>         return 0;
>  }
> +
> +/**
> + * xe_guc_log_section_size_capture - Get capture buffer size in log sections.
> + * @log: The log object.
> + *
> + * This function will return the capture buffer size in log sections.
> + *
> + * Return: capture buffer size.
> + */
> +u32 xe_guc_log_section_size_capture(struct xe_guc_log *log)
> +{
> +       return log_sizes[GUC_LOG_BUFFER_CAPTURE].bytes;
> +}
> +
> +/**
> + * xe_guc_check_log_buf_overflow - Check if log buffer overflowed
> + * @log: The log object.
> + * @type: The log buffer type
> + * @full_cnt: The count of buffer full
> + *
> + * This function will check count of buffer full against previous, mismatch
> + * indicate overflowed.
> + * Update the sampled_overflow counter, if the 4 bit counter overflowed, add
> + * up 16 to correct the value.
> + *
> + * Return: True if overflowed.
> + */
> +bool xe_guc_check_log_buf_overflow(struct xe_guc_log *log, enum guc_log_buffer_type type,
> +                                  unsigned int full_cnt)
> +{
> +       unsigned int prev_full_cnt = log->stats[type].sampled_overflow;
> +       bool overflow = false;
> +
> +       if (full_cnt != prev_full_cnt) {
> +               overflow = true;
> +
> +               log->stats[type].overflow = full_cnt;
> +               log->stats[type].sampled_overflow += full_cnt - prev_full_cnt;
> +
> +               if (full_cnt < prev_full_cnt) {
> +                       /* buffer_full_cnt is a 4 bit counter */
> +                       log->stats[type].sampled_overflow += 16;
> +               }
> +               xe_gt_notice(log_to_gt(log), "log buffer overflow\n");
> +       }
> +
> +       return overflow;
> +}
> +
> +/**
> + * xe_guc_get_log_buffer_size - Get log buffer size for a type.
> + * @log: The log object.
> + * @type: The log buffer type
> + *
> + * Return: buffer size.
> + */
> +u32 xe_guc_get_log_buffer_size(struct xe_guc_log *log, enum guc_log_buffer_type type)
> +{
> +       xe_gt_assert(log_to_gt(log), type < GUC_LOG_BUFFER_TYPE_MAX);
> +       return log_sizes[type].bytes;
> +}
> +
> +/**
> + * xe_guc_get_log_buffer_offset - Get offset in log buffer for a type.
> + * @log: The log object.
> + * @type: The log buffer type
> + *
> + * This function will return the offset in the log buffer for a type.
> + * Return: buffer offset.
> + */
> +u32 xe_guc_get_log_buffer_offset(struct xe_guc_log *log, enum guc_log_buffer_type type)
> +{
> +       enum guc_log_buffer_type i;
> +       u32 offset = PAGE_SIZE;/* for the log_buffer_states */
> +
> +       for (i = GUC_LOG_BUFFER_CRASH_DUMP; i < GUC_LOG_BUFFER_TYPE_MAX; ++i) {
> +               if (i == type)
> +                       break;
> +               offset += xe_guc_get_log_buffer_size(log, i);
> +       }
> +
> +       return offset;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.h b/drivers/gpu/drm/xe/xe_guc_log.h
> index 2d25ab28b4b3..1d3b123cebd3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.h
> +++ b/drivers/gpu/drm/xe/xe_guc_log.h
> @@ -7,6 +7,7 @@
>  #define _XE_GUC_LOG_H_
>  
>  #include "xe_guc_log_types.h"
> +#include "xe_guc_types.h"
>  
>  struct drm_printer;
>  
> @@ -17,7 +18,7 @@ struct drm_printer;
>  #else
>  #define CRASH_BUFFER_SIZE      SZ_8K
>  #define DEBUG_BUFFER_SIZE      SZ_64K
> -#define CAPTURE_BUFFER_SIZE    SZ_16K
> +#define CAPTURE_BUFFER_SIZE    SZ_1M
>  #endif
>  /*
>   * While we're using plain log level in i915, GuC controls are much more...
> @@ -45,4 +46,13 @@ xe_guc_log_get_level(struct xe_guc_log *log)
>         return log->level;
>  }
>  
> +u32 xe_guc_log_section_size_capture(struct xe_guc_log *log);
> +
> +bool xe_guc_check_log_buf_overflow(struct xe_guc_log *log,
> +                                  enum guc_log_buffer_type type,
> +                                  unsigned int full_cnt);
> +
> +u32 xe_guc_get_log_buffer_size(struct xe_guc_log *log, enum guc_log_buffer_type type);
> +u32 xe_guc_get_log_buffer_offset(struct xe_guc_log *log, enum guc_log_buffer_type type);
> +
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_guc_log_types.h b/drivers/gpu/drm/xe/xe_guc_log_types.h
> index 125080d138a7..e0122aa99aa1 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log_types.h
> +++ b/drivers/gpu/drm/xe/xe_guc_log_types.h
> @@ -7,9 +7,18 @@
>  #define _XE_GUC_LOG_TYPES_H_
>  
>  #include <linux/types.h>
> +#include "abi/guc_log_abi.h"
>  
>  struct xe_bo;
>  
> +/** @sizes: Allocation settings */
> +struct xe_log_sizes_type {
> +       u32 bytes;      /* Size in bytes */
> +       u32 units;      /* GuC API units - 1MB or 4KB */
> +       u32 count;      /* Number of API units */
> +       u32 flag;       /* GuC API units flag */
> +};
> +
>  /**
>   * struct xe_guc_log - GuC log
>   */
> @@ -18,6 +27,14 @@ struct xe_guc_log {
>         u32 level;
>         /** @bo: XE BO for GuC log */
>         struct xe_bo *bo;
> +       /** @sizes: Allocation settings */
> +       struct xe_log_sizes_type sizes[GUC_LOG_BUFFER_TYPE_MAX];
> +       /** @stats: logging related stats */
> +       struct {
> +               u32 sampled_overflow;
> +               u32 overflow;
> +               u32 flush;
> +       } stats[GUC_LOG_BUFFER_TYPE_MAX];
>  };
>  
>  #endif



More information about the Intel-xe mailing list