On Mon, Oct 17, 2022 at 03:52:46PM +0800, Jason Wang wrote:
[...]
> > > +struct vtd_iotlb_key {
> > > + uint16_t sid;
> > > + uint32_t pasid;
> > > + uint64_t gfn;
> > > + uint32_t level;
> > > };
> >
> > Nit: maybe re-arrange it a bit?
> >
> > struct vtd_iotlb_key {
> > uint64_t gfn;
> > uint32_t pasid;
> > uint32_t level;
> > uint16_t sid;
> > } __attribute__((__packed__));
> >
> > "packed" should save us 6 bytes for each in this case, maybe also
> > worthwhile but not strongly as we have a limit of 1k objs.
>
> I think it should be fine to rearrange but for 'packed', would this
> cause alignment issues that may cause troubles on some arches?
Do you mean the gfn reading can be split into 2 * 4 bytes? Would that
still work as long as we're protected with a lock when accessing iotlb
(even though it may be slower than aligned access)?
But at least I think you're right it's not always a benefit, so no strong
opinion here to have it packed.
>
> >
> > The name "gfn" seems a bit unfortunate - would "iova" be more suitable? I
> > do see we used it elsewhere too, so we can also leave that for later.
>
> Right, it has been used for VTDIOTLBEntry before this patch. If
> possible I would leave it to be done on top with a separate patch.
Definitely.
Thanks,
--
Peter Xu
