Re: [PATCH v7 05/12] hw/mem/cxl-type3: Refactor ct3_build_cdat_entries_for_mr to take mr size instead of mr as argument

[Gregory Price]

On Thu, Apr 18, 2024 at 04:10:56PM -0700, nifan@gmail.com wrote:
> From: Fan Ni 
> 
> The function ct3_build_cdat_entries_for_mr only uses size of the passed
> memory region argument, refactor the function definition to make the passed
> arguments more specific.
> 
> Reviewed-by: Jonathan Cameron 
> Signed-off-by: Fan Ni 
> ---
>  hw/mem/cxl_type3.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 5ceed0ab4c..a1fe268560 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -44,7 +44,7 @@ enum {
>  };

Reviewed-by: Gregory Price 

[PATCH v7 05/12] hw/mem/cxl-type3: Refactor ct3_build_cdat_entries_for_mr to take mr size instead of mr as argument

[nifan . cxl]

From: Fan Ni 

The function ct3_build_cdat_entries_for_mr only uses size of the passed
memory region argument, refactor the function definition to make the passed
arguments more specific.

Reviewed-by: Jonathan Cameron 
Signed-off-by: Fan Ni 
---
 hw/mem/cxl_type3.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 5ceed0ab4c..a1fe268560 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -44,7 +44,7 @@ enum {
 };
 
 static void ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table,
-  int dsmad_handle, MemoryRegion *mr,
+  int dsmad_handle, uint64_t size,
   bool is_pmem, uint64_t dpa_base)
 {
 CDATDsmas *dsmas;
@@ -63,7 +63,7 @@ static void ct3_build_cdat_entries_for_mr(CDATSubHeader 
**cdat_table,
 .DSMADhandle = dsmad_handle,
 .flags = is_pmem ? CDAT_DSMAS_FLAG_NV : 0,
 .DPA_base = dpa_base,
-.DPA_length = memory_region_size(mr),
+.DPA_length = size,
 };
 
 /* For now, no memory side cache, plausiblish numbers */
@@ -132,7 +132,7 @@ static void ct3_build_cdat_entries_for_mr(CDATSubHeader 
**cdat_table,
  */
 .EFI_memory_type_attr = is_pmem ? 2 : 1,
 .DPA_offset = 0,
-.DPA_length = memory_region_size(mr),
+.DPA_length = size,
 };
 
 /* Header always at start of structure */
@@ -149,6 +149,7 @@ static int ct3_build_cdat_table(CDATSubHeader 
***cdat_table, void *priv)
 g_autofree CDATSubHeader **table = NULL;
 CXLType3Dev *ct3d = priv;
 MemoryRegion *volatile_mr = NULL, *nonvolatile_mr = NULL;
+uint64_t vmr_size = 0, pmr_size = 0;
 int dsmad_handle = 0;
 int cur_ent = 0;
 int len = 0;
@@ -163,6 +164,7 @@ static int ct3_build_cdat_table(CDATSubHeader 
***cdat_table, void *priv)
 return -EINVAL;
 }
 len += CT3_CDAT_NUM_ENTRIES;
+vmr_size = memory_region_size(volatile_mr);
 }
 
 if (ct3d->hostpmem) {
@@ -171,21 +173,22 @@ static int ct3_build_cdat_table(CDATSubHeader 
***cdat_table, void *priv)
 return -EINVAL;
 }
 len += CT3_CDAT_NUM_ENTRIES;
+pmr_size = memory_region_size(nonvolatile_mr);
 }
 
 table = g_malloc0(len * sizeof(*table));
 
 /* Now fill them in */
 if (volatile_mr) {
-ct3_build_cdat_entries_for_mr(table, dsmad_handle++, volatile_mr,
+ct3_build_cdat_entries_for_mr(table, dsmad_handle++, vmr_size,
   false, 0);
 cur_ent = CT3_CDAT_NUM_ENTRIES;
 }
 
 if (nonvolatile_mr) {
-uint64_t base = volatile_mr ? memory_region_size(volatile_mr) : 0;
+uint64_t base = vmr_size;
 ct3_build_cdat_entries_for_mr(&(table[cur_ent]), dsmad_handle++,
-  nonvolatile_mr, true, base);
+  pmr_size, true, base);
 cur_ent += CT3_CDAT_NUM_ENTRIES;
 }
 assert(len == cur_ent);
-- 
2.43.0