On Mon, Sep 6, 2021 at 1:32 AM Jonathan Cameron <[email protected]> wrote: > > On Fri, 3 Sep 2021 14:49:34 -0700 > Dan Williams <[email protected]> wrote: > > > On Fri, Sep 3, 2021 at 5:53 AM Jonathan Cameron > > <[email protected]> wrote: > > > > > > On Tue, 24 Aug 2021 09:07:33 -0700 > > > Dan Williams <[email protected]> wrote: > > > > > > > Create an environment for CXL plumbing unit tests. Especially when it > > > > comes to an algorithm for HDM Decoder (Host-managed Device Memory > > > > Decoder) programming, the availability of an in-kernel-tree emulation > > > > environment for CXL configuration complexity and corner cases speeds > > > > development and deters regressions. > > > > > > > > The approach taken mirrors what was done for tools/testing/nvdimm/. I.e. > > > > an external module, cxl_test.ko built out of the tools/testing/cxl/ > > > > directory, provides mock implementations of kernel APIs and kernel > > > > objects to simulate a real world device hierarchy. > > > > > > > > One feedback for the tools/testing/nvdimm/ proposal was "why not do this > > > > in QEMU?". In fact, the CXL development community has developed a QEMU > > > > model for CXL [1]. However, there are a few blocking issues that keep > > > > QEMU from being a tight fit for topology + provisioning unit tests: > > > > > > > > 1/ The QEMU community has yet to show interest in merging any of this > > > > support that has had patches on the list since November 2020. So, > > > > testing CXL to date involves building custom QEMU with out-of-tree > > > > patches. > > > > > > That's a separate discussion I've been meaning to kick off. I'd like > > > to get that moving because there are various things we can do there > > > which can't necessarily be done with this approach or at least are easier > > > done in QEMU. I'll raise it on the qemu list and drag a few people in > > > who might be able to help us get things moving + help find solutions to > > > the bits that we can't currently do. > > > > > > > > > > > 2/ CXL mechanisms like cross-host-bridge interleave do not have a clear > > > > path to be emulated by QEMU without major infrastructure work. This > > > > is easier to achieve with the alloc_mock_res() approach taken in this > > > > patch to shortcut-define emulated system physical address ranges with > > > > interleave behavior. > > > > > > > > The QEMU enabling has been critical to get the driver off the ground, > > > > and may still move forward, but it does not address the ongoing needs of > > > > a regression testing environment and test driven development. > > > > > > Different purposes, so I would see having both as beneficial > > > > Oh certainly, especially because cxl_test skips all the PCI details. > > This regression environment is mainly for user space ABI regressions > > and the PCI agnostic machinery in the subsystem. I'd love for the QEMU > > work to move forward. > > > > > (in principle - I haven't played with this yet :) > > > > I have wondered if having a version of DOE emulation in tools/testing/ > > makes regression testing those protocols easier, but again that's PCI > > details where QEMU is more suitable. > > Maybe, but I'm not convinced yet. Particularly as the protocol complexity > that we are interested in can get pretty nasty and I'm not sure we want > the pain of implementing that anywhere near the kernel (e.g. CMA with > having to hook an SPDM implementation in). > > Could do discovery only I guess which would exercise the basics. > > > > > > > > > > > > > This patch adds an ACPI CXL Platform definition with emulated CXL > > > > multi-ported host-bridges. A follow on patch adds emulated memory > > > > expander devices. > > > > > > > > Acked-by: Ben Widawsky <[email protected]> > > > > Reported-by: Vishal Verma <[email protected]> > > > > Link: > > > > https://lore.kernel.org/r/[email protected] > > > > [1] > > > > Signed-off-by: Dan Williams <[email protected]> > > > > --- > > ... > > > > > > > > > > > + struct acpi_device *bridge = to_cxl_host_bridge(host, match); > > > > > > > > if (!bridge) > > > > return 0; > > > > @@ -319,7 +316,7 @@ static int add_host_bridge_dport(struct device > > > > *match, void *arg) > > > > struct acpi_cedt_chbs *chbs; > > > > struct cxl_port *root_port = arg; > > > > struct device *host = root_port->dev.parent; > > > > - struct acpi_device *bridge = to_cxl_host_bridge(match); > > > > + struct acpi_device *bridge = to_cxl_host_bridge(host, match); > > > > > > > > if (!bridge) > > > > return 0; > > > > @@ -371,6 +368,17 @@ static int add_root_nvdimm_bridge(struct device > > > > *match, void *data) > > > > return 1; > > > > } > > > > > > > ... > > > > > > > > > > diff --git a/tools/testing/cxl/mock_acpi.c > > > > b/tools/testing/cxl/mock_acpi.c > > > > new file mode 100644 > > > > index 000000000000..4c8a493ace56 > > > > --- /dev/null > > > > +++ b/tools/testing/cxl/mock_acpi.c > > > > @@ -0,0 +1,109 @@ > > > > > > > +static int match_add_root_port(struct pci_dev *pdev, void *data) > > > > > > Hmm. Nice not to duplicate this code, but I guess a bit tricky to > > > work around. Maybe a comment next to the 'main' version so we > > > remember to update this one as well if it is changed? > > > > I'd like to think that the __mock annotation next to the real one is > > the indication that a unit test might need updating. Sufficient? > > Agreed in general, but this particular function isn't annotated, the > caller of it is, so people have to notice that to be aware there is > a possible issue. If the change is something local to this they might > not notice.
The regression test will notice. Its primary function is to catch regressions of this nature. [..] > > > why that size? Should take window_size into account I think.. > > > > This *is* the window size, but you're right if ->interleave_ways is > > populated above and used here ->window_size can also be populated > > there. Then all that is left to do is dynamically populate the > > emulated ->base_hpa. > > Ok, so my confusion is that this code is alays using SZ_256M * ways > rather than say SZ_512M * ways. > > Perhaps a define at the top of the file or even a module parameter > to allow larger sizes? I changed this to put the size in the table definition directly so it can be edited there. The intent is for this size to be static / known to the unit test in advance. I.e. unlike QEMU testing where the test would need to be told of the configuration that was specified to the VM.
