* [PATCH v2] vfio/iova_bitmap: refactor iova_bitmap_set() to better handle page boundaries
@ 2022-11-29 13:12 Joao Martins
2022-12-01 10:49 ` Avihai Horon
2022-12-02 23:31 ` Alex Williamson
0 siblings, 2 replies; 3+ messages in thread
From: Joao Martins @ 2022-11-29 13:12 UTC (permalink / raw)
To: kvm
Cc: Alex Williamson, Cornelia Huck, Avihai Horon, Jason Gunthorpe,
Joao Martins
Commit f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
had fixed the unaligned bitmaps by capping the remaining iterable set at
the start of the bitmap. Although, that mistakenly worked around
iova_bitmap_set() incorrectly setting bits across page boundary.
Fix this by reworking the loop inside iova_bitmap_set() to iterate over a
range of bits to set (cur_bit .. last_bit) which may span different pinned
pages, thus updating @page_idx and @offset as it sets the bits. The
previous cap to the first page is now adjusted to be always accounted
rather than when there's only a non-zero pgoff.
While at it, make @page_idx , @offset and @nbits to be unsigned int given
that it won't be more than 512 and 4096 respectively (even a bigger
PAGE_SIZE or a smaller struct page size won't make this bigger than the
above 32-bit max). Also, delete the stale kdoc on Return type.
Cc: Avihai Horon <avihaih@nvidia.com>
Fixes: f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
Co-developed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
Changes since v1:
* Add Reviewed-by by Jason Gunthorpe
* Add Fixes tag (Alex Williamson)
It passes my tests but to be extra sure: Avihai could you take this
patch a spin in your rig/tests as well? Thanks!
---
drivers/vfio/iova_bitmap.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/vfio/iova_bitmap.c b/drivers/vfio/iova_bitmap.c
index de6d6ea5c496..0848f920efb7 100644
--- a/drivers/vfio/iova_bitmap.c
+++ b/drivers/vfio/iova_bitmap.c
@@ -298,9 +298,7 @@ static unsigned long iova_bitmap_mapped_remaining(struct iova_bitmap *bitmap)
{
unsigned long remaining, bytes;
- /* Cap to one page in the first iteration, if PAGE_SIZE unaligned. */
- bytes = !bitmap->mapped.pgoff ? bitmap->mapped.npages << PAGE_SHIFT :
- PAGE_SIZE - bitmap->mapped.pgoff;
+ bytes = (bitmap->mapped.npages << PAGE_SHIFT) - bitmap->mapped.pgoff;
remaining = bitmap->mapped_total_index - bitmap->mapped_base_index;
remaining = min_t(unsigned long, remaining,
@@ -399,29 +397,27 @@ int iova_bitmap_for_each(struct iova_bitmap *bitmap, void *opaque,
* Set the bits corresponding to the range [iova .. iova+length-1] in
* the user bitmap.
*
- * Return: The number of bits set.
*/
void iova_bitmap_set(struct iova_bitmap *bitmap,
unsigned long iova, size_t length)
{
struct iova_bitmap_map *mapped = &bitmap->mapped;
- unsigned long offset = (iova - mapped->iova) >> mapped->pgshift;
- unsigned long nbits = max_t(unsigned long, 1, length >> mapped->pgshift);
- unsigned long page_idx = offset / BITS_PER_PAGE;
- unsigned long page_offset = mapped->pgoff;
- void *kaddr;
-
- offset = offset % BITS_PER_PAGE;
+ unsigned long cur_bit = ((iova - mapped->iova) >>
+ mapped->pgshift) + mapped->pgoff * BITS_PER_BYTE;
+ unsigned long last_bit = (((iova + length - 1) - mapped->iova) >>
+ mapped->pgshift) + mapped->pgoff * BITS_PER_BYTE;
do {
- unsigned long size = min(BITS_PER_PAGE - offset, nbits);
+ unsigned int page_idx = cur_bit / BITS_PER_PAGE;
+ unsigned int offset = cur_bit % BITS_PER_PAGE;
+ unsigned int nbits = min(BITS_PER_PAGE - offset,
+ last_bit - cur_bit + 1);
+ void *kaddr;
kaddr = kmap_local_page(mapped->pages[page_idx]);
- bitmap_set(kaddr + page_offset, offset, size);
+ bitmap_set(kaddr, offset, nbits);
kunmap_local(kaddr);
- page_offset = offset = 0;
- nbits -= size;
- page_idx++;
- } while (nbits > 0);
+ cur_bit += nbits;
+ } while (cur_bit <= last_bit);
}
EXPORT_SYMBOL_GPL(iova_bitmap_set);
--
2.17.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] vfio/iova_bitmap: refactor iova_bitmap_set() to better handle page boundaries
2022-11-29 13:12 [PATCH v2] vfio/iova_bitmap: refactor iova_bitmap_set() to better handle page boundaries Joao Martins
@ 2022-12-01 10:49 ` Avihai Horon
2022-12-02 23:31 ` Alex Williamson
1 sibling, 0 replies; 3+ messages in thread
From: Avihai Horon @ 2022-12-01 10:49 UTC (permalink / raw)
To: Joao Martins, kvm; +Cc: Alex Williamson, Cornelia Huck, Jason Gunthorpe
Hi,
On 29/11/2022 15:12, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> Commit f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
> had fixed the unaligned bitmaps by capping the remaining iterable set at
> the start of the bitmap. Although, that mistakenly worked around
> iova_bitmap_set() incorrectly setting bits across page boundary.
>
> Fix this by reworking the loop inside iova_bitmap_set() to iterate over a
> range of bits to set (cur_bit .. last_bit) which may span different pinned
> pages, thus updating @page_idx and @offset as it sets the bits. The
> previous cap to the first page is now adjusted to be always accounted
> rather than when there's only a non-zero pgoff.
>
> While at it, make @page_idx , @offset and @nbits to be unsigned int given
> that it won't be more than 512 and 4096 respectively (even a bigger
> PAGE_SIZE or a smaller struct page size won't make this bigger than the
> above 32-bit max). Also, delete the stale kdoc on Return type.
>
> Cc: Avihai Horon <avihaih@nvidia.com>
> Fixes: f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
> Co-developed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> Changes since v1:
> * Add Reviewed-by by Jason Gunthorpe
> * Add Fixes tag (Alex Williamson)
>
> It passes my tests but to be extra sure: Avihai could you take this
> patch a spin in your rig/tests as well? Thanks!
Looks good on my side:
Tested-by: Avihai Horon <avihaih@nvidia.com>
> ---
> drivers/vfio/iova_bitmap.c | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/vfio/iova_bitmap.c b/drivers/vfio/iova_bitmap.c
> index de6d6ea5c496..0848f920efb7 100644
> --- a/drivers/vfio/iova_bitmap.c
> +++ b/drivers/vfio/iova_bitmap.c
> @@ -298,9 +298,7 @@ static unsigned long iova_bitmap_mapped_remaining(struct iova_bitmap *bitmap)
> {
> unsigned long remaining, bytes;
>
> - /* Cap to one page in the first iteration, if PAGE_SIZE unaligned. */
> - bytes = !bitmap->mapped.pgoff ? bitmap->mapped.npages << PAGE_SHIFT :
> - PAGE_SIZE - bitmap->mapped.pgoff;
> + bytes = (bitmap->mapped.npages << PAGE_SHIFT) - bitmap->mapped.pgoff;
>
> remaining = bitmap->mapped_total_index - bitmap->mapped_base_index;
> remaining = min_t(unsigned long, remaining,
> @@ -399,29 +397,27 @@ int iova_bitmap_for_each(struct iova_bitmap *bitmap, void *opaque,
> * Set the bits corresponding to the range [iova .. iova+length-1] in
> * the user bitmap.
> *
> - * Return: The number of bits set.
> */
> void iova_bitmap_set(struct iova_bitmap *bitmap,
> unsigned long iova, size_t length)
> {
> struct iova_bitmap_map *mapped = &bitmap->mapped;
> - unsigned long offset = (iova - mapped->iova) >> mapped->pgshift;
> - unsigned long nbits = max_t(unsigned long, 1, length >> mapped->pgshift);
> - unsigned long page_idx = offset / BITS_PER_PAGE;
> - unsigned long page_offset = mapped->pgoff;
> - void *kaddr;
> -
> - offset = offset % BITS_PER_PAGE;
> + unsigned long cur_bit = ((iova - mapped->iova) >>
> + mapped->pgshift) + mapped->pgoff * BITS_PER_BYTE;
> + unsigned long last_bit = (((iova + length - 1) - mapped->iova) >>
> + mapped->pgshift) + mapped->pgoff * BITS_PER_BYTE;
>
> do {
> - unsigned long size = min(BITS_PER_PAGE - offset, nbits);
> + unsigned int page_idx = cur_bit / BITS_PER_PAGE;
> + unsigned int offset = cur_bit % BITS_PER_PAGE;
> + unsigned int nbits = min(BITS_PER_PAGE - offset,
> + last_bit - cur_bit + 1);
> + void *kaddr;
>
> kaddr = kmap_local_page(mapped->pages[page_idx]);
> - bitmap_set(kaddr + page_offset, offset, size);
> + bitmap_set(kaddr, offset, nbits);
> kunmap_local(kaddr);
> - page_offset = offset = 0;
> - nbits -= size;
> - page_idx++;
> - } while (nbits > 0);
> + cur_bit += nbits;
> + } while (cur_bit <= last_bit);
> }
> EXPORT_SYMBOL_GPL(iova_bitmap_set);
> --
> 2.17.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] vfio/iova_bitmap: refactor iova_bitmap_set() to better handle page boundaries
2022-11-29 13:12 [PATCH v2] vfio/iova_bitmap: refactor iova_bitmap_set() to better handle page boundaries Joao Martins
2022-12-01 10:49 ` Avihai Horon
@ 2022-12-02 23:31 ` Alex Williamson
1 sibling, 0 replies; 3+ messages in thread
From: Alex Williamson @ 2022-12-02 23:31 UTC (permalink / raw)
To: Joao Martins; +Cc: kvm, Cornelia Huck, Avihai Horon, Jason Gunthorpe
On Tue, 29 Nov 2022 13:12:35 +0000
Joao Martins <joao.m.martins@oracle.com> wrote:
> Commit f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
> had fixed the unaligned bitmaps by capping the remaining iterable set at
> the start of the bitmap. Although, that mistakenly worked around
> iova_bitmap_set() incorrectly setting bits across page boundary.
>
> Fix this by reworking the loop inside iova_bitmap_set() to iterate over a
> range of bits to set (cur_bit .. last_bit) which may span different pinned
> pages, thus updating @page_idx and @offset as it sets the bits. The
> previous cap to the first page is now adjusted to be always accounted
> rather than when there's only a non-zero pgoff.
>
> While at it, make @page_idx , @offset and @nbits to be unsigned int given
> that it won't be more than 512 and 4096 respectively (even a bigger
> PAGE_SIZE or a smaller struct page size won't make this bigger than the
> above 32-bit max). Also, delete the stale kdoc on Return type.
>
> Cc: Avihai Horon <avihaih@nvidia.com>
> Fixes: f38044e5ef58 ("vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps")
> Co-developed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> Changes since v1:
> * Add Reviewed-by by Jason Gunthorpe
> * Add Fixes tag (Alex Williamson)
>
> It passes my tests but to be extra sure: Avihai could you take this
> patch a spin in your rig/tests as well? Thanks!
> ---
> drivers/vfio/iova_bitmap.c | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
Applied to vfio next branch for v6.2 with Avihai's tested-by. Thanks,
Alex
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-02 23:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29 13:12 [PATCH v2] vfio/iova_bitmap: refactor iova_bitmap_set() to better handle page boundaries Joao Martins
2022-12-01 10:49 ` Avihai Horon
2022-12-02 23:31 ` Alex Williamson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.