Re: [PATCH] docs/cxl: Add serial number for persistent-memdev

Re: [PATCH] docs/cxl: Add serial number for persistent-memdev

[Yuquan Wang]

> 
> On Tue, 4 Mar 2025 14:22:48 +0800
> Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> 
> > > 
> > > On Thu, Feb 20, 2025 at 04:12:13PM +0000, Jonathan Cameron wrote:  
> > > > On Mon, 17 Feb 2025 19:20:39 +0800
> > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > >   
> > > > > Add serial number parameter in the cxl persistent examples.
> > > > > 
> > > > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>  
> > > > Looks good.  I've queued it up on my gitlab staging tree, but
> > > > Michael if you want to pick this one directly that's fine as well.  
> > > 
> > > See no reason to, I was not even CC'd.  
> > 
> > Hi, Michael
> > 
> > I'm sorry, this is my fault. I used "get_maintainer.pl" to check this
> > patch's maintainers but it shows "No maintainers found, printing recent
> > contributors". 
> > 
> I usually stage up multiple series together and send on to Michael.
> So it was be being lazy for a minor change rather than anything much
> that you did wrong.
> 
> If I get time I'll post a series with this a few other patches
> later today.  
> 
> Jonathan
> 
Thank you!

BTW, I found a corner case in CXL numa node creation.

Condition: 
1) A UMA/NUMA system without SRAT, but with CEDT.CFMWS
2）Enable CONFIG_ACPI_NUMA

Results:
1) acpi_numa_init: the fake_pxm will be 0 and send to acpi_parse_cfmws()
2）If dynamically create cxl ram region, the cxl memory would be assigned
to node0 rather than a new node

Confusions:
1) Is a numa system a requirement for CXL memory usage?
2) Should we forbid this situation by adding fake_pxm check and returning
error in acpi_numa_init()? 
3）Or we can add some kernel code to allow create these fake nodes on a
system without SRAT?

Yuquan





信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。
Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.

Re: [PATCH] docs/cxl: Add serial number for persistent-memdev

[Jonathan Cameron]

On Wed, 5 Mar 2025 18:35:40 +0800
Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:

> > 
> > On Tue, 4 Mar 2025 14:22:48 +0800
> > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> >   
> > > > 
> > > > On Thu, Feb 20, 2025 at 04:12:13PM +0000, Jonathan Cameron wrote:    
> > > > > On Mon, 17 Feb 2025 19:20:39 +0800
> > > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > > >     
> > > > > > Add serial number parameter in the cxl persistent examples.
> > > > > > 
> > > > > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>    
> > > > > Looks good.  I've queued it up on my gitlab staging tree, but
> > > > > Michael if you want to pick this one directly that's fine as well.    
> > > > 
> > > > See no reason to, I was not even CC'd.    
> > > 
> > > Hi, Michael
> > > 
> > > I'm sorry, this is my fault. I used "get_maintainer.pl" to check this
> > > patch's maintainers but it shows "No maintainers found, printing recent
> > > contributors". 
> > >   
> > I usually stage up multiple series together and send on to Michael.
> > So it was be being lazy for a minor change rather than anything much
> > that you did wrong.
> > 
> > If I get time I'll post a series with this a few other patches
> > later today.  
> > 
> > Jonathan
> >   
> Thank you!
> 
> BTW, I found a corner case in CXL numa node creation.
> 
> Condition: 
> 1) A UMA/NUMA system without SRAT, but with CEDT.CFMWS
> 2）Enable CONFIG_ACPI_NUMA
> 
> Results:
> 1) acpi_numa_init: the fake_pxm will be 0 and send to acpi_parse_cfmws()
> 2）If dynamically create cxl ram region, the cxl memory would be assigned
> to node0 rather than a new node
> 
> Confusions:
> 1) Is a numa system a requirement for CXL memory usage?

Obviously discussion has gone on elsewhere, but I'd say in general it
would be a bad idea to not have an SRAT because the moment we add CXL
it is definitely a NUMA system and we want the Generic Port entry to
allow us to get perf information.

