On Wed, 9 Nov 2016 00:17:53 +0530 Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote: > On 11/8/2016 10:09 PM, Alex Williamson wrote: > > On Tue, 8 Nov 2016 19:25:35 +0530 > > Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote: > > > ... > > >>>> - > >>>> + int (*pin_pages)(void *iommu_data, unsigned long *user_pfn, > >>>> + int npage, int prot, > >>>> + unsigned long *phys_pfn); > >>>> + int (*unpin_pages)(void *iommu_data, > >>> > >>> Are we changing from long to int here simply because of the absurdity > >>> in passing in more than a 2^31 entry array, that would already consume > >>> more than 16GB itself? > >>> > >> > >> These are on demand pin/unpin request, will that request go beyond 16GB > >> limit? For Nvidia vGPU solution, pin request will not go beyond this limit. > > > > 16G is simply the size of the user_pfn or phys_pfn arrays at a maximal > > int32_t npage value, the interface actually allows mapping up to 8TB > > per call, but at that point we have 16GB of input, 16GB of output, and > > 80GB of vfio_pfns created. So I don't really have a problem changing > > form long to int given lack of scalability in the API in general, but > > it does make me second guess the API itself. Thanks, > > > > Changing to 'long', in future we might enhance this API without changing > it signature. I think the pfn arrays are more of a problem long term than whether we can only map 2^31 pfns in one call. I particularly dislike that the caller provides both the iova and pfn arrays for unpinning. Being an in-kernel driver, we should trust it, but it makes the interface difficult to use and seems like it indicates that our tracking data structures aren't architected the way they should be. Upstream, this API will need to be flexible and change over time, it's only the downstream distros that may lock in a kABI. Not breaking them should be a consideration, but also needs to be weighted against long term upstream goals. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
