All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron via <qemu-arm@nongnu.org>
To: Yuquan Wang <wangyuquan1236@phytium.com.cn>
Cc: qemu-arm <qemu-arm@nongnu.org>, <qemu-devel@nongnu.org>
Subject: Re: CXL Namespaces of ACPI disappearing in Qemu demo
Date: Tue, 22 Aug 2023 16:23:03 +0100	[thread overview]
Message-ID: <20230822162303.00007def@Huawei.com> (raw)
In-Reply-To: <2023081118312729037834@phytium.com.cn>

On Fri, 11 Aug 2023 18:31:28 +0800
Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:

> Hi, 
> On 2023-08-10 21:56,  jonathan.cameron wrote:
>  
> So took a look at your issue - be it on the cxl-2023-08-07 branch rebased on qemu/master
> from today (side effect of looking at the segfault that was stopping me getting to this).
>  
> For me at least the branch does create an ACPI0017 DSDT entry and an ACPI0016 one
> and all the CXL devices turn up in /sys/bus/cxl/devices as expected.
>  
> 
> Oh, thanks for your guidance. It works so now I can get  ACPI0017 & ACPI0016 information in DSDT.   : )
> 
> By the way,  I found that if we add a pcie root port which create the same bus number as we assigned to pxb-cxl, 
> the enumeration of cxl and pcie would be different from what we expected.  In this case,  we cannot find 
> CXL devices in /sys/bus/cxl/devices. 

So this seems to be a case of shooting ourselves in the foot, but not catching the nonsensical configuration
(as you observer later! :)
pxb-pcie complains if you try and add two at the same bus number, but that doesn't protect against overlapping
ranges because they aren't known until after enumeration (which is done by the bios - and I guess the bios
doesn't sanity check for this insanity).  Qemu could take another look when it builds the ACPI tables a
second time though.

Looking at edk2 logs I can see it is happily populating the root bus 1 on my arm64 setup and that it
observes there are no subordinate buses available for the main PCIe bus (0) that QEMU is creating by
default. The _CRS entries look correct but the kernel ignores them it seems. 

It is very much not a valid configuration so there is no reason the kernel should cope with it.

Maybe it's worth considering some hardening code?


> 
> According to my test, the error happened in 
> "devm_cxl_register_pci_bus()"  of  "add_host_bridge_uport" in  "cxl_acpi_probe".
> Actually,  in above case, the incorrect enumeration of pcie will also occur with pxb-pcie except for pxb-cxl, 
> hence I guess the kernel did not deal with such case and users just need to avoid it if they need a correct
> enumeration result.

Agreed - Protecting against ever corner case of impossible configuration is tricky to
do.

> 
> My qemu script (which will cause the incorrect enumeration):
> qemu-system-x86_64 \
> -M q35,nvdimm=on,cxl=on \
> -m 4G,maxmem=8G,slots=8 \
> -smp 1 \
> -object memory-backend-file,id=cxl-mem1,share=on,mem-path=./memfile/cxltest3.raw,size=256M \
> -object memory-backend-file,id=cxl-lsa1,share=on,mem-path=./memfile/lsa3.raw,size=256M \
> -device ioh3420,bus=pcie.0,id=root_port1,chassis=0,slot=1,addr=04 \
> -device qemu-xhci,bus=root_port1 \
> -device pxb-cxl,bus_nr=1,bus=pcie.0,id=cxl.1 \
> -device cxl-rp,port=0,bus=cxl.1,id=root_port13,chassis=0,slot=2 \
> -device cxl-type3,bus=root_port13,memdev=cxl-mem1,lsa=cxl-lsa1,id=cxl-pmem0 \
> -M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G \
> ......
> 
> Many thanks
> Yuquan


WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron via <qemu-devel@nongnu.org>
To: Yuquan Wang <wangyuquan1236@phytium.com.cn>
Cc: qemu-arm <qemu-arm@nongnu.org>, <qemu-devel@nongnu.org>
Subject: Re: CXL Namespaces of ACPI disappearing in Qemu demo
Date: Tue, 22 Aug 2023 16:23:03 +0100	[thread overview]
Message-ID: <20230822162303.00007def@Huawei.com> (raw)
In-Reply-To: <2023081118312729037834@phytium.com.cn>

On Fri, 11 Aug 2023 18:31:28 +0800
Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:

