On Wed, Feb 17, 2021 at 08:55:12AM -0600, Jonathon Jongsma wrote:
> On Tue, 16 Feb 2021 15:20:28 -0700
> Alex Williamson <alex.williamson@xxxxxxxxxx> wrote:
>
> > On Tue, 16 Feb 2021 16:00:40 -0600
> > Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
> >
> > > On Mon, 15 Feb 2021 18:22:08 +0100
> > > Erik Skultety <eskultet@xxxxxxxxxx> wrote:
> > >
> > > > On Wed, Feb 03, 2021 at 11:38:49AM -0600, Jonathon Jongsma wrote:
> > > >
> > > > > diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> > > > > b/tests/nodedevmdevctldata/mdevctl-list-multiple.json new file
> > > > > mode 100644 index 0000000000..eefcd90c62
> > > > > --- /dev/null
> > > > > +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> > > > > @@ -0,0 +1,59 @@
> > > > > +[
> > > > > + {
> > > > > + "0000:00:02.0": [
> > > > > + {
> > > > > + "200f228a-c80a-4d50-bfb7-f5a0e4e34045": {
> > > > > + "mdev_type": "i915-GVTg_V5_4",
> > > > > + "start": "manual"
> > > > > + }
> > > > > + },
> > > > > + {
> > > > > + "de807ffc-1923-4d5f-b6c9-b20ecebc6d4b": {
> > > > > + "mdev_type": "i915-GVTg_V5_4",
> > > > > + "start": "auto"
> > > > > + }
> > > > > + },
> > > > > + {
> > > > > + "435722ea-5f43-468a-874f-da34f1217f13": {
> > > > > + "mdev_type": "i915-GVTg_V5_8",
> > > > > + "start": "manual",
> > > > > + "attrs": [
> > > > > + {
> > > > > + "testattr": "42"
> > > > > + }
> > > > > + ]
> > > > > + }
> > > > > + }
> > > > > + ]
> > > > > + },
> > > > > + {
> > > > > + "matrix": [
> > > > > + { "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
> > > > > + "mdev_type": "vfio_ap-passthrough",
> > > > > + "start": "manual",
> > > > > + "attrs": [
> > > > > + {
> > > > > + "assign_adapter": "5"
> > > > > + },
> > > > > + {
> > > > > + "assign_adapter": "6"
> > > > > + },
> > > >
> > > > I'd still like to know why there are 2 assign_adapter and 2
> > > > assign_domain attributes here, I'm simply confused what the
> > > > outcome should be.
> > >
> > > As far as I can recall, i was just trying to use some real-world-ish
> > > mdevctl output to test the parsing and handling of mdev attributes.
> > > In this case, I believe that I simply copied the example output
> > > from the mdevctl documentation. See:
> > >
> > > https://github.com/mdevctl/mdevctl#advanced-usage-attributes-and-json
> > >
> > >
> > > >
> > > > > + {
> > > > > + "assign_domain": "0xab"
> > > > > + },
> > > > > + {
> > > > > + "assign_control_domain": "0xab"
> > > > > + },
> > > > > + {
> > > > > + "assign_domain": "4"
> > > > > + },
> > > > > + {
> > > > > + "assign_control_domain": "4"
> > > > > + }
> > > > > + ]
> > > > > + }
> > > > > + }
> > > > > + ]
> > > > > + }
> > > > > +]
> > > > > +
> > > >
> > > >
> > > > ...
> > > >
> > > >
> > > > > + <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name>
> > > > > + <parent>matrix</parent>
> > > > > + <capability type='mdev'>
> > > > > + <type id='vfio_ap-passthrough'/>
> > > > > + <iommuGroup number='0'/>
> > > > > + <attr name='assign_adapter' value='5'/>
> > > > > + <attr name='assign_adapter' value='6'/>
> > > > > + <attr name='assign_domain' value='0xab'/>
> > > > > + <attr name='assign_control_domain' value='0xab'/>
> > > > > + <attr name='assign_domain' value='4'/>
> > > > > + <attr name='assign_control_domain' value='4'/>
> > > >
> > > > Here too I'd like to hear an opinion (since v3) on naming the
> > > > attributes in such way that they correspond to the respective
> > > > elements: ap-adapter, ap-domain, etc. This naming is very
> > > > unintuitive if not documented properly; unless there's an internal
> > > > reason why they have to be named "assign_control", etc.
> > >
> > > These are the names of the attributes that are used to configure
> > > these devices in sysfs:
> > > https://www.ibm.com/support/knowledgecenter/linuxonibm/com.ibm.linux.z.ljdd/ljdd_c_vfio_bind_ap.html
> > >
> > > The idea here is that <attr> is a direct mapping to and from the
> > > mdev sysfs attributes, since that is how these devices are
> > > configured. An attribute name means nothing to libvirt, it is just
> > > an opaque name that we pass to mdevctl. If we were to deviate from
> > > this strict mapping and add "friendlier" names in libvirt, we would
> > > need to maintain a custom per-device mapping from mdev sysfs
> > > attribute name to libvirt friendly-name. That seems unmaintainable.
> > >
> >
> > Yep, and the names don't mean anything to mdevctl either, as you say
> > they're just sysfs attributes, mdevctl looks for the named attribute
> > and writes the value to it. Note that ordering of the attributes
> > might be important, which is why mdevctl stores them in an array and
> > provides some utility to re-index attributes. I can't tell if the
> > above example necessarily imposes that ordering from the xml. Thanks,
> >
>
> Yes, the order of xml attributes is significant, they should be passed
> to mdevctl in the same order as they are defined in the xml. Though I
> don't think I've documented that anywhere...
Okay, those are fair arguments, so we need to document the usage in libvirt
properly.
Erik
