Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Apr 04, 2023 at 09:18:53PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, April 4, 2023 3:35 AM
> 
> > > +Virtio extended PCI Express capability structure defines the location
> > > +of certain virtio device configuration related structures using PCI
> > > +Express extended capability. Virtio extended PCI Express capability
> > > +structure uses PCI Express vendor specific extended capability
> > > +(VSEC). It has a below
> > 
> > a layout below, or the following layout
> > 
> Yes. somehow it got trimmed.
> Will fix it.
> 
> > > +layout:
> > > +
> > > +\begin{lstlisting}
> > > +struct pcie_ext_cap {
> > > +        le16 cap_vendor_id; /* Generic PCI field: 0xB */
> > > +        le16 cap_version : 2; /* Generic PCI field: 0 */
> > > +        le16 next_cap_offset : 14; /* Generic PCI field: next cap or
> > > +0 */ };
> > > +
> > > +struct virtio_pcie_ext_cap {
> > > +        struct pcie_ext_cap pcie_ecap;
> > > +        u8 cfg_type; /* Identifies the structure. */
> > > +        u8 bar; /* Index of the BAR where its located */
> > > +        u8 id; /* Multiple capabilities of the same type */
> > > +        u8 zero_padding[1];
> > > +        le64 offset; /* Offset with the bar */
> > > +        le64 length; /* Length of the structure, in bytes. */
> > > +        u8 data[]; /* Optional variable length data */
> > 
> > Maybe le64 data[], for alignment?
> > 
> It gets harder to decode (typecasting ..) if its string with le64 data type.

In what language? In C you have to cast anyway, string is char *, often
signed, not u8.

> I will extend the comment, 
> 
> +        u8 data[]; /* Optional variable length data, must be aligned to 8 bytes */

I'd keep it le64 or u64, it is highly unlikely we'll pass strings through
this interface anyway.

-- 
MST