Re: [PATCH v8 1/3] KVM: s390: Add map/unmap ioctl and clean mappings post-guest

[Matthew Rosato]

On 5/25/26 9:53 PM, Douglas Freimuth wrote:
> s390 needs map/unmap ioctls, which map the adapter set
> indicator pages, so the pages can be accessed when interrupts are
> disabled. The mappings are cleaned up when the guest is removed.
> pin_user_pages_remote is used for both the ioctl as well
> as the pin-on-demand logic in adapter_indicators_set().
> 
> Map/Unmap ioctls are fenced in order to avoid the longterm pinning
> in Secure Execution environments. In Secure Execution
> environments the path of execution available before this patch is followed.
> 
> Statistical counters to count map/unmap functions for adapter indicator
> pages are added. The counters can be used to analyze
> map/unmap functions in non-Secure Execution environments and similarly
> can be used to analyze Secure Execution environments where the counters
> will not be incremented as the adapter indicator pages are not mapped.
> 
> Signed-off-by: Douglas Freimuth <freimuth@xxxxxxxxxxxxx>

I gave you a reviewed-by tag on v7 with the caveat that you move the kvm
lock earlier in register_io_adapter (which you did) so that review tag
should have been included with this patch.  Please include it in next version
or it will get lost.



> @@ -2412,6 +2412,7 @@ static int register_io_adapter(struct kvm_device *dev,
>  	struct s390_io_adapter *adapter;
>  	struct kvm_s390_io_adapter adapter_info;
>  
> +	mutex_lock(&dev->kvm->lock);
>  	if (copy_from_user(&adapter_info,
>  			   (void __user *)attr->addr, sizeof(adapter_info)))
>  		return -EFAULT;
That said...  
Please also handle the sashiko comment for this patch, you moved the kvm lock
earlier like I requested but that means now all of the nonzero return paths
have to drop it before returning, the above is just one example.  

Sounds like a case for either using guard(mutex) [1] or something like

int rc = 0;
if (bad_things_happen) {
  rc = -WHATEVER;
  goto out;
}
[...]
out:
   mutex_unlock(&dev->kvm->lock);
   return rc;

[1]: https://lwn.net/Articles/934679/