Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl
@ 2026-07-20 22:22 Jaehoon Kim
  2026-07-20 22:47 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jaehoon Kim @ 2026-07-20 22:22 UTC (permalink / raw)
  To: borntraeger, frankja, imbrenda, hca, gor, agordeev
  Cc: david, svens, mjrosato, freimuth, kvm, linux-s390, linux-kernel,
	Jaehoon Kim

FOLL_LONGTERM pinning fails for some memory types, such as file-backed
guest memory. As a result, kvm_s390_adapter_map() returns -EINVAL and
irqfd adapter registration fails even though interrupt delivery could
still work via the existing non-atomic path.

When FOLL_LONGTERM pinning fails, verify that the page is accessible
using a short-term pin instead. If the short-term pin succeeds,
immediately unpin the page and return success. The non-atomic irqfd
path already performs short-term pinning for interrupt delivery, so
this restores the previous behavior for memory that cannot be pinned
long-term.

Fixes: adcd5b3e758b ("KVM: s390: Add map/unmap ioctl and clean mappings post-guest")
Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 9e3e6b0d72ad..4b68e0cb51f6 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2520,8 +2520,19 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
 	map->addr = host_addr;
 	map->page = pin_map_page(kvm, host_addr, FOLL_LONGTERM);
 	if (!map->page) {
-		ret = -EINVAL;
-		goto out;
+		/*
+		 * Long-term pinning may fail for memory types such as file-backed
+		 * memory. Check whether short-term pinning is possible so that the
+		 * non-atomic irqfd path can handle interrupt injection.
+		 */
+		map->page = pin_map_page(kvm, host_addr, 0);
+		if (!map->page) {
+			ret = -EINVAL;
+			goto out;
+		}
+		unpin_user_page(map->page);
+		kfree(map);
+		return 0;
 	}
 	spin_lock_irqsave(&adapter->maps_lock, flags);
 	if (adapter->nr_maps < MAX_S390_ADAPTER_MAPS) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl
  2026-07-20 22:22 [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl Jaehoon Kim
@ 2026-07-20 22:47 ` sashiko-bot
  2026-07-20 22:55 ` Douglas Freimuth
  2026-07-22 15:51 ` Matthew Rosato
  2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-20 22:47 UTC (permalink / raw)
  To: Jaehoon Kim
  Cc: Vasily Gorbik, Heiko Carstens, Alexander Gordeev, linux-s390, kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] UAPI breakage: `kvm_s390_adapter_map` returns success for file-backed memory without recording the mapping, causing `kvm_s390_adapter_unmap` to fail with `-ENOENT`.
--

--- Patch [1]: [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl ---
commit a05b3db94a2a5dc18d9aa3a80842bcd850efd72d
Author: Jaehoon Kim <jhkim@linux.ibm.com>

KVM: s390: Fall back to short-term pinning in MAP ioctl

This commit adds a fallback to short-term pinning in kvm_s390_adapter_map()
when FOLL_LONGTERM pinning fails, such as for file-backed memory. If the
short-term pin succeeds, it unpins the page and returns success so the
non-atomic irqfd path can handle interrupt injection.

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 9e3e6b0d72ad6..4b68e0cb51f6e 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2520,8 +2520,19 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>  	map->addr = host_addr;
>  	map->page = pin_map_page(kvm, host_addr, FOLL_LONGTERM);
>  	if (!map->page) {
> -		ret = -EINVAL;
> -		goto out;
> +		/*
> +		 * Long-term pinning may fail for memory types such as file-backed
> +		 * memory. Check whether short-term pinning is possible so that the
> +		 * non-atomic irqfd path can handle interrupt injection.
> +		 */
> +		map->page = pin_map_page(kvm, host_addr, 0);
> +		if (!map->page) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		unpin_user_page(map->page);
> +		kfree(map);
> +		return 0;

[Severity: High]
Does returning 0 here break UAPI symmetry?

Since the function returns success without adding the mapping to the
adapter->maps list, a later call to kvm_s390_adapter_unmap() for this
successfully mapped address will fail to find it and return -ENOENT:

arch/s390/kvm/interrupt.c:kvm_s390_adapter_unmap() {
    ...
	spin_lock_irqsave(&adapter->maps_lock, flags);
	list_for_each_entry_safe(map, tmp, &adapter->maps, list) {
		if (map->guest_addr == addr) {
			found = true;
			...
		}
	}
	spin_unlock_irqrestore(&adapter->maps_lock, flags);

	return found ? 0 : -ENOENT;
}

Could this cause userspace hypervisors like QEMU to abort or fail device
cleanup and migration when the unmap ioctl unexpectedly fails?

>  	}
>  	spin_lock_irqsave(&adapter->maps_lock, flags);
>  	if (adapter->nr_maps < MAX_S390_ADAPTER_MAPS) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720222238.1576-1-jhkim@linux.ibm.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl
  2026-07-20 22:22 [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl Jaehoon Kim
  2026-07-20 22:47 ` sashiko-bot
@ 2026-07-20 22:55 ` Douglas Freimuth
  2026-07-22 15:51 ` Matthew Rosato
  2 siblings, 0 replies; 5+ messages in thread
From: Douglas Freimuth @ 2026-07-20 22:55 UTC (permalink / raw)
  To: Jaehoon Kim, borntraeger, frankja, imbrenda, hca, gor, agordeev
  Cc: david, svens, mjrosato, kvm, linux-s390, linux-kernel



On 7/20/26 6:22 PM, Jaehoon Kim wrote:
> FOLL_LONGTERM pinning fails for some memory types, such as file-backed
> guest memory. As a result, kvm_s390_adapter_map() returns -EINVAL and
> irqfd adapter registration fails even though interrupt delivery could
> still work via the existing non-atomic path.
> 
> When FOLL_LONGTERM pinning fails, verify that the page is accessible
> using a short-term pin instead. If the short-term pin succeeds,
> immediately unpin the page and return success. The non-atomic irqfd
> path already performs short-term pinning for interrupt delivery, so
> this restores the previous behavior for memory that cannot be pinned
> long-term.
> 
> Fixes: adcd5b3e758b ("KVM: s390: Add map/unmap ioctl and clean mappings post-guest")
> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> ---
>   arch/s390/kvm/interrupt.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 9e3e6b0d72ad..4b68e0cb51f6 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2520,8 +2520,19 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>   	map->addr = host_addr;
>   	map->page = pin_map_page(kvm, host_addr, FOLL_LONGTERM);
>   	if (!map->page) {
> -		ret = -EINVAL;
> -		goto out;
> +		/*
> +		 * Long-term pinning may fail for memory types such as file-backed
> +		 * memory. Check whether short-term pinning is possible so that the
> +		 * non-atomic irqfd path can handle interrupt injection.
> +		 */
> +		map->page = pin_map_page(kvm, host_addr, 0);
> +		if (!map->page) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		unpin_user_page(map->page);
> +		kfree(map);
> +		return 0;
>   	}
>   	spin_lock_irqsave(&adapter->maps_lock, flags);
>   	if (adapter->nr_maps < MAX_S390_ADAPTER_MAPS) {

Reviewed-by: Douglas Freimuth <freimuth@linux.ibm.com>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl
  2026-07-20 22:22 [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl Jaehoon Kim
  2026-07-20 22:47 ` sashiko-bot
  2026-07-20 22:55 ` Douglas Freimuth
@ 2026-07-22 15:51 ` Matthew Rosato
  2026-07-22 17:07   ` Christian Borntraeger
  2 siblings, 1 reply; 5+ messages in thread
From: Matthew Rosato @ 2026-07-22 15:51 UTC (permalink / raw)
  To: Jaehoon Kim, borntraeger, frankja, imbrenda, hca, gor, agordeev
  Cc: david, svens, freimuth, kvm, linux-s390, linux-kernel

On 7/20/26 6:22 PM, Jaehoon Kim wrote:
> FOLL_LONGTERM pinning fails for some memory types, such as file-backed
> guest memory. As a result, kvm_s390_adapter_map() returns -EINVAL and
> irqfd adapter registration fails even though interrupt delivery could
> still work via the existing non-atomic path.
> 
> When FOLL_LONGTERM pinning fails, verify that the page is accessible
> using a short-term pin instead. If the short-term pin succeeds,
> immediately unpin the page and return success. The non-atomic irqfd
> path already performs short-term pinning for interrupt delivery, so
> this restores the previous behavior for memory that cannot be pinned
> long-term.
> 
> Fixes: adcd5b3e758b ("KVM: s390: Add map/unmap ioctl and clean mappings post-guest")
> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> ---
>  arch/s390/kvm/interrupt.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 9e3e6b0d72ad..4b68e0cb51f6 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2520,8 +2520,19 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>  	map->addr = host_addr;
>  	map->page = pin_map_page(kvm, host_addr, FOLL_LONGTERM);
>  	if (!map->page) {
> -		ret = -EINVAL;
> -		goto out;
> +		/*
> +		 * Long-term pinning may fail for memory types such as file-backed
> +		 * memory. Check whether short-term pinning is possible so that the
> +		 * non-atomic irqfd path can handle interrupt injection.
> +		 */
> +		map->page = pin_map_page(kvm, host_addr, 0);
> +		if (!map->page) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		unpin_user_page(map->page);
> +		kfree(map);
> +		return 0;

This makes sense, but what happens later on the kvm_s390_adapter_unmap?

Won't we return -ENOENT because nothing was in the list?

Is it worth detecting/recognizing this case?  I could think of 2 ways...

1) re-doing the above test during unmap

2) stashing an entry in adapter->maps but with a new flag that indicates
it was approved for short-term pinning only (e.g. return NULL
immediately if a match is found during get_map_info() with this flag
set), and during kvm_s390_adapter_unmap() and
kvm_s390_unmap_all_adapters() just remove it from the list without doing
any unpin/page dirtying.

#2 might be a little more code but sounds cleaner?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl
  2026-07-22 15:51 ` Matthew Rosato
@ 2026-07-22 17:07   ` Christian Borntraeger
  0 siblings, 0 replies; 5+ messages in thread
From: Christian Borntraeger @ 2026-07-22 17:07 UTC (permalink / raw)
  To: Matthew Rosato, Jaehoon Kim, frankja, imbrenda, hca, gor,
	agordeev
  Cc: david, svens, freimuth, kvm, linux-s390, linux-kernel



Am 22.07.26 um 17:51 schrieb Matthew Rosato:
> On 7/20/26 6:22 PM, Jaehoon Kim wrote:
>> FOLL_LONGTERM pinning fails for some memory types, such as file-backed
>> guest memory. As a result, kvm_s390_adapter_map() returns -EINVAL and
>> irqfd adapter registration fails even though interrupt delivery could
>> still work via the existing non-atomic path.
>>
>> When FOLL_LONGTERM pinning fails, verify that the page is accessible
>> using a short-term pin instead. If the short-term pin succeeds,
>> immediately unpin the page and return success. The non-atomic irqfd
>> path already performs short-term pinning for interrupt delivery, so
>> this restores the previous behavior for memory that cannot be pinned
>> long-term.
>>
>> Fixes: adcd5b3e758b ("KVM: s390: Add map/unmap ioctl and clean mappings post-guest")
>> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
>> ---
>>   arch/s390/kvm/interrupt.c | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>> index 9e3e6b0d72ad..4b68e0cb51f6 100644
>> --- a/arch/s390/kvm/interrupt.c
>> +++ b/arch/s390/kvm/interrupt.c
>> @@ -2520,8 +2520,19 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>>   	map->addr = host_addr;
>>   	map->page = pin_map_page(kvm, host_addr, FOLL_LONGTERM);
>>   	if (!map->page) {
>> -		ret = -EINVAL;
>> -		goto out;
>> +		/*
>> +		 * Long-term pinning may fail for memory types such as file-backed
>> +		 * memory. Check whether short-term pinning is possible so that the
>> +		 * non-atomic irqfd path can handle interrupt injection.
>> +		 */
>> +		map->page = pin_map_page(kvm, host_addr, 0);
>> +		if (!map->page) {
>> +			ret = -EINVAL;
>> +			goto out;
>> +		}
>> +		unpin_user_page(map->page);
>> +		kfree(map);
>> +		return 0;
> 
> This makes sense, but what happens later on the kvm_s390_adapter_unmap?
> 
> Won't we return -ENOENT because nothing was in the list?
> 
> Is it worth detecting/recognizing this case?  I could think of 2 ways...
> 
> 1) re-doing the above test during unmap
> 
> 2) stashing an entry in adapter->maps but with a new flag that indicates
> it was approved for short-term pinning only (e.g. return NULL
> immediately if a match is found during get_map_info() with this flag
> set), and during kvm_s390_adapter_unmap() and
> kvm_s390_unmap_all_adapters() just remove it from the list without doing
> any unpin/page dirtying.
> 
> #2 might be a little more code but sounds cleaner?
> 

When we are at it, can we also fix  Documentation/virt/kvm/devices/s390_flic.rst ?

To me it looks like qemu does not check the error for unmapping, but yes, maybe 2 is
the right thing to do.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-22 17:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 22:22 [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl Jaehoon Kim
2026-07-20 22:47 ` sashiko-bot
2026-07-20 22:55 ` Douglas Freimuth
2026-07-22 15:51 ` Matthew Rosato
2026-07-22 17:07   ` Christian Borntraeger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox