[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ae59e9a4-a4f1-4839-977b-b667d927c647@amd.com>
Date: Thu, 17 Oct 2024 16:50:12 -0500
From: Ben Cheatham <benjamin.cheatham@....com>
To: <alejandro.lucero-palau@....com>
CC: <linux-cxl@...r.kernel.org>, <netdev@...r.kernel.org>,
<dan.j.williams@...el.com>, <martin.habets@...inx.com>,
<edward.cree@....com>, <davem@...emloft.net>, <kuba@...nel.org>,
<pabeni@...hat.com>, <edumazet@...gle.com>
Subject: Re: [PATCH v4 24/26] cxl: preclude device memory to be used for dax
On 10/17/24 11:52 AM, alejandro.lucero-palau@....com wrote:
> From: Alejandro Lucero <alucerop@....com>
>
> By definition a type2 cxl device will use the host managed memory for
> specific functionality, therefore it should not be available to other
> uses.
>
I disagree that this is a valid assumption. I don't think that the device memory
should be added as system ram, but I do think there is value in having the
option to have the memory be available as a device-dax region. My reasoning here is:
1) I can think of a possible use case where the memory benefits from being user space
accessible (CXL memory GPU buffers).
2) I think it's really early to say this is the only way we expect these devices to
be used. The flip side of this is that it is early, so we can always change it later
when we start seeing real devices, but I would vote to keep a more flexible structure
early and if no one uses it oh well.
My idea here is that whoever writes the driver indicates whether they want to make
the device memory device-dax mappable, or do it all manually like you are now. I've
been working on a RFC based on v3 of this series that has this (as well as the
"better" solution mentioned in patch 22/26) that I was planning
on sending out in the next week or two, but if the consensus here is that this is
not the direction we want to go I'll probably drop that portion.
Thanks,
Ben
> Signed-off-by: Alejandro Lucero <alucerop@....com>
> ---
> drivers/cxl/core/region.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 04c270a29e96..7c84d8f89af6 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3703,6 +3703,9 @@ static int cxl_region_probe(struct device *dev)
> case CXL_DECODER_PMEM:
> return devm_cxl_add_pmem_region(cxlr);
> case CXL_DECODER_RAM:
> + if (cxlr->type != CXL_DECODER_HOSTONLYMEM)
> + return 0;
> +
> /*
> * The region can not be manged by CXL if any portion of
> * it is already online as 'System RAM'
Powered by blists - more mailing lists