| Message ID | 20230123121712.29892-1-Jonathan.Cameron@huawei.com |
|---|---|
| Headers | show |
| Series | hw/cxl: Passthrough HDM decoder emulation | expand |
On Mon, Jan 23, 2023 at 12:17:10PM +0000, Jonathan Cameron wrote: > Until now, testing using CXL has relied up always using two root ports > below a host bridge, to work around a current assumption in the Linux > kernel support that, in the single root port case, the implementation will > use the allowed passthrough decoder implementation choice. If that choice > is made all accesses are routed from the host bridge to the single > root port that is present. Effectively we have a pass through decoder > (it is called that in the kernel driver). > > This patch series implements that functionality and makes it the default > See patch 2 for a discussion of why I think we can make this change > without backwards compatibility issues (basically if it didn't work before > who are we breaking by making it work?) > > Whilst this limitation has been known since the initial QEMU patch > postings / kernel CXL region support, Fan Ni Ran into it recently reminding > me that we should solve it. > > https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ > > Tree with a large set of patches before this at: > https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 > > I've done some basic testing, though I did hit what appears to be > a kernel race on region bring up of existing region / namespace in a > 1HB 2RP 2EP test case. That is proving hard to replicate consistently > but doesn't seem to have anything to do with the emulation other than > perhaps we are opening up a race by responding slowly to something. > > Jonathan Cameron (2): > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > hw/cxl/cxl-host.c | 31 +++++++++++++-------- > hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- > hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ > include/hw/cxl/cxl.h | 1 + > include/hw/cxl/cxl_component.h | 1 + > include/hw/pci/pci_bridge.h | 1 + > include/hw/pci/pcie_port.h | 2 ++ > 7 files changed, 100 insertions(+), 17 deletions(-) > > -- > 2.37.2 > > Tried three different cxl topology setups (1HB1RP, 1HB2RP2Memdev, with switch), the patch works fine for me. Btw, there seem some format issues with the patch, got warnings with checkpatch tool.
On Mon, 23 Jan 2023 17:53:24 +0000 Fan Ni <fan.ni@samsung.com> wrote: > On Mon, Jan 23, 2023 at 12:17:10PM +0000, Jonathan Cameron wrote: > > > > > Until now, testing using CXL has relied up always using two root ports > > below a host bridge, to work around a current assumption in the Linux > > kernel support that, in the single root port case, the implementation will > > use the allowed passthrough decoder implementation choice. If that choice > > is made all accesses are routed from the host bridge to the single > > root port that is present. Effectively we have a pass through decoder > > (it is called that in the kernel driver). > > > > This patch series implements that functionality and makes it the default > > See patch 2 for a discussion of why I think we can make this change > > without backwards compatibility issues (basically if it didn't work before > > who are we breaking by making it work?) > > > > Whilst this limitation has been known since the initial QEMU patch > > postings / kernel CXL region support, Fan Ni Ran into it recently reminding > > me that we should solve it. > > > > https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ > > > > Tree with a large set of patches before this at: > > https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 > > > > I've done some basic testing, though I did hit what appears to be > > a kernel race on region bring up of existing region / namespace in a > > 1HB 2RP 2EP test case. That is proving hard to replicate consistently > > but doesn't seem to have anything to do with the emulation other than > > perhaps we are opening up a race by responding slowly to something. > > > > Jonathan Cameron (2): > > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > > > hw/cxl/cxl-host.c | 31 +++++++++++++-------- > > hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- > > hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ > > include/hw/cxl/cxl.h | 1 + > > include/hw/cxl/cxl_component.h | 1 + > > include/hw/pci/pci_bridge.h | 1 + > > include/hw/pci/pcie_port.h | 2 ++ > > 7 files changed, 100 insertions(+), 17 deletions(-) > > > > -- > > 2.37.2 > > > > > Tried three different cxl topology setups (1HB1RP, 1HB2RP2Memdev, with > switch), the patch works fine for me. > Btw, there seem some format issues with the patch, got warnings with > checkpatch tool. Thanks! I'll clean those up. Was being lazy on it as it's an RFC for now :) Given this is small and useful I'll probably pull it nearer the head of the queue. When I repost, if you could give a Tested-by tag that would be great! Thanks, Jonathan
On Tue, Jan 24, 2023 at 09:47:20AM +0000, Jonathan Cameron wrote: > On Mon, 23 Jan 2023 17:53:24 +0000 > Fan Ni <fan.ni@samsung.com> wrote: > > > On Mon, Jan 23, 2023 at 12:17:10PM +0000, Jonathan Cameron wrote: > > > > > > > > > Until now, testing using CXL has relied up always using two root ports > > > below a host bridge, to work around a current assumption in the Linux > > > kernel support that, in the single root port case, the implementation will > > > use the allowed passthrough decoder implementation choice. If that choice > > > is made all accesses are routed from the host bridge to the single > > > root port that is present. Effectively we have a pass through decoder > > > (it is called that in the kernel driver). > > > > > > This patch series implements that functionality and makes it the default > > > See patch 2 for a discussion of why I think we can make this change > > > without backwards compatibility issues (basically if it didn't work before > > > who are we breaking by making it work?) > > > > > > Whilst this limitation has been known since the initial QEMU patch > > > postings / kernel CXL region support, Fan Ni Ran into it recently reminding > > > me that we should solve it. > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/__;!!EwVzqGoTKBqv-0DWAJBm!WsD6FV-KV_YnhhHWLll72cHLqLQ_8kpps3MlpAa6Bonsdz6aifuWT40-QnlRyFqWyeyaVb-RiC03_qajbeCGsI5DcPYv$ > > > > > > Tree with a large set of patches before this at: > > > https://urldefense.com/v3/__https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20__;!!EwVzqGoTKBqv-0DWAJBm!WsD6FV-KV_YnhhHWLll72cHLqLQ_8kpps3MlpAa6Bonsdz6aifuWT40-QnlRyFqWyeyaVb-RiC03_qajbeCGsPjbv12T$ > > > > > > I've done some basic testing, though I did hit what appears to be > > > a kernel race on region bring up of existing region / namespace in a > > > 1HB 2RP 2EP test case. That is proving hard to replicate consistently > > > but doesn't seem to have anything to do with the emulation other than > > > perhaps we are opening up a race by responding slowly to something. > > > > > > Jonathan Cameron (2): > > > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > > > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > > > > > hw/cxl/cxl-host.c | 31 +++++++++++++-------- > > > hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- > > > hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ > > > include/hw/cxl/cxl.h | 1 + > > > include/hw/cxl/cxl_component.h | 1 + > > > include/hw/pci/pci_bridge.h | 1 + > > > include/hw/pci/pcie_port.h | 2 ++ > > > 7 files changed, 100 insertions(+), 17 deletions(-) > > > > > > -- > > > 2.37.2 > > > > > > > > Tried three different cxl topology setups (1HB1RP, 1HB2RP2Memdev, with > > switch), the patch works fine for me. > > Btw, there seem some format issues with the patch, got warnings with > > checkpatch tool. > Thanks! I'll clean those up. Was being lazy on it as it's an RFC for > now :) Given this is small and useful I'll probably pull it nearer the > head of the queue. > > When I repost, if you could give a Tested-by tag that would be great! > > Thanks, > > Jonathan > Will do. Thanks.