So I wouldn't mind if we fail CXL init in this case, but maybe
it is worth papering over things.

Jonathan


> 2) Should we forbid this situation by adding fake_pxm check and returning
> error in acpi_numa_init()? 
> 3）Or we can add some kernel code to allow create these fake nodes on a
> system without SRAT?
> 
> Yuquan
> 
> 
> 
> 
> 
> 信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。
> Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.

Re: [PATCH] docs/cxl: Add serial number for persistent-memdev

[Dan Williams]

Jonathan Cameron wrote:
> On Wed, 5 Mar 2025 18:35:40 +0800
> Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> 
> > > 
> > > On Tue, 4 Mar 2025 14:22:48 +0800
> > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > >   
> > > > > 
> > > > > On Thu, Feb 20, 2025 at 04:12:13PM +0000, Jonathan Cameron wrote:    
> > > > > > On Mon, 17 Feb 2025 19:20:39 +0800
> > > > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > > > >     
> > > > > > > Add serial number parameter in the cxl persistent examples.
> > > > > > > 
> > > > > > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>    
> > > > > > Looks good.  I've queued it up on my gitlab staging tree, but
> > > > > > Michael if you want to pick this one directly that's fine as well.    
> > > > > 
> > > > > See no reason to, I was not even CC'd.    
> > > > 
> > > > Hi, Michael
> > > > 
> > > > I'm sorry, this is my fault. I used "get_maintainer.pl" to check this
> > > > patch's maintainers but it shows "No maintainers found, printing recent
> > > > contributors". 
> > > >   
> > > I usually stage up multiple series together and send on to Michael.
> > > So it was be being lazy for a minor change rather than anything much
> > > that you did wrong.
> > > 
> > > If I get time I'll post a series with this a few other patches
> > > later today.  
> > > 
> > > Jonathan
> > >   
> > Thank you!
> > 
> > BTW, I found a corner case in CXL numa node creation.
> > 
> > Condition: 
> > 1) A UMA/NUMA system without SRAT, but with CEDT.CFMWS
> > 2）Enable CONFIG_ACPI_NUMA
> > 
> > Results:
> > 1) acpi_numa_init: the fake_pxm will be 0 and send to acpi_parse_cfmws()
> > 2）If dynamically create cxl ram region, the cxl memory would be assigned
> > to node0 rather than a new node
> > 
> > Confusions:
> > 1) Is a numa system a requirement for CXL memory usage?
> 
> Obviously discussion has gone on elsewhere, but I'd say in general it
> would be a bad idea to not have an SRAT because the moment we add CXL
> it is definitely a NUMA system and we want the Generic Port entry to
> allow us to get perf information.
> 
> So I wouldn't mind if we fail CXL init in this case, but maybe
> it is worth papering over things.

I think that is too severe. If a driver has a path to advertise
resources, even in a less than ideal way, it should make every effort to
do that. There are plenty of ways for the NUMA information to fail, that
does not mean the memory needs to be prevented from coming online. Let
the end user decide if lack of performance information is fatal.

Re: [PATCH] docs/cxl: Add serial number for persistent-memdev

[Jonathan Cameron]

