linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression
@ 2025-06-07  3:32 Dan Williams
  2025-06-07  3:32 ` [PATCH 1/3] driver core: faux: Suppress bind attributes Dan Williams
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Dan Williams @ 2025-06-07  3:32 UTC (permalink / raw)
  To: gregkh, rafael.j.wysocki
  Cc: linux-cxl, linux-acpi, linux-kernel, Ben Cheatham,
	Danilo Krummrich, Rafael J. Wysocki, Sudeep Holla

git bisect flags:

6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")

...as the reason basic CXL unit tests are failing on latest mainline. In
addition to the fix to einj-core.c, this also needs some updates to
faux_device to make it behave more like platform_driver_probe(). Details
in the individual patches.

Dan Williams (3):
  driver core: faux: Suppress bind attributes
  driver core: faux: Quiet probe failures
  ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create()
    failure

 drivers/acpi/apei/einj-core.c | 9 +++------
 drivers/base/faux.c           | 3 ++-
 2 files changed, 5 insertions(+), 7 deletions(-)


base-commit: a9dfb7db96f7bc1f30feae673aab7fdbfbc94e9c
-- 
2.49.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] driver core: faux: Suppress bind attributes
  2025-06-07  3:32 [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Dan Williams
@ 2025-06-07  3:32 ` Dan Williams
  2025-06-09 10:19   ` Jonathan Cameron
  2025-06-07  3:32 ` [PATCH 2/3] driver core: faux: Quiet probe failures Dan Williams
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Dan Williams @ 2025-06-07  3:32 UTC (permalink / raw)
  To: gregkh, rafael.j.wysocki
  Cc: linux-cxl, linux-acpi, linux-kernel, Rafael J. Wysocki,
	Danilo Krummrich

faux_device_create() is almost a suitable candidate to replace
platform_driver_probe() if not for the fact that faux_device_create()
supports dynamic attach/detach of the driver.

Drop the bind attributes with the expectation that simple faux devices can
always assume that the device is permanently bound at create, and only
unbound at 'destroy'.

