* Re: [PATCH] cxl/port: Fix cxl_test register enumeration regression
2023-09-15 8:07 ` [PATCH] cxl/port: Fix cxl_test register enumeration regression Dan Williams
@ 2023-09-15 16:20 ` Dave Jiang
2023-09-26 19:16 ` Jim Harris
1 sibling, 0 replies; 3+ messages in thread
From: Dave Jiang @ 2023-09-15 16:20 UTC (permalink / raw)
To: Dan Williams, linux-cxl; +Cc: stable, Alison Schofield
On 9/15/23 01:07, Dan Williams wrote:
> The cxl_test unit test environment models a CXL topology for
> sysfs/user-ABI regression testing. It uses interface mocking via the
> "--wrap=" linker option to redirect cxl_core routines that parse
> hardware registers with versions that just publish objects, like
> devm_cxl_enumerate_decoders().
>
> Starting with:
>
> Commit 19ab69a60e3b ("cxl/port: Store the port's Component Register mappings in struct cxl_port")
>
> ...port register enumeration is moved into devm_cxl_add_port(). This
> conflicts with the "cxl_test avoids emulating registers stance" so
> either the port code needs to be refactored (too violent), or modified
> so that register enumeration is skipped on "fake" cxl_test ports
> (annoying, but straightforward).
>
> This conflict has happened previously and the "check for platform
> device" workaround to avoid instrusive refactoring was deployed in those
> scenarios. In general, refactoring should only benefit production code,
> test code needs to remain minimally instrusive to the greatest extent
> possible.
>
> This was missed previously because it may sometimes just cause warning
> messages to be emitted, but it can also cause test failures. The
> backport to -stable is only nice to have for clean cxl_test runs.
>
> Fixes: 19ab69a60e3b ("cxl/port: Store the port's Component Register mappings in struct cxl_port")
> Cc: <stable@vger.kernel.org>
> Reported-by: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Tested-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/cxl/core/port.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 724be8448eb4..7ca01a834e18 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
> +#include <linux/platform_device.h>
> #include <linux/memregion.h>
> #include <linux/workqueue.h>
> #include <linux/debugfs.h>
> @@ -706,16 +707,20 @@ static int cxl_setup_comp_regs(struct device *dev, struct cxl_register_map *map,
> return cxl_setup_regs(map);
> }
>
> -static inline int cxl_port_setup_regs(struct cxl_port *port,
> - resource_size_t component_reg_phys)
> +static int cxl_port_setup_regs(struct cxl_port *port,
> + resource_size_t component_reg_phys)
> {
> + if (dev_is_platform(port->uport_dev))
> + return 0;
> return cxl_setup_comp_regs(&port->dev, &port->comp_map,
> component_reg_phys);
> }
>
> -static inline int cxl_dport_setup_regs(struct cxl_dport *dport,
> - resource_size_t component_reg_phys)
> +static int cxl_dport_setup_regs(struct cxl_dport *dport,
> + resource_size_t component_reg_phys)
> {
> + if (dev_is_platform(dport->dport_dev))
> + return 0;
> return cxl_setup_comp_regs(dport->dport_dev, &dport->comp_map,
> component_reg_phys);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] cxl/port: Fix cxl_test register enumeration regression
2023-09-15 8:07 ` [PATCH] cxl/port: Fix cxl_test register enumeration regression Dan Williams
2023-09-15 16:20 ` Dave Jiang
@ 2023-09-26 19:16 ` Jim Harris
1 sibling, 0 replies; 3+ messages in thread
From: Jim Harris @ 2023-09-26 19:16 UTC (permalink / raw)
To: Dan Williams, linux-cxl@vger.kernel.org
Cc: stable@vger.kernel.org, Alison Schofield
On 9/15/2023 1:07 AM, Dan Williams wrote:
> The cxl_test unit test environment models a CXL topology for
> sysfs/user-ABI regression testing. It uses interface mocking via the
> "--wrap=" linker option to redirect cxl_core routines that parse
> hardware registers with versions that just publish objects, like
> devm_cxl_enumerate_decoders().
>
> Starting with:
>
> Commit 19ab69a60e3b ("cxl/port: Store the port's Component Register mappings in struct cxl_port")
>
> ...port register enumeration is moved into devm_cxl_add_port(). This
> conflicts with the "cxl_test avoids emulating registers stance" so
> either the port code needs to be refactored (too violent), or modified
> so that register enumeration is skipped on "fake" cxl_test ports
> (annoying, but straightforward).
>
> This conflict has happened previously and the "check for platform
> device" workaround to avoid instrusive refactoring was deployed in those
> scenarios. In general, refactoring should only benefit production code,
> test code needs to remain minimally instrusive to the greatest extent
> possible.
>
> This was missed previously because it may sometimes just cause warning
> messages to be emitted, but it can also cause test failures. The
> backport to -stable is only nice to have for clean cxl_test runs.
>
> Fixes: 19ab69a60e3b ("cxl/port: Store the port's Component Register mappings in struct cxl_port")
> Cc: <stable@vger.kernel.org>
> Reported-by: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
This patch has already hit upstream, but I'd like to report
this also fixed a 100% reproducible issue just loading
cxl_test modules in a Parallels ARM VM on a MacBook Pro.
^ permalink raw reply [flat|nested] 3+ messages in thread