On Fri, 14 Mar 2025 08:44:05 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> > On Wed, 5 Mar 2025 18:35:40 +0800
> > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> >   
> > > > 
> > > > On Tue, 4 Mar 2025 14:22:48 +0800
> > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > >     
> > > > > > 
> > > > > > On Thu, Feb 20, 2025 at 04:12:13PM +0000, Jonathan Cameron wrote:      
> > > > > > > On Mon, 17 Feb 2025 19:20:39 +0800
> > > > > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > > > > >       
> > > > > > > > Add serial number parameter in the cxl persistent examples.
> > > > > > > > 
> > > > > > > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>      
> > > > > > > Looks good.  I've queued it up on my gitlab staging tree, but
> > > > > > > Michael if you want to pick this one directly that's fine as well.      
> > > > > > 
> > > > > > See no reason to, I was not even CC'd.      
> > > > > 
> > > > > Hi, Michael
> > > > > 
> > > > > I'm sorry, this is my fault. I used "get_maintainer.pl" to check this
> > > > > patch's maintainers but it shows "No maintainers found, printing recent
> > > > > contributors". 
> > > > >     
> > > > I usually stage up multiple series together and send on to Michael.
> > > > So it was be being lazy for a minor change rather than anything much
> > > > that you did wrong.
> > > > 
> > > > If I get time I'll post a series with this a few other patches
> > > > later today.  
> > > > 
> > > > Jonathan
> > > >     
> > > Thank you!
> > > 
> > > BTW, I found a corner case in CXL numa node creation.
> > > 
> > > Condition: 
> > > 1) A UMA/NUMA system without SRAT, but with CEDT.CFMWS
> > > 2）Enable CONFIG_ACPI_NUMA
> > > 
> > > Results:
> > > 1) acpi_numa_init: the fake_pxm will be 0 and send to acpi_parse_cfmws()
> > > 2）If dynamically create cxl ram region, the cxl memory would be assigned
> > > to node0 rather than a new node
> > > 
> > > Confusions:
> > > 1) Is a numa system a requirement for CXL memory usage?  
> > 
> > Obviously discussion has gone on elsewhere, but I'd say in general it
> > would be a bad idea to not have an SRAT because the moment we add CXL
> > it is definitely a NUMA system and we want the Generic Port entry to
> > allow us to get perf information.
> > 
> > So I wouldn't mind if we fail CXL init in this case, but maybe
> > it is worth papering over things.  
> 
> I think that is too severe. If a driver has a path to advertise
> resources, even in a less than ideal way, it should make every effort to
> do that. There are plenty of ways for the NUMA information to fail, that
> does not mean the memory needs to be prevented from coming online. Let
> the end user decide if lack of performance information is fatal.

You are too nice to those firmware folk ;)  How will they learn!

Everything in default node is fine.  I don't much like the having
broken normal numa setup combined with CXL trying to carry on with
its node. I suspect that will be fragile in the long run.

numa_off and other things set in that path tend to spread there wings
into surprising places and broken / missing SRAT + CXL is something
that isn't likely to get much testing.

Jonathan


> 

Re: [PATCH] docs/cxl: Add serial number for persistent-memdev

[Yuquan Wang]

> -----原始邮件-----
> 发件人: "Jonathan Cameron" <Jonathan.Cameron@huawei.com>
> 发送时间:2025-03-13 02:10:35 (星期四)
> 收件人: "Yuquan Wang" <wangyuquan1236@phytium.com.cn>
> 抄送: qemu-devel@nongnu.org, linux-cxl@vger.kernel.org
> 主题: Re: [PATCH] docs/cxl: Add serial number for persistent-memdev
> 
> On Wed, 5 Mar 2025 18:35:40 +0800
> Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> 
> > > 
> > > On Tue, 4 Mar 2025 14:22:48 +0800
> > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > >   
> > > > > 
> > > > > On Thu, Feb 20, 2025 at 04:12:13PM +0000, Jonathan Cameron wrote:    
> > > > > > On Mon, 17 Feb 2025 19:20:39 +0800
> > > > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > > > >     
> > > > > > > Add serial number parameter in the cxl persistent examples.
> > > > > > > 
> > > > > > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>    
> > > > > > Looks good.  I've queued it up on my gitlab staging tree, but
> > > > > > Michael if you want to pick this one directly that's fine as well.    
> > > > > 
> > > > > See no reason to, I was not even CC'd.    
> > > > 
> > > > Hi, Michael
> > > > 
> > > > I'm sorry, this is my fault. I used "get_maintainer.pl" to check this
> > > > patch's maintainers but it shows "No maintainers found, printing recent
> > > > contributors". 
> > > >   
> > > I usually stage up multiple series together and send on to Michael.
> > > So it was be being lazy for a minor change rather than anything much
> > > that you did wrong.
> > > 
> > > If I get time I'll post a series with this a few other patches
> > > later today.  
> > > 
> > > Jonathan
> > >   
> > Thank you!
> > 
> > BTW, I found a corner case in CXL numa node creation.
> > 
> > Condition: 
> > 1) A UMA/NUMA system without SRAT, but with CEDT.CFMWS
> > 2）Enable CONFIG_ACPI_NUMA
> > 
> > Results:
> > 1) acpi_numa_init: the fake_pxm will be 0 and send to acpi_parse_cfmws()
> > 2）If dynamically create cxl ram region, the cxl memory would be assigned
> > to node0 rather than a new node
> > 
> > Confusions:
> > 1) Is a numa system a requirement for CXL memory usage?
> 
> Obviously discussion has gone on elsewhere, but I'd say in general it
> would be a bad idea to not have an SRAT because the moment we add CXL
> it is definitely a NUMA system and we want the Generic Port entry to
> allow us to get perf information.
> 
> So I wouldn't mind if we fail CXL init in this case, but maybe
> it is worth papering over things.
> 
> Jonathan
> 

