* Re: [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model
2024-10-14 9:36 ` [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model Jonathan Cameron
@ 2024-10-30 6:01 ` Klaus Jensen
0 siblings, 0 replies; 2+ messages in thread
From: Klaus Jensen @ 2024-10-30 6:01 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Corey Minyard, Andrew Jeffery, Matt Johnston, Corey Minyard,
Paolo Bonzini, Peter Maydell, Jason Wang, Keith Busch,
Lior Weintraub, Jeremy Kerr, Peter Delevoryas, qemu-devel,
qemu-arm, qemu-block, Klaus Jensen, Davidlohr Bueso, linux-cxl,
Hannes Reinecke
[-- Attachment #1: Type: text/plain, Size: 5515 bytes --]
On Oct 14 10:36, Jonathan Cameron via wrote:
> On Wed, 20 Sep 2023 09:36:34 -0500
> Corey Minyard <minyard@acm.org> wrote:
>
> > On Wed, Sep 20, 2023 at 06:31:25AM -0700, Klaus Jensen wrote:
> > > On Sep 20 07:54, Corey Minyard wrote:
> > > > On Wed, Sep 20, 2023 at 12:48:03PM +0100, Jonathan Cameron via wrote:
> > > > > On Thu, 14 Sep 2023 11:53:40 +0200
> > > > > Klaus Jensen <its@irrelevant.dk> wrote:
> > > > >
> > > > > > This adds a generic MCTP endpoint model that other devices may derive
> > > > > > from.
> > > > > >
> > > > > > Also included is a very basic implementation of an NVMe-MI device,
> > > > > > supporting only a small subset of the required commands.
> > > > > >
> > > > > > Since this all relies on i2c target mode, this can currently only be
> > > > > > used with an SoC that includes the Aspeed I2C controller.
> > > > > >
> > > > > > The easiest way to get up and running with this, is to grab my buildroot
> > > > > > overlay[1] (aspeed_ast2600evb_nmi_defconfig). It includes modified a
> > > > > > modified dts as well as a couple of required packages.
> > > > > >
> > > > > > QEMU can then be launched along these lines:
> > > > > >
> > > > > > qemu-system-arm \
> > > > > > -nographic \
> > > > > > -M ast2600-evb \
> > > > > > -kernel output/images/zImage \
> > > > > > -initrd output/images/rootfs.cpio \
> > > > > > -dtb output/images/aspeed-ast2600-evb-nmi.dtb \
> > > > > > -nic user,hostfwd=tcp::2222-:22 \
> > > > > > -device nmi-i2c,address=0x3a \
> > > > > > -serial mon:stdio
> > > > > >
> > > > > > From within the booted system,
> > > > > >
> > > > > > mctp addr add 8 dev mctpi2c15
> > > > > > mctp link set mctpi2c15 up
> > > > > > mctp route add 9 via mctpi2c15
> > > > > > mctp neigh add 9 dev mctpi2c15 lladdr 0x3a
> > > > > > mi-mctp 1 9 info
> > > > > >
> > > > > > Comments are very welcome!
> > > > > >
> > > > > > [1]: https://github.com/birkelund/hwtests/tree/main/br2-external
> > > > > >
> > > > > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > > > >
> > > > > Hi Klaus,
> > > > >
> > > > > Silly question, but who is likely to pick this up? + likely to be soon?
> > > > >
> > > > > I'm going to post the CXL stuff that makes use of the core support shortly
> > > > > and whilst I can point at this patch set on list, I'd keen to see it upstream
> > > > > to reduce the dependencies (it's got 2 sets ahead of it of CXL stuff
> > > > > anyway but that will all hopefully go through Michael Tsirkin's tree
> > > > > for PCI stuff in one go).
> > > >
> > > > I can pick it up, but he can just request a merge, too.
> > > >
> > > > I did have a question I asked earlier about tests. It would be unusual
> > > > at this point to add something like this without having some tests,
> > > > especially injecting invalid data.
> > > >
> > >
> > > Hi all,
> > >
> > > Sorry for the late reply. I'm currently at SDC, but I will write up some
> > > tests when I get back to in the office on Monday.
> > >
> > > Corey, what kinds of tests would be best here? Avocado "acceptance"
> > > tests or would you like to see something lower level?
> >
> > My main concern is testing what happens when bad data gets injected, to
> > avoid people coming up with clever names for exploits in qemu. It's not
> > so much for this code, it's for the changes that comes in the future.
> >
> > And, of course, normal functional tests to make sure it works. What a
> > friend of mine calls "dead chicken" tests. You wave a dead chicken at
> > it, and if the chicken is still dead everything is ok :).
> >
> > I'm fine with either type of tests, but I'm not sure you can do this
> > with avocado. It's probably about the same amount of work either path
> > you choose.
> >
> > -corey
>
> Hi Klaus, All,
>
> I was looking at what dependencies I'm carrying on my CXL tree and this
> series is one of the bigger bits :(
>
> Any plans to take it forwards?
> I have some other stuff to solve to have a fully upstream QEMU
> solution for the CXL fm-api over mctp (direct from host anyway), but
> if this is blocked indefinitely tackling how to get a controller onto
> a typical server system isn't going to be productive :(
>
> As Davidlohr called out at in the CXL LPC Uconf [1] this is really handy
> for testing his work on libcxlmi. A number of people are looking
> at more sophisticated CXL fabric emulation and that will also need
> us to close this gap!
>
> No promises but maybe we can find someone to help with adding tests
> if that's the only remaining blocker.
>
Yes, I believe the lack of tests are the blocker currently. I've mostly
been testing this through a buildroot. That allows me to at least kick
it with invalid fields and so on, but it all depends on the linux mctp
driver doing the bulk of the work (and it's not really supposed to do
"bad stuff"), so it's limited how low we can go.
While I have the infrastructure to do that, I'm really not sure where to
start if we want to test what happens when the emulated mctp device gets
bad packets. I don't think we can instruct the mctp subsystem in linux
to do that easily. And if we do not use it (to forcefully write invalid
packets on the bus), I'm not sure how to deal with the required target
mode.
Would anyone know if the i2c-slave-testunit could be viable way forward
to test this?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread