Re: [PATCH V3 4/4] intel-iommu: PASID support

[Peter Xu <peterx@redhat.com>]

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