[libvirt PATCH v5 5/7] Add PCI VPD Capability Support

[Dmitrii Shcherbakov]

> >
> > I've got a general comment about what do any of these 2-letter
> > keywords actually mean. I presume they are explaned in the
> > PCI spec, but AFAICT the spec is not publically available for
> > free.

> > So at the very least we need to document each one's meaning
> > in libvirt docs IMHO.
> >
> > If I had insight into what they meant, then I might also
> > suggest giving them meaningful names in libvirt. These two
> > level codes are presumably chosen for reasons of space
> > efficiency at the low level. This is not a constraint
> > we especially care about in libvirt, where ease of
> > understanding is usually more important.
>
> Ok, I found a copy of the PCI spec

Ack, I could document those fields in a doc update for users to
understand what to expect.

Just a note: those specs are available if an e-mail of a PCI SIG
member company is used during registration at the PCI SIG website so I
think it should be available to you too.

>
> I see the data is classified at 2 levels, with the first
> level being:
>
> String Tag: This tag is the first item in the VPD storage
>             component.  It contains the name of the add-in card
>             in alphanumeric characters.
>
> VPD-R Tag: This tag contains the read only VPD keywords for an
>            add-in card.
>
> VPD-W Tag: This tag contains the read/write VPD keywords for
>            an add-in card
>
>
> Then for VPD-R, the next level
>
>    PN: Add-in Card Part Number
>    EC: Engineering change level of the card
>    FG: Fabric Geography
>    LC: Location
>    MN: Manufacture ID
>    PG: PCI Geography
>    SN: Serial number
>    Vx: Vendor string (Repeated many times for 'x' in range 0-9, A-Z)
>    CP: Extended capability
>    RV: Checksum + reserved space
>
> I don't see a need to report the "CP" and "RV" tags in the XML.

Agreed, "CP" is currently ignored and "RV" is used for checksum
validation only in the current code.

There is also the "RW" keyword which is used to identify an unused
portion of the read-write space: AFAIU the read-write VPD memory will
have some free space for future writes and the "RW" keyword value
probably needs to be updated by the software making writes to this
area. I have not actually seen any devices with a pre-programmed VPD-W
yet but so far the focus has been on reading what's in there properly.

There is only one string resource specified in the spec at the moment
(the device name). I suppose we could assume that there will only be
one and deal with possible VPD format changes as they arise.

Likewise, for VPD-R and VPD-W: there is currently no use-case for
having multiple instances for the same type - so maybe I was too
forward-looking in this case (i.e. the format hasn't seen much change
in years).

>
>
> Then for VPD-W,the next level
>
>    Vx: Vendor string (Repeated many times for 'x' in range 0-9, A-Z)
>    Sx: System string (Repeated many times for 'x' in range 0-9, B-Z)
>    YA: Asset tag
>
> With all this in mind, I think we a better to represent
> these all as meaingfully named elements
>
>   <capability type="vpd">
>     <name>BlueField-2 DPU 25GbE Dual-Port SFP56, Crypto Enabled, 16GB on-board DDR, 1GbE OOB management, Tall Bracket</name>
>     <tags access="readonly">
>        <change_level>B1</change_level>
>        <part_number>MBF2H332A-AEEOT</part_number>
>        <serial_number>MT2113X00000</serial_number>
>        <vendor_string index="0">PCIeGen4 x8</vendor_string>
>        <vendor_string index="2">MBF2H332A-AEEOT</vendor_string>
>        <vendor_string index="3">3c53d07eec484d8aab34dabd24fe575aa</vendor_string>
>     </tags>
>     <tags access="readwrite">
>        ...
>     </tags>
>   </capability>
>
>

Ack, this certainly has benefits for human readability but maybe
introduces slightly more work to map human-readable names to spec
keywords at the client side (not a big deal for the purposes I plan to
use it though).

Going to adjust what I have to match the above.