Re: [PATCH 17/20] drm/xe: reset mmio mappings with devm

[Andrzej Hajda <andrzej.hajda@intel.com>]

On 10.05.2024 20:12, Matthew Auld wrote:
> Set our various mmio mappings to NULL. This should make it easier to
> catch something rogue trying to mess with mmio after device removal. For
> example, we might unmap everything and then start hitting some mmio
> address which has already been unmamped by us and then remapped by
> something else, causing all kinds of carnage.
> 
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej
> ---
>   drivers/gpu/drm/xe/xe_device.c |  4 +++-
>   drivers/gpu/drm/xe/xe_mmio.c   | 35 ++++++++++++++++++++++++++++------
>   drivers/gpu/drm/xe/xe_mmio.h   |  2 +-
>   3 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 40983ad66e3c..ce8bccbb06ba 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -554,7 +554,9 @@ int xe_device_probe(struct xe_device *xe)
>   	if (err)
>   		return err;
>   
> -	xe_mmio_probe_tiles(xe);
> +	err = xe_mmio_probe_tiles(xe);
> +	if (err)
> +		return err;
>   
>   	xe_ttm_sys_mgr_init(xe);
>   
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index a3094e741db8..0ad840bab8f3 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -255,6 +255,21 @@ static int xe_mmio_tile_vram_size(struct xe_tile *tile, u64 *vram_size,
>   	return xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>   }
>   
> +static void vram_fini(void *arg)
> +{
> +	struct xe_device *xe = arg;
> +	struct xe_tile *tile;
> +	int id;
> +
> +	if (xe->mem.vram.mapping)
> +		iounmap(xe->mem.vram.mapping);
> +
> +	xe->mem.vram.mapping = NULL;
> +
> +	for_each_tile(tile, xe, id)
> +		tile->mem.vram.mapping = NULL;
> +}
> +
>   int xe_mmio_probe_vram(struct xe_device *xe)
>   {
>   	struct xe_tile *tile;
> @@ -331,10 +346,20 @@ int xe_mmio_probe_vram(struct xe_device *xe)
>   	drm_info(&xe->drm, "Available VRAM: %pa, %pa\n", &xe->mem.vram.io_start,
>   		 &available_size);
>   
> -	return 0;
> +	return devm_add_action_or_reset(xe->drm.dev, vram_fini, xe);
>   }
>   
> -void xe_mmio_probe_tiles(struct xe_device *xe)
> +static void tiles_fini(void *arg)
> +{
> +	struct xe_device *xe = arg;
> +	struct xe_tile *tile;
> +	int id;
> +
> +	for_each_tile(tile, xe, id)
> +		tile->mmio.regs = NULL;
> +}
> +
> +int xe_mmio_probe_tiles(struct xe_device *xe)
>   {
>   	size_t tile_mmio_size = SZ_16M, tile_mmio_ext_size = xe->info.tile_mmio_ext_size;
>   	u8 id, tile_count = xe->info.tile_count;
> @@ -385,6 +410,8 @@ void xe_mmio_probe_tiles(struct xe_device *xe)
>   			regs += tile_mmio_ext_size;
>   		}
>   	}
> +
> +	return devm_add_action_or_reset(xe->drm.dev, tiles_fini, xe);
>   }
>   
>   static void mmio_fini(void *arg)
> @@ -392,10 +419,6 @@ static void mmio_fini(void *arg)
>   	struct xe_device *xe = arg;
>   
>   	pci_iounmap(to_pci_dev(xe->drm.dev), xe->mmio.regs);
> -	if (xe->mem.vram.mapping)
> -		iounmap(xe->mem.vram.mapping);
> -
> -	xe->mem.vram.mapping = NULL;
>   	xe->mmio.regs = NULL;
>   }
>   
> diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
> index 445ec6a0753e..6488705c1ffe 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.h
> +++ b/drivers/gpu/drm/xe/xe_mmio.h
> @@ -21,7 +21,7 @@ struct xe_device;
>   #define LMEM_BAR		2
>   
>   int xe_mmio_init(struct xe_device *xe);
> -void xe_mmio_probe_tiles(struct xe_device *xe);
> +int xe_mmio_probe_tiles(struct xe_device *xe);
>   
>   u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg);
>   u16 xe_mmio_read16(struct xe_gt *gt, struct xe_reg reg);