Re: [PATCH v3 25/25] tools/testing/cxl: Add DC Regions to mock mem data

[Ira Weiny]

Jonathan Cameron wrote:
> On Fri, 16 Aug 2024 09:44:33 -0500
> Ira Weiny <ira.weiny@xxxxxxxxx> wrote:
> 
> > cxl_test provides a good way to ensure quick smoke and regression
> > testing.  The complexity of Dynamic Capacity (DC) extent processing as
> > well as the complexity of the new sparse DAX regions can mostly be
> > tested through cxl_test.  This includes management of sparse regions and
> > DAX devices on those regions; the management of extent device lifetimes;
> > and the processing of DCD events.
> > 
> > The only missing functionality from this test is actual interrupt
> > processing.
> > 
> > Mock memory devices can easily mock DC information and manage fake
> > extent data.
> > 
> > Define mock_dc_region information within the mock memory data.  Add
> > sysfs entries on the mock device to inject and delete extents.
> > 
> > The inject format is <start>:<length>:<tag>:<more_flag>
> > The delete format is <start>:<length>
> > 
> > Directly call the event irq callback to simulate irqs to process the
> > test extents.
> > 
> > Add DC mailbox commands to the CEL and implement those commands.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> 
> Minor stuff inline.
> 
> Thanks,
> 
> Jonathan
> 
> > +static int mock_get_dc_config(struct device *dev,
> > +			      struct cxl_mbox_cmd *cmd)
> > +{
> > +	struct cxl_mbox_get_dc_config_in *dc_config = cmd->payload_in;
> > +	struct cxl_mockmem_data *mdata = dev_get_drvdata(dev);
> > +	u8 region_requested, region_start_idx, region_ret_cnt;
> > +	struct cxl_mbox_get_dc_config_out *resp;
> > +	int i;
> > +
> > +	region_requested = dc_config->region_count;
> > +	if (region_requested > NUM_MOCK_DC_REGIONS)
> > +		region_requested = NUM_MOCK_DC_REGIONS;
> 
> 	region_requested = min(...)

Sure.

> 
> > +
> > +	if (cmd->size_out < struct_size(resp, region, region_requested))
> > +		return -EINVAL;
> > +
> > +	memset(cmd->payload_out, 0, cmd->size_out);
> > +	resp = cmd->payload_out;
> > +
> > +	region_start_idx = dc_config->start_region_index;
> > +	region_ret_cnt = 0;
> > +	for (i = 0; i < NUM_MOCK_DC_REGIONS; i++) {
> > +		if (i >= region_start_idx) {
> > +			memcpy(&resp->region[region_ret_cnt],
> > +				&mdata->dc_regions[i],
> > +				sizeof(resp->region[region_ret_cnt]));
> > +			region_ret_cnt++;
> > +		}
> > +	}
> > +	resp->avail_region_count = NUM_MOCK_DC_REGIONS;
> > +	resp->regions_returned = i;
> > +
> > +	dev_dbg(dev, "Returning %d dc regions\n", region_ret_cnt);
> > +	return 0;
> > +}
> 
> 
> 
> > +static void cxl_mock_mem_remove(struct platform_device *pdev)
> > +{
> > +	struct cxl_mockmem_data *mdata = dev_get_drvdata(&pdev->dev);
> > +	struct cxl_memdev_state *mds = mdata->mds;
> > +
> > +	dev_dbg(mds->cxlds.dev, "Removing extents\n");
> 
> Clean this up as it doesn't do anything!

Opps...  Must have been some previous xarray thing.  Or perhaps I'm leaking
some memory here?  I'll double check.

> 
> > +}
> > +
> 
> > @@ -1689,14 +2142,261 @@ static ssize_t sanitize_timeout_store(struct device *dev,
> >  
> >  	return count;
> >  }
> > -
> Grump ;)  No whitespace changes in a patch doing anything 'useful'.
> >  static DEVICE_ATTR_RW(sanitize_timeout);
> >  
> 
> > +static int log_dc_event(struct cxl_mockmem_data *mdata, enum dc_event type,
> > +			u64 start, u64 length, const char *tag_str, bool more)
> > +{
> > +	struct device *dev = mdata->mds->cxlds.dev;
> > +	struct cxl_test_dcd *dcd_event;
> > +
> > +	dev_dbg(dev, "mock device log event %d\n", type);
> > +
> > +	dcd_event = devm_kmemdup(dev, &dcd_event_rec_template,
> > +				     sizeof(*dcd_event), GFP_KERNEL);
> > +	if (!dcd_event)
> > +		return -ENOMEM;
> > +
> > +	dcd_event->rec.flags = 0;
> > +	if (more)
> > +		dcd_event->rec.flags |= CXL_DCD_EVENT_MORE;
> > +	dcd_event->rec.event_type = type;
> > +	dcd_event->rec.extent.start_dpa = cpu_to_le64(start);
> > +	dcd_event->rec.extent.length = cpu_to_le64(length);
> > +	memcpy(dcd_event->rec.extent.tag, tag_str,
> > +	       min(sizeof(dcd_event->rec.extent.tag),
> > +		   strlen(tag_str)));
> > +
> > +	mes_add_event(mdata, CXL_EVENT_TYPE_DCD,
> > +		      (struct cxl_event_record_raw *)dcd_event);
> I guess this is where the missing event in previous patch come from.
> 
> Increment the number here, not back in that patch.

No that patch needed to pass the counts correctly for the event test.  DCD
event records have nothing to do with that.

I've cleaned up the patch.

Ira