> Hi, 
> On 2023-08-10 21:56,  jonathan.cameron wrote:
>  
> So took a look at your issue - be it on the cxl-2023-08-07 branch rebased on qemu/master
> from today (side effect of looking at the segfault that was stopping me getting to this).
>  
> For me at least the branch does create an ACPI0017 DSDT entry and an ACPI0016 one
> and all the CXL devices turn up in /sys/bus/cxl/devices as expected.
>  
> 
> Oh, thanks for your guidance. It works so now I can get  ACPI0017 & ACPI0016 information in DSDT.   : )
> 
> By the way,  I found that if we add a pcie root port which create the same bus number as we assigned to pxb-cxl, 
> the enumeration of cxl and pcie would be different from what we expected.  In this case,  we cannot find 
> CXL devices in /sys/bus/cxl/devices. 

So this seems to be a case of shooting ourselves in the foot, but not catching the nonsensical configuration
(as you observer later! :)
pxb-pcie complains if you try and add two at the same bus number, but that doesn't protect against overlapping
ranges because they aren't known until after enumeration (which is done by the bios - and I guess the bios
doesn't sanity check for this insanity).  Qemu could take another look when it builds the ACPI tables a
second time though.

Looking at edk2 logs I can see it is happily populating the root bus 1 on my arm64 setup and that it
observes there are no subordinate buses available for the main PCIe bus (0) that QEMU is creating by
default. The _CRS entries look correct but the kernel ignores them it seems. 

It is very much not a valid configuration so there is no reason the kernel should cope with it.

Maybe it's worth considering some hardening code?


> 
> According to my test, the error happened in 
> "devm_cxl_register_pci_bus()"  of  "add_host_bridge_uport" in  "cxl_acpi_probe".
> Actually,  in above case, the incorrect enumeration of pcie will also occur with pxb-pcie except for pxb-cxl, 
> hence I guess the kernel did not deal with such case and users just need to avoid it if they need a correct
> enumeration result.

Agreed - Protecting against ever corner case of impossible configuration is tricky to
do.

> 
> My qemu script (which will cause the incorrect enumeration):
> qemu-system-x86_64 \
> -M q35,nvdimm=on,cxl=on \
> -m 4G,maxmem=8G,slots=8 \
> -smp 1 \
> -object memory-backend-file,id=cxl-mem1,share=on,mem-path=./memfile/cxltest3.raw,size=256M \
> -object memory-backend-file,id=cxl-lsa1,share=on,mem-path=./memfile/lsa3.raw,size=256M \
> -device ioh3420,bus=pcie.0,id=root_port1,chassis=0,slot=1,addr=04 \
> -device qemu-xhci,bus=root_port1 \
> -device pxb-cxl,bus_nr=1,bus=pcie.0,id=cxl.1 \
> -device cxl-rp,port=0,bus=cxl.1,id=root_port13,chassis=0,slot=2 \
> -device cxl-type3,bus=root_port13,memdev=cxl-mem1,lsa=cxl-lsa1,id=cxl-pmem0 \
> -M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G \
> ......
> 
> Many thanks
> Yuquan



  reply	other threads:[~2023-08-22 15:23 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-16  7:43 A confusion about CXL in arm virt machine Yuquan Wang
2023-06-16 18:10 ` Gregory Price
2023-06-19  9:58   ` Jonathan Cameron via
2023-06-19  9:58     ` Jonathan Cameron via
2023-08-10  9:30     ` CXL Namespaces of ACPI disappearing in Qemu demo Yuquan Wang
2023-08-10 10:04       ` Jonathan Cameron via
2023-08-10 13:56         ` Jonathan Cameron via
2023-08-11 10:31           ` Yuquan Wang
2023-08-22 15:23             ` Jonathan Cameron via [this message]
2023-08-22 15:23               ` Jonathan Cameron via
2023-09-18 12:41     ` A confusion about CXL in arm virt machine Peter Maydell
2023-09-18 15:03       ` Jonathan Cameron via
2023-09-18 15:03         ` Jonathan Cameron via
  -- strict thread matches above, loose matches on Subject: below --
2023-08-22  7:22 CXL Namespaces of ACPI disappearing in Qemu demo Yuquan Wang
2023-08-23 19:44 ` Gregory Price
2023-08-24  1:11   ` Yuquan Wang
2023-08-24  9:06   ` Jonathan Cameron via
2023-09-04 10:27     ` Yuquan Wang
2023-09-04 12:43       ` Jonathan Cameron via
2023-09-04 12:43         ` Jonathan Cameron via
2023-09-05 10:45         ` Yuquan Wang
2023-09-05 14:34           ` Jonathan Cameron via
2023-09-05 14:34             ` Jonathan Cameron via
2023-09-06 11:22             ` Yuquan Wang
2023-09-07 10:58               ` Jonathan Cameron via
2023-09-07 10:58                 ` Jonathan Cameron via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230822162303.00007def@Huawei.com \
    --to=qemu-arm@nongnu.org \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=qemu-devel@nongnu.org \
    --cc=wangyuquan1236@phytium.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.