Re: [PATCH 3/5] tools/testing/cxl: Mock the Inject Poison mailbox command

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Tue, 29 Nov 2022 20:34:35 -0800
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> Mock the injection of poison by storing the device:address entry in a
> cxl_test array: mock_poison[]. Limit the array to 64 entries and fail
> new inject requests when full.
> 
> Introducing the mock_poison[] list here, makes it available for use in
> the mock of Clear Poison, and the mock of Get Poison List.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Main question I have here is whether we want to mock per device injected poison
lists, or just one global.  I think we want per device so we can reflect
the limits as would be retrieved from Identify Memory Device Output Payload.
Whilst we don't do anything useful with it yet, we should also update
the mocked response to that command to reflect this.

Perhaps we should have a sysfs attribute to read how many entries we can
inject?  Seems like that would be useful for testing flows on real devices,
particularly if the device only supports a very small number!

> ---
>  tools/testing/cxl/test/mem.c | 53 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index a4f81915ec03..98acb9a644df 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -13,6 +13,7 @@
>  #define LSA_SIZE SZ_128K
>  #define DEV_SIZE SZ_2G
>  #define EFFECT(x) (1U << x)
> +#define MOCK_INJECT_POISON_MAX 64
>  
>  static struct cxl_cel_entry mock_cel[] = {
>  	{
> @@ -43,6 +44,10 @@ static struct cxl_cel_entry mock_cel[] = {
>  		.opcode = cpu_to_le16(CXL_MBOX_OP_GET_POISON),
>  		.effect = cpu_to_le16(0),
>  	},
> +	{
> +		.opcode = cpu_to_le16(CXL_MBOX_OP_INJECT_POISON),
> +		.effect = cpu_to_le16(0),
> +	},
>  };
>  
>  /* See CXL 2.0 Table 181 Get Health Info Output Payload */
> @@ -210,6 +215,51 @@ static int mock_health_info(struct cxl_dev_state *cxlds,
>  	return 0;
>  }
>  
> +static struct mock_poison {
> +	struct cxl_dev_state *cxlds;
> +	u64 dpa;
> +} mock_poison[MOCK_INJECT_POISON_MAX];

Don't we want one of these per device instance?

> +
> +static bool mock_poison_add(struct cxl_dev_state *cxlds, u64 dpa)
> +{
> +	for (int i = 0; i < MOCK_INJECT_POISON_MAX; i++) {
> +		if (!mock_poison[i].cxlds) {
> +			mock_poison[i].cxlds = cxlds;
> +			mock_poison[i].dpa = dpa;
> +			return true;
> +		}
> +	}
> +	dev_dbg(cxlds->dev, "Mock poison list full: %d\n",
> +		MOCK_INJECT_POISON_MAX);
Slightly nicer maybe to use the text from
https://elixir.bootlin.com/linux/latest/source/drivers/cxl/cxlmem.h#L128
and have: "Mock poison injection limit has been reached: %d\n" ..

> +	return false;
> +}