RE: [PATCH v2 06/11] iommufd: IOMMU_HWPT_ALLOC allocation with user data

[Tian, Kevin]

> From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> Sent: Wednesday, May 24, 2023 1:41 PM
> 
> On Wed, May 24, 2023 at 05:16:35AM +0000, Tian, Kevin wrote:
> 
> > > From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > > Sent: Saturday, May 20, 2023 3:48 AM
> > >
> > >
> > > > >
> > > > > +     switch (pt_obj->type) {
> > > > > +     case IOMMUFD_OBJ_IOAS:
> > > > > +             ioas = container_of(pt_obj, struct iommufd_ioas, obj);
> > > > > +             break;
> > > >
> > > > this should fail if parent is specified.
> > >
> > > I don't think that's necessaray: the parent is NULL by default
> > > and only specified (if IOMMUFD_OBJ_HW_PAGETABLE) by the exact
> > > pt_id/pt_obj here.
> >
> > I didn't get. The uAPI describes that only hwpt not ioas can be specified
> > in the pt_id field as the parent.
> >
> > If we don't check here it means the user can specify an ioas id as the
> > parent?
> 
> I meant that the parent pointer isn't specified at this line:
> the declaration of the parent is simply NULL, and not touched
> in this IOMMUFD_OBJ_IOAS case, as the parent pointer would be
> only specified in the IOMMUFD_OBJ_HW_PAGETABLE case that is
> behind this line.
> 

I see your point. As long as the type is IOAS the alloc request is
always interpreted as creating a s2 hwpt under the IOAS. Only
when it's HWPT type then it's treated as the parent.

I kept a wrong impression that there is another flag/field to mark
the parent requirement then there could be wrong combination
of setting that flag/field plus using an IOAS pt_id. 😊