From: Alex Williamson <alex.williamson@redhat.com>
To: Kirti Wankhede <kwankhede@nvidia.com>
Cc: <cjia@nvidia.com>, <kevin.tian@intel.com>, <ziye.yang@intel.com>,
<changpeng.liu@intel.com>, <yi.l.liu@intel.com>,
<mlevitsk@redhat.com>, <eskultet@redhat.com>, <cohuck@redhat.com>,
<dgilbert@redhat.com>, <jonathan.davies@nutanix.com>,
<eauger@redhat.com>, <aik@ozlabs.ru>, <pasic@linux.ibm.com>,
<felipe@nutanix.com>, <Zhengxiao.zx@Alibaba-inc.com>,
<shuangtai.tst@alibaba-inc.com>, <Ken.Xue@amd.com>,
<zhi.a.wang@intel.com>, <yan.y.zhao@intel.com>,
<qemu-devel@nongnu.org>, <kvm@vger.kernel.org>
Subject: Re: [PATCH v14 Kernel 4/7] vfio iommu: Implementation of ioctl for dirty pages tracking.
Date: Thu, 19 Mar 2020 14:54:21 -0600 [thread overview]
Message-ID: <20200319145421.4b8bd4eb@w520.home> (raw)
In-Reply-To: <8e537411-b60e-cc45-498c-5e516382206e@nvidia.com>
On Fri, 20 Mar 2020 01:55:10 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:
> On 3/19/2020 9:52 PM, Alex Williamson wrote:
> > On Thu, 19 Mar 2020 20:22:41 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >
> >> On 3/19/2020 9:15 AM, Alex Williamson wrote:
> >>> On Thu, 19 Mar 2020 01:11:11 +0530
> >>> Kirti Wankhede <kwankhede@nvidia.com> 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)
nit, can we just use INT_MAX here?
> #define DIRTY_BITMAP_SIZE_MAX \
> DIRTY_BITMAP_BYTES(DIRTY_BITMAP_PAGES_MAX)
>
>
> Thanks,
> Kirti
>
next prev parent reply other threads:[~2020-03-19 20:54 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-18 19:41 [PATCH v14 Kernel 0/7] KABIs to support migration for VFIO devices Kirti Wankhede
2020-03-18 19:41 ` [PATCH v14 Kernel 1/7] vfio: KABI for migration interface for device state Kirti Wankhede
2020-03-19 1:17 ` Yan Zhao
2020-03-19 3:49 ` Alex Williamson
2020-03-19 5:05 ` Yan Zhao
2020-03-19 13:09 ` Alex Williamson
2020-03-20 1:30 ` Yan Zhao
2020-03-20 2:34 ` Alex Williamson
2020-03-20 3:06 ` Yan Zhao
2020-03-20 4:09 ` Alex Williamson
2020-03-20 4:20 ` Yan Zhao
2020-03-23 14:45 ` Auger Eric
2020-03-23 11:45 ` Auger Eric
2020-03-24 19:14 ` Kirti Wankhede
2020-03-18 19:41 ` [PATCH v14 Kernel 2/7] vfio iommu: Remove atomicity of ref_count of pinned pages Kirti Wankhede
2020-03-23 11:59 ` Auger Eric
2020-03-18 19:41 ` [PATCH v14 Kernel 3/7] vfio iommu: Add ioctl definition for dirty pages tracking Kirti Wankhede
2020-03-19 3:44 ` Alex Williamson
2020-03-18 19:41 ` [PATCH v14 Kernel 4/7] vfio iommu: Implementation of ioctl " Kirti Wankhede
2020-03-19 3:06 ` Yan Zhao
2020-03-19 4:01 ` Alex Williamson
2020-03-19 4:15 ` Yan Zhao
2020-03-19 4:40 ` Alex Williamson
2020-03-19 6:15 ` Yan Zhao
2020-03-19 13:06 ` Alex Williamson
2020-03-19 16:57 ` Kirti Wankhede
2020-03-20 0:51 ` Yan Zhao
2020-03-19 3:45 ` Alex Williamson
2020-03-19 14:52 ` Kirti Wankhede
2020-03-19 16:22 ` Alex Williamson
2020-03-19 20:25 ` Kirti Wankhede
2020-03-19 20:54 ` Alex Williamson [this message]
2020-03-19 18:57 ` Kirti Wankhede
2020-03-18 19:41 ` [PATCH v14 Kernel 5/7] vfio iommu: Update UNMAP_DMA ioctl to get dirty bitmap before unmap Kirti Wankhede
2020-03-19 3:45 ` Alex Williamson
2020-03-20 8:35 ` Yan Zhao
2020-03-20 15:40 ` Alex Williamson
2020-03-20 15:47 ` Alex Williamson
2020-03-20 19:14 ` Kirti Wankhede
2020-03-20 19:28 ` Alex Williamson
2020-03-23 1:10 ` Yan Zhao
2020-03-18 19:41 ` [PATCH v14 Kernel 6/7] vfio iommu: Adds flag to indicate dirty pages tracking capability support Kirti Wankhede
2020-03-18 19:41 ` [PATCH v14 Kernel 7/7] vfio: Selective dirty page tracking if IOMMU backed device pins pages Kirti Wankhede
2020-03-19 3:45 ` Alex Williamson
2020-03-19 6:24 ` Yan Zhao
2020-03-20 19:41 ` Alex Williamson
2020-03-23 2:43 ` Yan Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200319145421.4b8bd4eb@w520.home \
--to=alex.williamson@redhat.com \
--cc=Ken.Xue@amd.com \
--cc=Zhengxiao.zx@Alibaba-inc.com \
--cc=aik@ozlabs.ru \
--cc=changpeng.liu@intel.com \
--cc=cjia@nvidia.com \
--cc=cohuck@redhat.com \
--cc=dgilbert@redhat.com \
--cc=eauger@redhat.com \
--cc=eskultet@redhat.com \
--cc=felipe@nutanix.com \
--cc=jonathan.davies@nutanix.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=mlevitsk@redhat.com \
--cc=pasic@linux.ibm.com \
--cc=qemu-devel@nongnu.org \
--cc=shuangtai.tst@alibaba-inc.com \
--cc=yan.y.zhao@intel.com \
--cc=yi.l.liu@intel.com \
--cc=zhi.a.wang@intel.com \
--cc=ziye.yang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).