On 3/19/2020 9:52 PM, Alex Williamson wrote:
On Thu, 19 Mar 2020 20:22:41 +0530
Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote:
On 3/19/2020 9:15 AM, Alex Williamson wrote:
On Thu, 19 Mar 2020 01:11:11 +0530
Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote:
<snip>
+
+static int verify_bitmap_size(uint64_t npages, uint64_t bitmap_size)
+{
+ uint64_t bsize;
+
+ if (!npages || !bitmap_size || bitmap_size > UINT_MAX)
As commented previously, how do we derive this UINT_MAX limitation?
Sorry, I missed that earlier
> UINT_MAX seems arbitrary, is this specified in our API? The size of a
> vfio_dma is limited to what the user is able to pin, and therefore
> their locked memory limit, but do we have an explicit limit elsewhere
> that results in this limit here. I think a 4GB bitmap would track
> something like 2^47 bytes of memory, that's pretty excessive, but still
> an arbitrary limit.
There has to be some upper limit check. In core KVM, in
virt/kvm/kvm_main.c there is max number of pages check:
if (new.npages > KVM_MEM_MAX_NR_PAGES)
Where
/*
* Some of the bitops functions do not support too long bitmaps.
* This number must be determined not to exceed such limits.
*/
#define KVM_MEM_MAX_NR_PAGES ((1UL << 31) - 1)
Though I don't know which bitops functions do not support long bitmaps.
Something similar as above can be done or same as you also mentioned of
4GB bitmap limit? that is U32_MAX instead of UINT_MAX?
Let's see, we use bitmap_set():
void bitmap_set(unsigned long *map, unsigned int start, unsigned int nbits)
So we're limited to an unsigned int number of bits, but for an
unaligned, multi-bit operation this will call __bitmap_set():
void __bitmap_set(unsigned long *map, unsigned int start, int len)
So we're down to a signed int number of bits (seems like an API bug in
bitops there), so it makes sense that KVM is testing against MAX_INT
number of pages, ie. number of bits. But that still suggests a bitmap
size of MAX_UINT is off by a factor of 16. So we can have 2^31 bits
divided by 2^3 bits/byte yields a maximum bitmap size of 2^28 (ie.
256MB), which maps 2^31 * 2^12 = 2^43 (8TB) on a 4K system.
Let's fix the limit check and put a nice comment explaining it. Thanks,
Agreed. Adding DIRTY_BITMAP_SIZE_MAX macro and comment as below.
/*
* Input argument of number of bits to bitmap_set() is unsigned
integer, which
* further casts to signed integer for unaligned multi-bit operation,
* __bitmap_set().
* Then maximum bitmap size supported is 2^31 bits divided by 2^3
bits/byte,
* that is 2^28 (256 MB) which maps to 2^31 * 2^12 = 2^43 (8TB) on 4K page
* system.
*/
#define DIRTY_BITMAP_PAGES_MAX ((1UL << 31) - 1)
#define DIRTY_BITMAP_SIZE_MAX \
DIRTY_BITMAP_BYTES(DIRTY_BITMAP_PAGES_MAX)
Thanks,
Kirti
