Re: [RFC PATCH] cxl/region: Allow out of order assembly of autodiscovered regions

[Dan Williams <dan.j.williams@intel.com>]

Alison Schofield wrote:
> 
> On Thu, Jan 18, 2024 at 11:46:44AM -0800, Dan Williams wrote:
> 
> - snip to decoder replay patch -
> 
> > 
> > So one of the largest roadblocks for creating arbitrary region assembly
> > scenarios with cxl_test is the inability to save and restore decoder
> > settings.
> > 
> > The patch below adds support for recording decoder settings and skipping
> > the reset of those values when unloading the driver. Then, when the
> > driver is reloaded, it simulates the case of BIOS created CXL regions
> > prior to OS boot.
> > 
> > We can go after even finer grained cases once the uunit effort settles,
> > but in the meantime cxl_test can add auto-assembly regression tests with
> > the current topology.
> > 
> > With the below you can simply trigger "cxl {en,dis}able-memdev" in the
> > proper order to cause the violation.
> > 
> > -- >8 --
> > From 5839392a3dbb64dbbac0630d930b1f48a8ef7ea6 Mon Sep 17 00:00:00 2001
> > From: Dan Williams <dan.j.williams@intel.com>
> > Date: Wed, 17 Jan 2024 20:56:20 -0800
> > Subject: [PATCH] tools/testing/cxl: Add decoder save/restore support
> > 
> > Record decoder values at init and mock_decoder_commit() time, and
> > restore them at the next invocation of mock_init_hdm_decoder(). Add 2
> > attributes to the cxl_test "cxl_acpi" device to optionally flush the
> > cache of topology decoder values, or disable updating the decoder at
> > mock_decoder_reset() time.
> > 
> > This enables replaying a saved decoder configuration when re-triggering
> > a topology scan by re-binding the cxl_acpi driver to "cxl_acpi.0" (the
> > cxl_test emulation of an ACPI0017 instance).
> 
> Hi Dan,
> 
> Sorry it's taken a while to come back on this. I did find it useful
> in testing the auto-assembly order issue as you suggested. 
> 
> I didn't use this one: &dev_attr_decoder_registry_invalidate.attr,
> I just reloaded the cxl-test module to do same.

Makes sense, that can go.

> This I used:  &dev_attr_decoder_registry_reset_disable.attr,
> with your decoders state fixup to set CXL_DECODER_STATE_AUTO,
> and a work-around to avoid pmem_probe failures on pmem region
> auto create. 
> 
> More generally, I'm wondering about the implementation of the
> 'registry_save'. Here it continuously updates during all
> cxl-test usage. Did you consider only creating the registry upon
> user request and then at the next mock_init_hmd_decoder() look
> for and use that registry if it exists.
> 
> Usage would be: 
> # echo 1 > /sys/bus/platform/devices/cxl_acpi.0/decoder_registry_create

You mean wait to snapshot decoder state when triggered?

> # echo cxl_acpi.0 > /sys/bus/platform/drivers/cxl_acpi/unbind
> # echo cxl_acpi.0 > /sys/bus/platform/drivers/cxl_acpi/bind
> 
> And then maybe, for folks who like to acpi/unbind,bind, rather
> than reload module, we could offer a decoder_registry_remove attr.
> 
> Am I missing something regarding the need to keep it updated
> on the fly? 

I can see it being an alternate way, but not sure how to weigh one
approach vs the other. Is the dynamic update getting in the way of a
test case you are thinking about? Otherwise it seemed easy to reason
that the registry is always on, but only takes effect when
registry_reset_disable is set, and cxl_acpi rebinds the test device.