Messages in this thread | | | From | "Winkler, Tomas" <> | Subject | RE: [char-misc-next 2/2] mei: revamp mei extension header structure layout. | Date | Sat, 19 Jun 2021 20:42:04 +0000 |
| |
> > On Wed, Jun 16, 2021 at 12:15:57AM +0300, Tomas Winkler wrote: > > The mei extension header was build as array of flexible structures > > which will not work if actually more headers are added > > Why not? What is wrong with what you currently have? Because it is not possible to create array of flexible structures in C as far as I know.
> And did you forget a '.' here? Thanks will resend.
> > > Use basic type u8 for the variable sized extension. > > Define explicitly mei_ext_hdr_vtag structure. > > Fix also mei_ext_next() function to point correctly to the end of the > > header. > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> > > --- > > drivers/misc/mei/client.c | 16 +++++++++------- > > drivers/misc/mei/hw.h | 28 ++++++++++++++++++++-------- > > drivers/misc/mei/interrupt.c | 23 ++++++++++------------- > > 3 files changed, 39 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c > > index 18e49479d8b0..96f4e59c32a5 100644 > > --- a/drivers/misc/mei/client.c > > +++ b/drivers/misc/mei/client.c > > @@ -1726,12 +1726,15 @@ int mei_cl_read_start(struct mei_cl *cl, size_t > length, const struct file *fp) > > return rets; > > } > > > > -static inline u8 mei_ext_hdr_set_vtag(struct mei_ext_hdr *ext, u8 > > vtag) > > +static inline u8 mei_ext_hdr_set_vtag(void *ext, u8 vtag) > > { > > - ext->type = MEI_EXT_HDR_VTAG; > > - ext->ext_payload[0] = vtag; > > - ext->length = mei_data2slots(sizeof(*ext)); > > - return ext->length; > > + struct mei_ext_hdr_vtag *vtag_hdr = ext; > > + > > + vtag_hdr->hdr.type = MEI_EXT_HDR_VTAG; > > + vtag_hdr->hdr.length = mei_data2slots(sizeof(*vtag_hdr)); > > + vtag_hdr->vtag = vtag; > > + vtag_hdr->reserved = 0; > > + return vtag_hdr->hdr.length; > > } > > > > /** > > @@ -1745,7 +1748,6 @@ static struct mei_msg_hdr > > *mei_msg_hdr_init(const struct mei_cl_cb *cb) { > > size_t hdr_len; > > struct mei_ext_meta_hdr *meta; > > - struct mei_ext_hdr *ext; > > struct mei_msg_hdr *mei_hdr; > > bool is_ext, is_vtag; > > > > @@ -1764,7 +1766,7 @@ static struct mei_msg_hdr > > *mei_msg_hdr_init(const struct mei_cl_cb *cb) > > > > hdr_len += sizeof(*meta); > > if (is_vtag) > > - hdr_len += sizeof(*ext); > > + hdr_len += sizeof(struct mei_ext_hdr_vtag); > > > > setup_hdr: > > mei_hdr = kzalloc(hdr_len, GFP_KERNEL); diff --git > > a/drivers/misc/mei/hw.h b/drivers/misc/mei/hw.h index > > b10606550613..dfd60c916da0 100644 > > --- a/drivers/misc/mei/hw.h > > +++ b/drivers/misc/mei/hw.h > > @@ -235,9 +235,8 @@ enum mei_ext_hdr_type { struct mei_ext_hdr { > > u8 type; > > u8 length; > > - u8 ext_payload[2]; > > - u8 hdr[]; > > -}; > > + u8 data[]; > > +} __packed; > > why packed? It's an aligned structure but still It's HW interface. > > > > > /** > > * struct mei_ext_meta_hdr - extend header meta data @@ -250,8 > > +249,21 @@ struct mei_ext_meta_hdr { > > u8 count; > > u8 size; > > u8 reserved[2]; > > - struct mei_ext_hdr hdrs[]; > > -}; > > + u8 hdrs[]; > > +} __packed; > > Why packed? Same here.
> > > + > > +/** > > + * struct mei_ext_hdr_vtag - extend header for vtag > > + * > > + * @hdr: standard extend header > > + * @vtag: virtual tag > > + * @reserved: reserved > > + */ > > +struct mei_ext_hdr_vtag { > > + struct mei_ext_hdr hdr; > > + u8 vtag; > > + u8 reserved; > > +} __packed; > > Why packed? > > These are not being read directly from hardware are they? They are.
Thanks Tomas
| |