Re: [PATCH v2 06/10] hw/cxl_type3: Add DC Region bitmap lock

[Fan Ni]

On Thu, May 08, 2025 at 12:01:02AM +0000, anisa.su...@gmail.com wrote:
> From: Anisa Su <anisa...@samsung.com>
> 
> Add a lock on the bitmap of each CXLDCRegion in preparation for the next
> patch which implements FMAPI Set DC Region Configuration. This command
> can modify the block size, which means the region's bitmap must be updated
> accordingly.
> 
> The lock becomes necessary when commands that add/release extents
> (meaning they update the bitmap too) are enabled on a different CCI than
> the CCI on which the FMAPI commands are enabled. 

read/write access also need to touch the blk_bitmap to ensure the range
to access is backed by DC blocks through ct3_test_region_block_backed().

Otherwise,


Reviewed-by: Fan Ni <fan...@samsung.com>




> 
> Signed-off-by: Anisa Su <anisa...@samsung.com>
> ---
>  hw/mem/cxl_type3.c          | 4 ++++
>  include/hw/cxl/cxl_device.h | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 6ad48f55ce..b5b3df5edf 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -824,6 +824,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, 
> Error **errp)
>          };
>          ct3d->dc.total_capacity += region->len;
>          region->blk_bitmap = bitmap_new(region->len / region->block_size);
> +        qemu_mutex_init(&region->bitmap_lock);
>      }
>      QTAILQ_INIT(&ct3d->dc.extents);
>      QTAILQ_INIT(&ct3d->dc.extents_pending);
> @@ -1176,6 +1177,7 @@ void ct3_set_region_block_backed(CXLType3Dev *ct3d, 
> uint64_t dpa,
>          return;
>      }
>  
> +    QEMU_LOCK_GUARD(&region->bitmap_lock);
>      bitmap_set(region->blk_bitmap, (dpa - region->base) / region->block_size,
>                 len / region->block_size);
>  }
> @@ -1202,6 +1204,7 @@ bool ct3_test_region_block_backed(CXLType3Dev *ct3d, 
> uint64_t dpa,
>       * if bits between [dpa, dpa + len) are all 1s, meaning the DPA range is
>       * backed with DC extents, return true; else return false.
>       */
> +    QEMU_LOCK_GUARD(&region->bitmap_lock);
>      return find_next_zero_bit(region->blk_bitmap, nr + nbits, nr) == nr + 
> nbits;
>  }
>  
> @@ -1223,6 +1226,7 @@ void ct3_clear_region_block_backed(CXLType3Dev *ct3d, 
> uint64_t dpa,
>  
>      nr = (dpa - region->base) / region->block_size;
>      nbits = len / region->block_size;
> +    QEMU_LOCK_GUARD(&region->bitmap_lock);
>      bitmap_clear(region->blk_bitmap, nr, nbits);
>  }
>  
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index cbcc1bc9f5..9cfd9c5a9f 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -618,6 +618,7 @@ typedef struct CXLDCRegion {
>      uint8_t flags;
>      unsigned long *blk_bitmap;
>      uint64_t supported_blk_size_bitmask;
> +    QemuMutex bitmap_lock;
>      /* Following bools make up dsmas flags, as defined in the CDAT */
>      bool nonvolatile;
>      bool sharable;
> -- 
> 2.47.2
> 

-- 
Fan Ni