linux-kernel - Re: [PATCH v3 3/3] drm/i915/gt: document TLB cache invalidation functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0698c5a5-3bf2-daa4-e10e-2715f9b0d080@infradead.org>
Date:   Thu, 4 Aug 2022 12:00:22 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Jonathan Corbet <corbet@....net>
Cc:     Niranjana Vishwanathapura <niranjana.vishwanathapura@...el.com>,
        Andi Shyti <andi.shyti@...ux.intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        Chris Wilson <chris.p.wilson@...el.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        dri-devel@...ts.freedesktop.org, intel-gfx@...ts.freedesktop.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] drm/i915/gt: document TLB cache invalidation
 functions

Hi Mauro,

On 8/4/22 00:37, Mauro Carvalho Chehab wrote:
> Add a description for the TLB cache invalidation algorithm and for
> the related kAPI functions.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@...nel.org>
> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH v3 0/3] at: https://lore.kernel.org/all/cover.1659598090.git.mchehab@kernel.org/
> 
>  Documentation/gpu/i915.rst          |  7 ++
>  drivers/gpu/drm/i915/gt/intel_tlb.c | 25 ++++++++
>  drivers/gpu/drm/i915/gt/intel_tlb.h | 99 +++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
> 

> diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.c b/drivers/gpu/drm/i915/gt/intel_tlb.c
> index af8cae979489..16b918ffe824 100644
> --- a/drivers/gpu/drm/i915/gt/intel_tlb.c
> +++ b/drivers/gpu/drm/i915/gt/intel_tlb.c
> @@ -145,6 +145,18 @@ static void mmio_invalidate_full(struct intel_gt *gt)
>  	intel_uncore_forcewake_put_delayed(uncore, FORCEWAKE_ALL);
>  }
>  
> +/**
> + * intel_gt_invalidate_tlb_full - do full TLB cache invalidation
> + * @gt: GT structure

In multiple places (here and below) it would be nice to know what a
GT structure is. I looked thru multiple C and header files yesterday
and didn't find any comments about it.

Just saying that @gt is a GT structure isn't very helpful, other
than making kernel-doc shut up.

> + * @seqno: sequence number
> + *
> + * Do a full TLB cache invalidation if the @seqno is bigger than the last
> + * full TLB cache invalidation.
> + *
> + * Note:
> + * The TLB cache invalidation logic depends on GEN-specific registers.
> + * It currently supports MMIO-based TLB flush for GEN8 to GEN12.
> + */
>  void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32 seqno)
>  {
>  	intel_wakeref_t wakeref;
> @@ -171,12 +183,25 @@ void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32 seqno)
>  	}
>  }
>  
> +/**
> + * intel_gt_init_tlb - initialize TLB-specific vars
> + * @gt: GT structure
> + *
> + * TLB cache invalidation logic internally uses some resources that require
> + * initialization. Should be called before doing any TLB cache invalidation.
> + */
>  void intel_gt_init_tlb(struct intel_gt *gt)
>  {
>  	mutex_init(&gt->tlb.invalidate_lock);
>  	seqcount_mutex_init(&gt->tlb.seqno, &gt->tlb.invalidate_lock);
>  }
>  
> +/**
> + * intel_gt_fini_tlb - free TLB-specific vars
> + * @gt: GT structure
> + *
> + * Frees any resources needed by TLB cache invalidation logic.
> + */
>  void intel_gt_fini_tlb(struct intel_gt *gt)
>  {
>  	mutex_destroy(&gt->tlb.invalidate_lock);
> diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.h b/drivers/gpu/drm/i915/gt/intel_tlb.h
> index 46ce25bf5afe..2838c051f872 100644
> --- a/drivers/gpu/drm/i915/gt/intel_tlb.h
> +++ b/drivers/gpu/drm/i915/gt/intel_tlb.h
> @@ -11,16 +11,115 @@
>  
>  #include "intel_gt_types.h"
>  
> +/**
> + * DOC: TLB cache invalidation logic
> + *
...
> +
>  void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32 seqno);
>  
>  void intel_gt_init_tlb(struct intel_gt *gt);
>  void intel_gt_fini_tlb(struct intel_gt *gt);
>  
> +/**
> + * intel_gt_tlb_seqno - Returns the current TLB invlidation sequence number
> + * @gt: GT structure
> + *
> + * There's no need to lock while calling it, as seqprop_sequence is thread-safe
> + */
>  static inline u32 intel_gt_tlb_seqno(const struct intel_gt *gt)
>  {
>  	return seqprop_sequence(&gt->tlb.seqno);
>  }
>  
> +/**
> + * intel_gt_next_invalidate_tlb_full - Returns the next TLB full invalidation
> + *	sequence number
> + * @gt: GT structure
> + *
> + * There's no need to lock while calling it, as seqprop_sequence is thread-safe
> + */
>  static inline u32 intel_gt_next_invalidate_tlb_full(const struct intel_gt *gt)
>  {
>  	return intel_gt_tlb_seqno(gt) | 1;

thanks.

-- 
~Randy