| Message ID | 20210924155705.4258-14-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/15] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev() | expand |
On Fri, Sep 24, 2021 at 05:57:03PM +0200, Christoph Hellwig wrote: > Ensure pgsize_bitmap is always valid by initializing it to ULONG_MAX > in vfio_iommu_type1_open and remove the now pointless update for > the external domain case in vfio_iommu_type1_attach_group, which was > just setting pgsize_bitmap to ULONG_MAX when only external domains > were attached. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > drivers/vfio/vfio_iommu_type1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index a48e9f597cb213..2c698e1a29a1d8 100644 > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -2196,7 +2196,6 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > if (!iommu->external_domain) { > INIT_LIST_HEAD(&domain->group_list); > iommu->external_domain = domain; > - vfio_update_pgsize_bitmap(iommu); > } else { > kfree(domain); > } > @@ -2582,6 +2581,7 @@ static void *vfio_iommu_type1_open(unsigned long arg) > mutex_init(&iommu->lock); > BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier); > init_waitqueue_head(&iommu->vaddr_wait); > + iommu->pgsize_bitmap = ULONG_MAX; I wonder if this needs the PAGE_MASK/SIZE stuff? iommu->pgsize_bitmap = ULONG_MASK & PAGE_MASK; ? vfio_update_pgsize_bitmap() goes to some trouble to avoid setting bits below the CPU page size here Jason
On Fri, 24 Sep 2021 14:48:52 -0300 Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Fri, Sep 24, 2021 at 05:57:03PM +0200, Christoph Hellwig wrote: > > Ensure pgsize_bitmap is always valid by initializing it to ULONG_MAX > > in vfio_iommu_type1_open and remove the now pointless update for > > the external domain case in vfio_iommu_type1_attach_group, which was > > just setting pgsize_bitmap to ULONG_MAX when only external domains > > were attached. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > drivers/vfio/vfio_iommu_type1.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > > index a48e9f597cb213..2c698e1a29a1d8 100644 > > +++ b/drivers/vfio/vfio_iommu_type1.c > > @@ -2196,7 +2196,6 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > > if (!iommu->external_domain) { > > INIT_LIST_HEAD(&domain->group_list); > > iommu->external_domain = domain; > > - vfio_update_pgsize_bitmap(iommu); > > } else { > > kfree(domain); > > } > > @@ -2582,6 +2581,7 @@ static void *vfio_iommu_type1_open(unsigned long arg) > > mutex_init(&iommu->lock); > > BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier); > > init_waitqueue_head(&iommu->vaddr_wait); > > + iommu->pgsize_bitmap = ULONG_MAX; > > I wonder if this needs the PAGE_MASK/SIZE stuff? > > iommu->pgsize_bitmap = ULONG_MASK & PAGE_MASK; > > ? > > vfio_update_pgsize_bitmap() goes to some trouble to avoid setting bits > below the CPU page size here Yep, though PAGE_MASK should already be UL, so just PAGE_MASK itself should work. The ULONG_MAX in the update function just allows us to detect sub-page, ex. if the IOMMU supports 2K we can expose 4K minimum, but we can't if the min IOMMU page is 64K. Thanks, Alex
On Fri, Sep 24, 2021 at 12:37:55PM -0600, Alex Williamson wrote: > > > + iommu->pgsize_bitmap = ULONG_MAX; > > > > I wonder if this needs the PAGE_MASK/SIZE stuff? > > > > iommu->pgsize_bitmap = ULONG_MASK & PAGE_MASK; > > > > ? > > > > vfio_update_pgsize_bitmap() goes to some trouble to avoid setting bits > > below the CPU page size here > > Yep, though PAGE_MASK should already be UL, so just PAGE_MASK itself > should work. The ULONG_MAX in the update function just allows us to > detect sub-page, ex. if the IOMMU supports 2K we can expose 4K minimum, > but we can't if the min IOMMU page is 64K. Thanks, Do you just want to update this or do you want a full resend of the series?
On Mon, 27 Sep 2021 13:49:28 +0200 Christoph Hellwig <hch@lst.de> wrote: > On Fri, Sep 24, 2021 at 12:37:55PM -0600, Alex Williamson wrote: > > > > + iommu->pgsize_bitmap = ULONG_MAX; > > > > > > I wonder if this needs the PAGE_MASK/SIZE stuff? > > > > > > iommu->pgsize_bitmap = ULONG_MASK & PAGE_MASK; > > > > > > ? > > > > > > vfio_update_pgsize_bitmap() goes to some trouble to avoid setting bits > > > below the CPU page size here > > > > Yep, though PAGE_MASK should already be UL, so just PAGE_MASK itself > > should work. The ULONG_MAX in the update function just allows us to > > detect sub-page, ex. if the IOMMU supports 2K we can expose 4K minimum, > > but we can't if the min IOMMU page is 64K. Thanks, > > Do you just want to update this or do you want a full resend of the > series? So long as we have agreement, I can do it inline. Are we doing s/ULONG_MAX/PAGE_MASK/ across both the diff and commit log? Thanks, Alex
On Mon, Sep 27, 2021 at 06:56:37AM -0600, Alex Williamson wrote: > > > Yep, though PAGE_MASK should already be UL, so just PAGE_MASK itself > > > should work. The ULONG_MAX in the update function just allows us to > > > detect sub-page, ex. if the IOMMU supports 2K we can expose 4K minimum, > > > but we can't if the min IOMMU page is 64K. Thanks, > > > > Do you just want to update this or do you want a full resend of the > > series? > > So long as we have agreement, I can do it inline. Are we doing > s/ULONG_MAX/PAGE_MASK/ across both the diff and commit log? Thanks, Sounds good to me.
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index a48e9f597cb213..2c698e1a29a1d8 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -2196,7 +2196,6 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, if (!iommu->external_domain) { INIT_LIST_HEAD(&domain->group_list); iommu->external_domain = domain; - vfio_update_pgsize_bitmap(iommu); } else { kfree(domain); } @@ -2582,6 +2581,7 @@ static void *vfio_iommu_type1_open(unsigned long arg) mutex_init(&iommu->lock); BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier); init_waitqueue_head(&iommu->vaddr_wait); + iommu->pgsize_bitmap = ULONG_MAX; return iommu; }
Ensure pgsize_bitmap is always valid by initializing it to ULONG_MAX in vfio_iommu_type1_open and remove the now pointless update for the external domain case in vfio_iommu_type1_attach_group, which was just setting pgsize_bitmap to ULONG_MAX when only external domains were attached. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/vfio/vfio_iommu_type1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)