2023-02-18 01:38:42

by Alison Schofield

[permalink] [raw]
Subject: [PATCH] cxl/hdm: dev_warn() on unsupported mixed mode decoder

From: Alison Schofield <[email protected]>

A mixed mode decoder is programmed with device physical addresses
that span both ram and pmem partitions of a memdev.

Linux does not support mixed mode decoders. The driver rejects
sysfs writes that try to set decoder mode to mixed, and if a
resource bieng allocated is not wholly contained in either the
pmem or ram partition of a memdev, it is also rejected. Basically,
the CXL region driver is not going to create regions with mixed
mode decoders, but the BIOS could.

If the kernel driver sees the mixed mode decoder, it will fail to
enable the region, and emit a dev_dbg() message.

A dev_dbg() is not noisy enough in this case. Change the message
to be a dev_warn() that explicitly says mixed mode is not supported.

Suggested-by: Dan Williams <[email protected]>
Signed-off-by: Alison Schofield <[email protected]>
---
drivers/cxl/core/hdm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 45deda18ed32..9eaf93c8ebb0 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -294,8 +294,8 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
else if (resource_contains(&cxlds->ram_res, res))
cxled->mode = CXL_DECODER_RAM;
else {
- dev_dbg(dev, "decoder%d.%d: %pr mixed\n", port->id,
- cxled->cxld.id, cxled->dpa_res);
+ dev_warn(dev, "decoder%d.%d: %pr mixed mode not supported\n",
+ port->id, cxled->cxld.id, cxled->dpa_res);
cxled->mode = CXL_DECODER_MIXED;
}


base-commit: a5fcd228ca1db9810ba1ed461c90b6ee933b9daf
--
2.37.3



2023-02-18 03:18:08

by Verma, Vishal L

[permalink] [raw]
Subject: Re: [PATCH] cxl/hdm: dev_warn() on unsupported mixed mode decoder

On Fri, 2023-02-17 at 17:38 -0800, [email protected] wrote:
> From: Alison Schofield <[email protected]>
>
> A mixed mode decoder is programmed with device physical addresses
> that span both ram and pmem partitions of a memdev.
>
> Linux does not support mixed mode decoders. The driver rejects
> sysfs writes that try to set decoder mode to mixed, and if a
> resource bieng allocated is not wholly contained in either the
> pmem or ram partition of a memdev, it is also rejected. Basically,
> the CXL region driver is not going to create regions with mixed
> mode decoders, but the BIOS could.
>
> If the kernel driver sees the mixed mode decoder, it will fail to
> enable the region, and emit a dev_dbg() message.
>
> A dev_dbg() is not noisy enough in this case. Change the message
> to be a dev_warn() that explicitly says mixed mode is not supported.
>
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Alison Schofield <[email protected]>
> ---
>  drivers/cxl/core/hdm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Looks good,
Reviewed-by: Vishal Verma <[email protected]>

>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 45deda18ed32..9eaf93c8ebb0 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -294,8 +294,8 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
>         else if (resource_contains(&cxlds->ram_res, res))
>                 cxled->mode = CXL_DECODER_RAM;
>         else {
> -               dev_dbg(dev, "decoder%d.%d: %pr mixed\n", port->id,
> -                       cxled->cxld.id, cxled->dpa_res);
> +               dev_warn(dev, "decoder%d.%d: %pr mixed mode not supported\n",
> +                        port->id, cxled->cxld.id, cxled->dpa_res);
>                 cxled->mode = CXL_DECODER_MIXED;
>         }
>  
>
> base-commit: a5fcd228ca1db9810ba1ed461c90b6ee933b9daf

2023-02-20 14:31:22

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] cxl/hdm: dev_warn() on unsupported mixed mode decoder

On Fri, 17 Feb 2023 17:38:34 -0800
[email protected] wrote:

> From: Alison Schofield <[email protected]>
>
> A mixed mode decoder is programmed with device physical addresses
> that span both ram and pmem partitions of a memdev.
>
> Linux does not support mixed mode decoders. The driver rejects
> sysfs writes that try to set decoder mode to mixed, and if a
> resource bieng allocated is not wholly contained in either the
> pmem or ram partition of a memdev, it is also rejected. Basically,
> the CXL region driver is not going to create regions with mixed
> mode decoders, but the BIOS could.
>
> If the kernel driver sees the mixed mode decoder, it will fail to
> enable the region, and emit a dev_dbg() message.
>
> A dev_dbg() is not noisy enough in this case. Change the message
> to be a dev_warn() that explicitly says mixed mode is not supported.
>
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Alison Schofield <[email protected]>
Makes sense.
FWIW
Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> drivers/cxl/core/hdm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 45deda18ed32..9eaf93c8ebb0 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -294,8 +294,8 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> else if (resource_contains(&cxlds->ram_res, res))
> cxled->mode = CXL_DECODER_RAM;
> else {
> - dev_dbg(dev, "decoder%d.%d: %pr mixed\n", port->id,
> - cxled->cxld.id, cxled->dpa_res);
> + dev_warn(dev, "decoder%d.%d: %pr mixed mode not supported\n",
> + port->id, cxled->cxld.id, cxled->dpa_res);
> cxled->mode = CXL_DECODER_MIXED;
> }
>
>
> base-commit: a5fcd228ca1db9810ba1ed461c90b6ee933b9daf


2023-02-21 17:15:21

by Dave Jiang

[permalink] [raw]
Subject: Re: [PATCH] cxl/hdm: dev_warn() on unsupported mixed mode decoder



On 2/17/23 6:38 PM, [email protected] wrote:
> From: Alison Schofield <[email protected]>
>
> A mixed mode decoder is programmed with device physical addresses
> that span both ram and pmem partitions of a memdev.
>
> Linux does not support mixed mode decoders. The driver rejects
> sysfs writes that try to set decoder mode to mixed, and if a
> resource bieng allocated is not wholly contained in either the
> pmem or ram partition of a memdev, it is also rejected. Basically,
> the CXL region driver is not going to create regions with mixed
> mode decoders, but the BIOS could.
>
> If the kernel driver sees the mixed mode decoder, it will fail to
> enable the region, and emit a dev_dbg() message.
>
> A dev_dbg() is not noisy enough in this case. Change the message
> to be a dev_warn() that explicitly says mixed mode is not supported.
>
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Alison Schofield <[email protected]>

Reviewed-by: Dave Jiang <[email protected]>

> ---
> drivers/cxl/core/hdm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 45deda18ed32..9eaf93c8ebb0 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -294,8 +294,8 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> else if (resource_contains(&cxlds->ram_res, res))
> cxled->mode = CXL_DECODER_RAM;
> else {
> - dev_dbg(dev, "decoder%d.%d: %pr mixed\n", port->id,
> - cxled->cxld.id, cxled->dpa_res);
> + dev_warn(dev, "decoder%d.%d: %pr mixed mode not supported\n",
> + port->id, cxled->cxld.id, cxled->dpa_res);
> cxled->mode = CXL_DECODER_MIXED;
> }
>
>
> base-commit: a5fcd228ca1db9810ba1ed461c90b6ee933b9daf