Hi, Jonathan

Recentlty I managed to do some hot-plug tests on cxl type3 device on QEMU.
I tried use "device add" qemu command in monitor, but it failed. I also used
unbind/bind cxl_pci driver in sysfs, I can see the software flow on device but
no expected actions on cxl root ports linked(like pcie hot-plug interrupt and
so on).

Could we simulate a hot-add flow of type3 device in qemu now? Maybe I used the
wrong method :(

Yuquan


信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。
Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.

Re: [PATCH] docs/cxl: Add serial number for persistent-memdev

[Jonathan Cameron]

On Tue, 25 Mar 2025 15:49:37 +0800
Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:

> > -----原始邮件-----
> > 发件人: "Jonathan Cameron" <Jonathan.Cameron@huawei.com>
> > 发送时间:2025-03-13 02:10:35 (星期四)
> > 收件人: "Yuquan Wang" <wangyuquan1236@phytium.com.cn>
> > 抄送: qemu-devel@nongnu.org, linux-cxl@vger.kernel.org
> > 主题: Re: [PATCH] docs/cxl: Add serial number for persistent-memdev
> > 
> > On Wed, 5 Mar 2025 18:35:40 +0800
> > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> >   
> > > > 
> > > > On Tue, 4 Mar 2025 14:22:48 +0800
> > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > >     
> > > > > > 
> > > > > > On Thu, Feb 20, 2025 at 04:12:13PM +0000, Jonathan Cameron wrote:      
> > > > > > > On Mon, 17 Feb 2025 19:20:39 +0800
> > > > > > > Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:
> > > > > > >       
> > > > > > > > Add serial number parameter in the cxl persistent examples.
> > > > > > > > 
> > > > > > > > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>      
> > > > > > > Looks good.  I've queued it up on my gitlab staging tree, but
> > > > > > > Michael if you want to pick this one directly that's fine as well.      
> > > > > > 
> > > > > > See no reason to, I was not even CC'd.      
> > > > > 
> > > > > Hi, Michael
> > > > > 
> > > > > I'm sorry, this is my fault. I used "get_maintainer.pl" to check this
> > > > > patch's maintainers but it shows "No maintainers found, printing recent
> > > > > contributors". 
> > > > >     
> > > > I usually stage up multiple series together and send on to Michael.
> > > > So it was be being lazy for a minor change rather than anything much
> > > > that you did wrong.
> > > > 
> > > > If I get time I'll post a series with this a few other patches
> > > > later today.  
> > > > 
> > > > Jonathan
> > > >     
> > > Thank you!
> > > 
> > > BTW, I found a corner case in CXL numa node creation.
> > > 
> > > Condition: 
> > > 1) A UMA/NUMA system without SRAT, but with CEDT.CFMWS
> > > 2）Enable CONFIG_ACPI_NUMA
> > > 
> > > Results:
> > > 1) acpi_numa_init: the fake_pxm will be 0 and send to acpi_parse_cfmws()
> > > 2）If dynamically create cxl ram region, the cxl memory would be assigned
> > > to node0 rather than a new node
> > > 
> > > Confusions:
> > > 1) Is a numa system a requirement for CXL memory usage?  
> > 
> > Obviously discussion has gone on elsewhere, but I'd say in general it
> > would be a bad idea to not have an SRAT because the moment we add CXL
> > it is definitely a NUMA system and we want the Generic Port entry to
> > allow us to get perf information.
> > 
> > So I wouldn't mind if we fail CXL init in this case, but maybe
> > it is worth papering over things.
> > 
> > Jonathan
> >   
> 
> Hi, Jonathan
> 
> Recentlty I managed to do some hot-plug tests on cxl type3 device on QEMU.
> I tried use "device add" qemu command in monitor, but it failed. I also used
> unbind/bind cxl_pci driver in sysfs, I can see the software flow on device but
> no expected actions on cxl root ports linked(like pcie hot-plug interrupt and
> so on).
> 
> Could we simulate a hot-add flow of type3 device in qemu now? Maybe I used the
> wrong method :(

Only tweaks needed should be to set hotplug=true for the root port or switch
downstream ports and then via the qemu monitor something like:
device_add cxl-type3,bus_cxl_rp_port1,volatile-memdev=cxl-mem3,id=cxl-memD,sn=5
and you should see hotplug occur.

I just tested this on an arm64 setup (using my staging tree) but shouldn't
make any real difference as all native hotplug flows.

 pcieport 0000:0c:01.0: pciehp: Slot(3): Button press: will power on in 5 sec
 pcieport 0000:0c:01.0: pciehp: Slot(3): Card present
 pcieport 0000:0c:01.0: pciehp: Slot(3): Link Up
 pci 0000:0e:00.0: [8086:0d93] type 00 class 0x050210 PCIe Endpoint
 pci 0000:0e:00.0: BAR 0 [mem 0x00000000-0x0000ffff 64bit]
 pci 0000:0e:00.0: BAR 2 [mem 0x00000000-0x0003ffff 64bit]
 pci 0000:0e:00.0: BAR 4 [mem 0x00000000-0x00000fff]
 pci 0000:0e:00.0: enabling Extended Tags
 pcieport 0000:0c:01.0: bridge window [io  0x1000-0x0fff] to [bus 0e] add_size 1000
 pcieport 0000:0c:01.0: bridge window [mem 0x00100000-0x000fffff 64bit pref] to [bus 0e] add_size 200000 add_align 100000
 pcieport 0000:0c:01.0: bridge window [mem size 0x00200000 64bit pref]: can't assign; no space
 pcieport 0000:0c:01.0: bridge window [mem size 0x00200000 64bit pref]: failed to assign
 pcieport 0000:0c:01.0: bridge window [io  size 0x1000]: can't assign; no space
 pcieport 0000:0c:01.0: bridge window [io  size 0x1000]: failed to assign
 pcieport 0000:0c:01.0: bridge window [mem size 0x00200000 64bit pref]: can't assign; no space
 pcieport 0000:0c:01.0: bridge window [mem size 0x00200000 64bit pref]: failed to assign
 pcieport 0000:0c:01.0: bridge window [io  size 0x1000]: can't assign; no space
 pcieport 0000:0c:01.0: bridge window [io  size 0x1000]: failed to assign
 pci 0000:0e:00.0: BAR 2 [mem 0x20000000-0x2003ffff 64bit]: assigned
 pci 0000:0e:00.0: BAR 0 [mem 0x20040000-0x2004ffff 64bit]: assigned
 pci 0000:0e:00.0: BAR 4 [mem 0x20050000-0x20050fff]: assigned
 pcieport 0000:0c:01.0: PCI bridge to [bus 0e]
 pcieport 0000:0c:01.0:   bridge window [mem 0x20000000-0x27ffffff]
 cxl_pci 0000:0e:00.0: enabling device (0000 -> 0002)

Whilst there are some corners where resource assignment actually fails
(various fixes have merged recently so maybe that all works now).
In this case it succeeds after a few tries (it reduces the requested
padding in this case I think but I haven't chased this one through).

If you are still having trouble I can fire up a test case on x86 but
probably not today.

Jonathan



> 
> Yuquan
> 
> 
> 信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。
> Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.