On Sun, Apr 27, 2025 at 03:22:13PM +0800, Baolu Lu wrote:
On 4/26/25 13:58, Nicolin Chen wrote:
The new vCMDQ object will be added for HW to access the guest memory for a HW-accelerated virtualization feature. It needs to ensure the guest memory pages are pinned when HW accesses them and they are contiguous in physical address space.
This is very like the existing iommufd_access_pin_pages() that outputs the pinned page list for the caller to test its contiguity.
Move those code from iommufd_access_pin/unpin_pages() and related function for a pair of iopt helpers that can be shared with the vCMDQ allocator. As the vCMDQ allocator will be a user-space triggered ioctl function, WARN_ON would not be a good fit in the new iopt_unpin_pages(), thus change them to use WARN_ON_ONCE instead.
I'm uncertain, but perhaps pr_warn_ratelimited() would be a better alternative to WARN_ON() here? WARN_ON_ONCE() generates warning messages with kernel call traces in the kernel messages, which might lead users to believe that something serious has happened in the kernel.
We already have similar practice, e.g. iommufd_hwpt_nested_alloc.
In my review, a WARN_ON/WARN_ON_ONCE means there is a kernel bug, which shouldn't occur in the first place and isn't something that user space should concern. In case that it is hit, a WARN_ON_ONCE only spits one piece of traces that is enough for kernel folks to identify what's wrong, while pr_warn_ratelimited would likely end up with periodical warnings (more lines) that are neither related to user space nor useful for kernel.
Thanks Nicolin