On Wed, Apr 05, 2023 at 01:16:31PM +0000, Parav Pandit wrote: > > > > From: Michael S. Tsirkin <[email protected]> > > Sent: Wednesday, April 5, 2023 1:11 AM > > > > > > +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. > > Ok. will change. > > What about rest of the patches? If we proceed using MMR interface, rest of > the patches are fine?
Biggest problem is 7/11 - the new IDs, breaking all existing drivers. I thought I replied on that but don't see it on that specific patch. Let me repost my thoughts now I had time to think it over. -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
