On Tue, Apr 29, 2025 at 01:39:09PM -0700, Nicolin Chen wrote:
> On Tue, Apr 29, 2025 at 08:34:56PM +0000, Pranjal Shrivastava wrote:
> > On Tue, Apr 29, 2025 at 08:24:33PM +0000, Pranjal Shrivastava wrote:
> > > On Fri, Apr 25, 2025 at 10:58:08PM -0700, Nicolin Chen wrote:
> > > > +       struct iommufd_mmap *immap;
> > > > +       int rc;
> > > > +
> > > > +       if (WARN_ON_ONCE(!immap_id))
> > > > +               return -EINVAL;
> > > > +       if (base & ~PAGE_MASK)
> > > > +               return -EINVAL;
> > > > +       if (!size || size & ~PAGE_MASK)
> > > > +               return -EINVAL;
> > > > +
> > > > +       immap = kzalloc(sizeof(*immap), GFP_KERNEL);
> > > > +       if (!immap)
> > > > +               return -ENOMEM;
> > > > +       immap->pfn_start = base >> PAGE_SHIFT;
> > > > +       immap->pfn_end = immap->pfn_start + (size >> PAGE_SHIFT) - 1;
> > > > +
> > > > +       rc = mtree_alloc_range(&ictx->mt_mmap, immap_id, immap, 
> > > > sizeof(immap),
> > > 
> > > I believe this should be sizeof(*immap) ?
> > 
> > Ugh, Sorry, shouldn't this be size >> PAGE_SHIFT (num_indices to alloc) ?
> 
> mtree_load() returns a "struct iommufd_map *" pointer.

I'm not talking about mtree_load. I meant mtree_alloc_range takes in a
"size" parameter, which is being passed as sizeof(imap) in this patch.
IIUC, the mtree_alloc_range, via mas_empty_area, gets a range that is
sufficient for the given "size". 

Now in this case, "size" would be the no. of pfns which are mmap-able.
By passing sizeof(immap), we're simply reserving sizeof(ptr) i.e. 8 pfns
for a 64-bit machine. Whereas we really, just want to reserve a range
for size >> PAGE_SHIFT pfns.

> 
> Nicolin

Thanks,
Praan

Reply via email to