Re: [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model

[Corey Minyard <minyard@acm.org>]

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