The acpi-einj driver depends on static bind.

Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/base/faux.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/faux.c b/drivers/base/faux.c
index 9054d346bd7f..934da77ca48b 100644
--- a/drivers/base/faux.c
+++ b/drivers/base/faux.c
@@ -86,6 +86,7 @@ static struct device_driver faux_driver = {
 	.name		= "faux_driver",
 	.bus		= &faux_bus_type,
 	.probe_type	= PROBE_FORCE_SYNCHRONOUS,
+	.suppress_bind_attrs = true,
 };
 
 static void faux_device_release(struct device *dev)
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] driver core: faux: Quiet probe failures
  2025-06-07  3:32 [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Dan Williams
  2025-06-07  3:32 ` [PATCH 1/3] driver core: faux: Suppress bind attributes Dan Williams
@ 2025-06-07  3:32 ` Dan Williams
  2025-06-09 10:19   ` Jonathan Cameron
  2025-06-07  3:32 ` [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure Dan Williams
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Dan Williams @ 2025-06-07  3:32 UTC (permalink / raw)
  To: gregkh, rafael.j.wysocki
  Cc: linux-cxl, linux-acpi, linux-kernel, Rafael J. Wysocki,
	Danilo Krummrich

The acpi-einj conversion to faux_device_create() leads to a noisy error
message when the error injection facility is disabled. Quiet the error as
CXL error injection via ACPI expects the module to stay loaded even if the
error injection facility is disabled.

This situation arose because CXL knows proper kernel named objects to
trigger errors against, but acpi-einj knows how to perform the error
injection. The injection mechanism is shared with non-CXL use cases. The
result is CXL now has a module dependency on einj-core.ko, and init/probe
failures are handled at runtime.

Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/base/faux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/faux.c b/drivers/base/faux.c
index 934da77ca48b..f5fbda0a9a44 100644
--- a/drivers/base/faux.c
+++ b/drivers/base/faux.c
@@ -170,7 +170,7 @@ struct faux_device *faux_device_create_with_groups(const char *name,
 	 * successful is almost impossible to determine by the caller.
 	 */
 	if (!dev->driver) {
-		dev_err(dev, "probe did not succeed, tearing down the device\n");
+		dev_dbg(dev, "probe did not succeed, tearing down the device\n");
 		faux_device_destroy(faux_dev);
 		faux_dev = NULL;
 	}
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure
  2025-06-07  3:32 [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Dan Williams
  2025-06-07  3:32 ` [PATCH 1/3] driver core: faux: Suppress bind attributes Dan Williams
  2025-06-07  3:32 ` [PATCH 2/3] driver core: faux: Quiet probe failures Dan Williams
@ 2025-06-07  3:32 ` Dan Williams
  2025-06-09 10:17   ` Jonathan Cameron
  2025-06-09 13:28   ` Cheatham, Benjamin
  2025-06-09 13:39 ` [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Sudeep Holla
  2025-06-09 15:04 ` Dave Jiang
  4 siblings, 2 replies; 16+ messages in thread
From: Dan Williams @ 2025-06-07  3:32 UTC (permalink / raw)
  To: gregkh, rafael.j.wysocki
  Cc: linux-cxl, linux-acpi, linux-kernel, Rafael J. Wysocki,
	Sudeep Holla, Ben Cheatham

CXL has a symbol dependency on einj_core.ko, so if einj_init() fails then
cxl_core.ko fails to load. Prior to the faux_device_create() conversion,
einj_probe() failures were tracked by the einj_initialized flag without
failing einj_init().

Revert to that behavior and always succeed einj_init() given there is no
way, and no pressing need, to discern faux device-create vs device-probe
failures.

This situation arose because CXL knows proper kernel named objects to
trigger errors against, but acpi-einj knows how to perform the error
injection. The injection mechanism is shared with non-CXL use cases. The
result is CXL now has a module dependency on einj-core.ko, and init/probe
failures are handled at runtime.

Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Ben Cheatham <Benjamin.Cheatham@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/apei/einj-core.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index fea11a35eea3..9b041415a9d0 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -883,19 +883,16 @@ static int __init einj_init(void)
 	}
 
 	einj_dev = faux_device_create("acpi-einj", NULL, &einj_device_ops);
-	if (!einj_dev)
-		return -ENODEV;
 
-	einj_initialized = true;
+	if (einj_dev)
+		einj_initialized = true;
 
 	return 0;
 }
 
 static void __exit einj_exit(void)
 {
-	if (einj_initialized)
-		faux_device_destroy(einj_dev);
-
+	faux_device_destroy(einj_dev);
 }
 
 module_init(einj_init);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure
  2025-06-07  3:32 ` [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure Dan Williams
@ 2025-06-09 10:17   ` Jonathan Cameron
  2025-06-09 10:42     ` Greg KH
  2025-06-09 13:28   ` Cheatham, Benjamin
  1 sibling, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2025-06-09 10:17 UTC (permalink / raw)
  To: Dan Williams
  Cc: gregkh, rafael.j.wysocki, linux-cxl, linux-acpi, linux-kernel,
	Rafael J. Wysocki, Sudeep Holla, Ben Cheatham

On Fri, 6 Jun 2025 20:32:28 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> CXL has a symbol dependency on einj_core.ko, so if einj_init() fails then
> cxl_core.ko fails to load. Prior to the faux_device_create() conversion,
> einj_probe() failures were tracked by the einj_initialized flag without
> failing einj_init().
> 
> Revert to that behavior and always succeed einj_init() given there is no
> way, and no pressing need, to discern faux device-create vs device-probe
> failures.
> 
> This situation arose because CXL knows proper kernel named objects to
> trigger errors against, but acpi-einj knows how to perform the error
> injection. The injection mechanism is shared with non-CXL use cases. The
> result is CXL now has a module dependency on einj-core.ko, and init/probe
> failures are handled at runtime.
> 
> Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Ben Cheatham <Benjamin.Cheatham@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/acpi/apei/einj-core.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index fea11a35eea3..9b041415a9d0 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -883,19 +883,16 @@ static int __init einj_init(void)
>  	}
>  
>  	einj_dev = faux_device_create("acpi-einj", NULL, &einj_device_ops);
> -	if (!einj_dev)
> -		return -ENODEV;
>  
> -	einj_initialized = true;
> +	if (einj_dev)
> +		einj_initialized = true;
>  
>  	return 0;
>  }
>  
>  static void __exit einj_exit(void)
>  {
> -	if (einj_initialized)
> -		faux_device_destroy(einj_dev);
> -
> +	faux_device_destroy(einj_dev);

Hi Dan,

Thi bit is sort of fine though not really related, because
faux_device_destroy() checks

void faux_device_destroy(struct faux_device *faux_dev)
{
	struct device *dev = &faux_dev->dev;

	if (!faux_dev)
		return;

Though that check is after a dereference of faux_dev
which doesn't look right to me.  Might be fine because
of how the kernel is built (I can't remember where we ended
up on topic of compilers making undefined behavior based
optimizations).  Still not that nice from a logical point of view!

>  }
>  
>  module_init(einj_init);


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] driver core: faux: Quiet probe failures
  2025-06-07  3:32 ` [PATCH 2/3] driver core: faux: Quiet probe failures Dan Williams
@ 2025-06-09 10:19   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2025-06-09 10:19 UTC (permalink / raw)
  To: Dan Williams
  Cc: gregkh, rafael.j.wysocki, linux-cxl, linux-acpi, linux-kernel,
	Rafael J. Wysocki, Danilo Krummrich

On Fri, 6 Jun 2025 20:32:27 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> The acpi-einj conversion to faux_device_create() leads to a noisy error
> message when the error injection facility is disabled. Quiet the error as
> CXL error injection via ACPI expects the module to stay loaded even if the
> error injection facility is disabled.
> 
> This situation arose because CXL knows proper kernel named objects to
> trigger errors against, but acpi-einj knows how to perform the error
> injection. The injection mechanism is shared with non-CXL use cases. The
> result is CXL now has a module dependency on einj-core.ko, and init/probe
> failures are handled at runtime.
> 
> Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

I guess this is fair enough. 

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/base/faux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/faux.c b/drivers/base/faux.c
> index 934da77ca48b..f5fbda0a9a44 100644
> --- a/drivers/base/faux.c
> +++ b/drivers/base/faux.c
> @@ -170,7 +170,7 @@ struct faux_device *faux_device_create_with_groups(const char *name,
>  	 * successful is almost impossible to determine by the caller.
>  	 */
>  	if (!dev->driver) {
> -		dev_err(dev, "probe did not succeed, tearing down the device\n");
> +		dev_dbg(dev, "probe did not succeed, tearing down the device\n");
>  		faux_device_destroy(faux_dev);
>  		faux_dev = NULL;
>  	}


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] driver core: faux: Suppress bind attributes
  2025-06-07  3:32 ` [PATCH 1/3] driver core: faux: Suppress bind attributes Dan Williams
@ 2025-06-09 10:19   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2025-06-09 10:19 UTC (permalink / raw)
  To: Dan Williams
  Cc: gregkh, rafael.j.wysocki, linux-cxl, linux-acpi, linux-kernel,
	Rafael J. Wysocki, Danilo Krummrich

On Fri, 6 Jun 2025 20:32:26 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> faux_device_create() is almost a suitable candidate to replace
> platform_driver_probe() if not for the fact that faux_device_create()
> supports dynamic attach/detach of the driver.
> 
> Drop the bind attributes with the expectation that simple faux devices can
> always assume that the device is permanently bound at create, and only
> unbound at 'destroy'.
> 
> The acpi-einj driver depends on static bind.
> 
> Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Seems like a fair assumption that these should never be used so
we might as well hide the.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/base/faux.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/base/faux.c b/drivers/base/faux.c
> index 9054d346bd7f..934da77ca48b 100644
> --- a/drivers/base/faux.c
> +++ b/drivers/base/faux.c
> @@ -86,6 +86,7 @@ static struct device_driver faux_driver = {
>  	.name		= "faux_driver",
>  	.bus		= &faux_bus_type,
>  	.probe_type	= PROBE_FORCE_SYNCHRONOUS,
> +	.suppress_bind_attrs = true,
>  };
>  
>  static void faux_device_release(struct device *dev)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure
  2025-06-09 10:17   ` Jonathan Cameron
@ 2025-06-09 10:42     ` Greg KH
  2025-06-09 16:05       ` Jonathan Cameron
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2025-06-09 10:42 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Dan Williams, rafael.j.wysocki, linux-cxl, linux-acpi,
	linux-kernel, Rafael J. Wysocki, Sudeep Holla, Ben Cheatham

On Mon, Jun 09, 2025 at 11:17:58AM +0100, Jonathan Cameron wrote:
> On Fri, 6 Jun 2025 20:32:28 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > CXL has a symbol dependency on einj_core.ko, so if einj_init() fails then
> > cxl_core.ko fails to load. Prior to the faux_device_create() conversion,
> > einj_probe() failures were tracked by the einj_initialized flag without
> > failing einj_init().
> > 
> > Revert to that behavior and always succeed einj_init() given there is no
> > way, and no pressing need, to discern faux device-create vs device-probe
> > failures.
> > 
> > This situation arose because CXL knows proper kernel named objects to
> > trigger errors against, but acpi-einj knows how to perform the error
> > injection. The injection mechanism is shared with non-CXL use cases. The
> > result is CXL now has a module dependency on einj-core.ko, and init/probe
> > failures are handled at runtime.
> > 
> > Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Ben Cheatham <Benjamin.Cheatham@amd.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  drivers/acpi/apei/einj-core.c | 9 +++------
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> > index fea11a35eea3..9b041415a9d0 100644
> > --- a/drivers/acpi/apei/einj-core.c
> > +++ b/drivers/acpi/apei/einj-core.c
> > @@ -883,19 +883,16 @@ static int __init einj_init(void)
> >  	}
> >  
> >  	einj_dev = faux_device_create("acpi-einj", NULL, &einj_device_ops);
> > -	if (!einj_dev)
> > -		return -ENODEV;
> >  
> > -	einj_initialized = true;
> > +	if (einj_dev)
> > +		einj_initialized = true;
> >  
> >  	return 0;
> >  }
> >  
> >  static void __exit einj_exit(void)
> >  {
> > -	if (einj_initialized)
> > -		faux_device_destroy(einj_dev);
> > -
> > +	faux_device_destroy(einj_dev);
> 
> Hi Dan,
> 
> Thi bit is sort of fine though not really related, because
> faux_device_destroy() checks
> 
> void faux_device_destroy(struct faux_device *faux_dev)
> {
> 	struct device *dev = &faux_dev->dev;
> 
> 	if (!faux_dev)
> 		return;
> 
> Though that check is after a dereference of faux_dev
> which doesn't look right to me.  Might be fine because
> of how the kernel is built (I can't remember where we ended
> up on topic of compilers making undefined behavior based
> optimizations).  Still not that nice from a logical point of view!

I think this is fine as we just put "0 + offset of dev" into dev, and
didn't do anything with that (i.e. no actual read of that memory
location happened).  The compiler shouldn't be doing anything that could
happen after the return before we check for a valid pointer here, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure
  2025-06-07  3:32 ` [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure Dan Williams
  2025-06-09 10:17   ` Jonathan Cameron
@ 2025-06-09 13:28   ` Cheatham, Benjamin
  1 sibling, 0 replies; 16+ messages in thread
From: Cheatham, Benjamin @ 2025-06-09 13:28 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-cxl, linux-acpi, linux-kernel, Rafael J. Wysocki,
	Sudeep Holla, gregkh, rafael.j.wysocki

On 6/6/2025 10:32 PM, Dan Williams wrote:
> CXL has a symbol dependency on einj_core.ko, so if einj_init() fails then
> cxl_core.ko fails to load. Prior to the faux_device_create() conversion,
> einj_probe() failures were tracked by the einj_initialized flag without
> failing einj_init().
> 
> Revert to that behavior and always succeed einj_init() given there is no
> way, and no pressing need, to discern faux device-create vs device-probe
> failures.
> 
> This situation arose because CXL knows proper kernel named objects to
> trigger errors against, but acpi-einj knows how to perform the error
> injection. The injection mechanism is shared with non-CXL use cases. The
> result is CXL now has a module dependency on einj-core.ko, and init/probe
> failures are handled at runtime.
> 
> Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Ben Cheatham <Benjamin.Cheatham@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---

Thanks for sending this out!

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression
  2025-06-07  3:32 [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Dan Williams
                   ` (2 preceding siblings ...)
  2025-06-07  3:32 ` [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure Dan Williams
@ 2025-06-09 13:39 ` Sudeep Holla
  2025-06-09 15:04 ` Dave Jiang
  4 siblings, 0 replies; 16+ messages in thread
From: Sudeep Holla @ 2025-06-09 13:39 UTC (permalink / raw)
  To: Dan Williams
  Cc: gregkh, rafael.j.wysocki, Sudeep Holla, linux-cxl, linux-acpi,
	linux-kernel, Ben Cheatham, Danilo Krummrich, Rafael J. Wysocki

On Fri, Jun 06, 2025 at 08:32:25PM -0700, Dan Williams wrote:
> git bisect flags:
> 
> 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> 
> ...as the reason basic CXL unit tests are failing on latest mainline. In
> addition to the fix to einj-core.c, this also needs some updates to
> faux_device to make it behave more like platform_driver_probe(). Details
> in the individual patches.
> 

Thanks for not only reporting the issue with the commit above but also
solving it and posting patches.

They all look good to me.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression
  2025-06-07  3:32 [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Dan Williams
                   ` (3 preceding siblings ...)
  2025-06-09 13:39 ` [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Sudeep Holla
@ 2025-06-09 15:04 ` Dave Jiang
  2025-06-09 18:58   ` Rafael J. Wysocki
  4 siblings, 1 reply; 16+ messages in thread
From: Dave Jiang @ 2025-06-09 15:04 UTC (permalink / raw)
  To: Dan Williams, gregkh, rafael.j.wysocki
  Cc: linux-cxl, linux-acpi, linux-kernel, Ben Cheatham,
	Danilo Krummrich, Rafael J. Wysocki, Sudeep Holla



On 6/6/25 8:32 PM, Dan Williams wrote:
> git bisect flags:
> 
> 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> 
> ...as the reason basic CXL unit tests are failing on latest mainline. In
> addition to the fix to einj-core.c, this also needs some updates to
> faux_device to make it behave more like platform_driver_probe(). Details
> in the individual patches.
> 
> Dan Williams (3):
>   driver core: faux: Suppress bind attributes
>   driver core: faux: Quiet probe failures
>   ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create()
>     failure

LGTM
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> 
>  drivers/acpi/apei/einj-core.c | 9 +++------
>  drivers/base/faux.c           | 3 ++-
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> 
> base-commit: a9dfb7db96f7bc1f30feae673aab7fdbfbc94e9c


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure
  2025-06-09 10:42     ` Greg KH
@ 2025-06-09 16:05       ` Jonathan Cameron
  2025-06-10 17:22         ` Dan Williams
  0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2025-06-09 16:05 UTC (permalink / raw)
  To: Greg KH
  Cc: Dan Williams, rafael.j.wysocki, linux-cxl, linux-acpi,
	linux-kernel, Rafael J. Wysocki, Sudeep Holla, Ben Cheatham

On Mon, 9 Jun 2025 12:42:53 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Jun 09, 2025 at 11:17:58AM +0100, Jonathan Cameron wrote:
> > On Fri, 6 Jun 2025 20:32:28 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >   
> > > CXL has a symbol dependency on einj_core.ko, so if einj_init() fails then
> > > cxl_core.ko fails to load. Prior to the faux_device_create() conversion,
> > > einj_probe() failures were tracked by the einj_initialized flag without
> > > failing einj_init().
> > > 
> > > Revert to that behavior and always succeed einj_init() given there is no
> > > way, and no pressing need, to discern faux device-create vs device-probe
> > > failures.
> > > 
> > > This situation arose because CXL knows proper kernel named objects to
> > > trigger errors against, but acpi-einj knows how to perform the error
> > > injection. The injection mechanism is shared with non-CXL use cases. The
> > > result is CXL now has a module dependency on einj-core.ko, and init/probe
> > > failures are handled at runtime.
> > > 
> > > Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > > Cc: Ben Cheatham <Benjamin.Cheatham@amd.com>
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  drivers/acpi/apei/einj-core.c | 9 +++------
> > >  1 file changed, 3 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> > > index fea11a35eea3..9b041415a9d0 100644
> > > --- a/drivers/acpi/apei/einj-core.c
> > > +++ b/drivers/acpi/apei/einj-core.c
> > > @@ -883,19 +883,16 @@ static int __init einj_init(void)
> > >  	}
> > >  
> > >  	einj_dev = faux_device_create("acpi-einj", NULL, &einj_device_ops);
> > > -	if (!einj_dev)
> > > -		return -ENODEV;
> > >  
> > > -	einj_initialized = true;
> > > +	if (einj_dev)
> > > +		einj_initialized = true;
> > >  
> > >  	return 0;
> > >  }
> > >  
> > >  static void __exit einj_exit(void)
> > >  {
> > > -	if (einj_initialized)
> > > -		faux_device_destroy(einj_dev);
> > > -
> > > +	faux_device_destroy(einj_dev);  
> > 
> > Hi Dan,
> > 
> > Thi bit is sort of fine though not really related, because
> > faux_device_destroy() checks
> > 
> > void faux_device_destroy(struct faux_device *faux_dev)
> > {
> > 	struct device *dev = &faux_dev->dev;
> > 
> > 	if (!faux_dev)
> > 		return;
> > 
> > Though that check is after a dereference of faux_dev
> > which doesn't look right to me.  Might be fine because
> > of how the kernel is built (I can't remember where we ended
> > up on topic of compilers making undefined behavior based
> > optimizations).  Still not that nice from a logical point of view!  
> 
> I think this is fine as we just put "0 + offset of dev" into dev, and
> didn't do anything with that (i.e. no actual read of that memory
> location happened).  The compiler shouldn't be doing anything that could
> happen after the return before we check for a valid pointer here, right?

Hmm. I did some digging. Seems that was debated 10 years ago without
a huge amount of clarity on the answer beyond all sane people telling
compiler folk not to use this in optimizations :)

Comes down to whether any dereference of NULL is UB whether or not
the compiler can just do a simple offset calculation.

Anyhow, whilst fine, it's still a little ugly to my eyes :(

Jonathan



> 
> thanks,
> 
> greg k-h
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression
  2025-06-09 15:04 ` Dave Jiang
@ 2025-06-09 18:58   ` Rafael J. Wysocki
  2025-06-10 10:12     ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Rafael J. Wysocki @ 2025-06-09 18:58 UTC (permalink / raw)
  To: Dave Jiang, gregkh
  Cc: Dan Williams, rafael.j.wysocki, linux-cxl, linux-acpi,
	linux-kernel, Ben Cheatham, Danilo Krummrich, Sudeep Holla

On Mon, Jun 9, 2025 at 5:04 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
>
>
> On 6/6/25 8:32 PM, Dan Williams wrote:
> > git bisect flags:
> >
> > 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> >
> > ...as the reason basic CXL unit tests are failing on latest mainline. In
> > addition to the fix to einj-core.c, this also needs some updates to
> > faux_device to make it behave more like platform_driver_probe(). Details
> > in the individual patches.
> >
> > Dan Williams (3):
> >   driver core: faux: Suppress bind attributes
> >   driver core: faux: Quiet probe failures
> >   ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create()
> >     failure
>
> LGTM
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> >
> >  drivers/acpi/apei/einj-core.c | 9 +++------
> >  drivers/base/faux.c           | 3 ++-
> >  2 files changed, 5 insertions(+), 7 deletions(-)

Greg, I think it's better if I route this through the ACPI tree as the
issue being fixed was introduced through it.

Any concerns regarding this?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression
  2025-06-09 18:58   ` Rafael J. Wysocki
@ 2025-06-10 10:12     ` Greg KH
  2025-06-10 17:24       ` Rafael J. Wysocki
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2025-06-10 10:12 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Dave Jiang, Dan Williams, rafael.j.wysocki, linux-cxl, linux-acpi,
	linux-kernel, Ben Cheatham, Danilo Krummrich, Sudeep Holla

On Mon, Jun 09, 2025 at 08:58:26PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jun 9, 2025 at 5:04 PM Dave Jiang <dave.jiang@intel.com> wrote:
> >
> >
> >
> > On 6/6/25 8:32 PM, Dan Williams wrote:
> > > git bisect flags:
> > >
> > > 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> > >
> > > ...as the reason basic CXL unit tests are failing on latest mainline. In
> > > addition to the fix to einj-core.c, this also needs some updates to
> > > faux_device to make it behave more like platform_driver_probe(). Details
> > > in the individual patches.
> > >
> > > Dan Williams (3):
> > >   driver core: faux: Suppress bind attributes
> > >   driver core: faux: Quiet probe failures
> > >   ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create()
> > >     failure
> >
> > LGTM
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> >
> > >
> > >  drivers/acpi/apei/einj-core.c | 9 +++------
> > >  drivers/base/faux.c           | 3 ++-
> > >  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> Greg, I think it's better if I route this through the ACPI tree as the
> issue being fixed was introduced through it.
> 
> Any concerns regarding this?

None from me!

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure
  2025-06-09 16:05       ` Jonathan Cameron
@ 2025-06-10 17:22         ` Dan Williams
  0 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2025-06-10 17:22 UTC (permalink / raw)
  To: Jonathan Cameron, Greg KH
  Cc: Dan Williams, rafael.j.wysocki, linux-cxl, linux-acpi,
	linux-kernel, Rafael J. Wysocki, Sudeep Holla, Ben Cheatham

Jonathan Cameron wrote:
[..]
> Hmm. I did some digging. Seems that was debated 10 years ago without
> a huge amount of clarity on the answer beyond all sane people telling
> compiler folk not to use this in optimizations :)
> 
> Comes down to whether any dereference of NULL is UB whether or not
> the compiler can just do a simple offset calculation.
> 
> Anyhow, whilst fine, it's still a little ugly to my eyes :(

I recall we had this conversation with Dan Carpenter on a smatch patch
and resolved that while it looks "interesting" it does no harm.

For this patch I am not motivated to spin it because even if the the
compiler took advantage of the NULL check to drop UB work, that would
only mean dropping the assignment.

Otherwise, this conversion lines up with the intent of both
einj_initialized and faux_device_destroy() whereby faux_device_destroy()
is already prepared for the case where faux_device_create() fails.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression
  2025-06-10 10:12     ` Greg KH
@ 2025-06-10 17:24       ` Rafael J. Wysocki
  0 siblings, 0 replies; 16+ messages in thread
From: Rafael J. Wysocki @ 2025-06-10 17:24 UTC (permalink / raw)
  To: Greg KH, Dan Williams
  Cc: Rafael J. Wysocki, Dave Jiang, rafael.j.wysocki, linux-cxl,
	linux-acpi, linux-kernel, Ben Cheatham, Danilo Krummrich,
	Sudeep Holla

On Tue, Jun 10, 2025 at 12:12 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jun 09, 2025 at 08:58:26PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jun 9, 2025 at 5:04 PM Dave Jiang <dave.jiang@intel.com> wrote:
> > >
> > >
> > >
> > > On 6/6/25 8:32 PM, Dan Williams wrote:
> > > > git bisect flags:
> > > >
> > > > 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> > > >
> > > > ...as the reason basic CXL unit tests are failing on latest mainline. In
> > > > addition to the fix to einj-core.c, this also needs some updates to
> > > > faux_device to make it behave more like platform_driver_probe(). Details
> > > > in the individual patches.
> > > >
> > > > Dan Williams (3):
> > > >   driver core: faux: Suppress bind attributes
> > > >   driver core: faux: Quiet probe failures
> > > >   ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create()
> > > >     failure
> > >
> > > LGTM
> > > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > >
> > > >
> > > >  drivers/acpi/apei/einj-core.c | 9 +++------
> > > >  drivers/base/faux.c           | 3 ++-
> > > >  2 files changed, 5 insertions(+), 7 deletions(-)
> >
> > Greg, I think it's better if I route this through the ACPI tree as the
> > issue being fixed was introduced through it.
> >
> > Any concerns regarding this?
>
> None from me!
>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

So applied as 6.16-rc material.

Thank you!

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-06-10 17:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-07  3:32 [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Dan Williams
2025-06-07  3:32 ` [PATCH 1/3] driver core: faux: Suppress bind attributes Dan Williams
2025-06-09 10:19   ` Jonathan Cameron
2025-06-07  3:32 ` [PATCH 2/3] driver core: faux: Quiet probe failures Dan Williams
2025-06-09 10:19   ` Jonathan Cameron
2025-06-07  3:32 ` [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device_create() failure Dan Williams
2025-06-09 10:17   ` Jonathan Cameron
2025-06-09 10:42     ` Greg KH
2025-06-09 16:05       ` Jonathan Cameron
2025-06-10 17:22         ` Dan Williams
2025-06-09 13:28   ` Cheatham, Benjamin
2025-06-09 13:39 ` [PATCH 0/3] CXL: ACPI: faux: Fix cxl_core.ko module load regression Sudeep Holla
2025-06-09 15:04 ` Dave Jiang
2025-06-09 18:58   ` Rafael J. Wysocki
2025-06-10 10:12     ` Greg KH
2025-06-10 17:24       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).