Re: [PATCH v2] KVM: s390: Fall back to short-term pinning in MAP ioctl

[JAEHOON KIM <jhkim@linux.ibm.com>]

On 7/23/2026 2:12 PM, Matthew Rosato wrote:
> On 7/23/26 2:10 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, unpin
>> the page and add a map entry with pinned=false to preserve MAP/UNMAP
>> symmetry. 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.
>>
>> get_map_info() is updated to return NULL for unpinned entries so that
>> the atomic irqfd fast path falls back to the non-atomic path.
>> kvm_s390_adapter_unmap() and kvm_s390_unmap_all_adapters() skip dirty
>> marking and unpin for unpinned entries.
>>
>> Update Documentation/virt/kvm/devices/s390_flic.rst to reflect the
>> new MAP/UNMAP behavior.
>>
>> Fixes: adcd5b3e758b ("KVM: s390: Add map/unmap ioctl and clean mappings post-guest")
> cc stable?

Thanks for your feedback.

Since the fixed commit first landed in v7.2-rc1, I don't think this needs to be
CC'd to stable.

The Fixes: tag should reference the mainline commit c9a5688380865b instead of
adcd5b3e758b, which is from the development branch.

I'll update it in the next version.

>
>> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
>> Reviewed-by: Douglas Freimuth <freimuth@linux.ibm.com>
>> ---
>>   Documentation/virt/kvm/devices/s390_flic.rst | 11 +++-
>>   arch/s390/include/asm/kvm_host.h             |  5 ++
>>   arch/s390/kvm/interrupt.c                    | 59 +++++++++++++++-----
>>   3 files changed, 58 insertions(+), 17 deletions(-)
>>
>> diff --git a/Documentation/virt/kvm/devices/s390_flic.rst b/Documentation/virt/kvm/devices/s390_flic.rst
>> index b784f8016748..ec9aaae04e22 100644
>> --- a/Documentation/virt/kvm/devices/s390_flic.rst
>> +++ b/Documentation/virt/kvm/devices/s390_flic.rst
>> @@ -112,9 +112,16 @@ Groups:
>>         mask or unmask the adapter, as specified in mask
>>   
>>       KVM_S390_IO_ADAPTER_MAP
>> -      This is now a no-op. The mapping is purely done by the irq route.
>> +      Map an adapter indicator or summary page for long-term pinning so that
>> +      interrupt injection can be performed in atomic context. If long-term
>> +      pinning is not possible (e.g. file-backed memory), the page is verified
>> +      via a short-term pin and the ioctl returns success; interrupt injection
>> +      will use the non-atomic irqfd path with short-term pinning on each
>> +      interrupt. In Secure Execution mode this is a no-op.
> Nit:
> ... this is a no-op and the ioctl returns success.

I will fix in the next version, thanks.

>
>>       KVM_S390_IO_ADAPTER_UNMAP
>> -      This is now a no-op. The mapping is purely done by the irq route.
>> +      Unmap a previously mapped adapter indicator or summary page and release
>> +      the long-term pin. If the page was not long-term pinned (e.g. file-backed
>> +      memory or Secure Execution mode), this is a no-op and returns success.
> Technically not a no-op in the file-backed case, you will still remove a
> list entry.
>
> How about a little re-wording:
>
> KVM_S390_IO_ADAPTER_UNMAP
>    Unmap a previously mapped adapter indicator or summary page and release
>    the long-term pin. If the page was not long-term pinned (e.g. file-backed
>    memory), the map entry is removed and success is returned; if no prior
>    map entry exists, -ENOENT is returned. In Secure Execution mode this is
>    a no-op and the ioctl returns success.

You're right. In the file-backed case, the map entry is still removed, so
describing it as a no-op is inaccurate. I'll update the documentation based
on your suggested in the next version.

Thanks for the review!

>
> Otherwise, code looks good to me.  Thanks Jaehoon!
>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
>
>>   
>>     KVM_DEV_FLIC_AISM
>>       modify the adapter-interruption-suppression mode for a given isc if the
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index eaa34c5bd3c1..c172f9b212d1 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -476,6 +476,11 @@ struct s390_map_info {
>>   	__u64 guest_addr;
>>   	__u64 addr;
>>   	struct page *page;
>> +	/*
>> +	 * True if the page is long-term pinned. False if long-term pinning
>> +	 * failed and this entry exists only to preserve MAP/UNMAP symmetry.
>> +	 */
>> +	bool pinned;
>>   };
>>   
>>   struct s390_io_adapter {
>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>> index 9e3e6b0d72ad..75eacd1578ef 100644
>> --- a/arch/s390/kvm/interrupt.c
>> +++ b/arch/s390/kvm/interrupt.c
>> @@ -2520,8 +2520,22 @@ 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. Verify that short-term pinning succeeds 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);
>> +		map->page = NULL;
>> +		map->pinned = false;
>> +		/* Add an entry to preserve MAP/UNMAP symmetry. */
>> +	} else {
>> +		map->pinned = true;
>>   	}
>>   	spin_lock_irqsave(&adapter->maps_lock, flags);
>>   	if (adapter->nr_maps < MAX_S390_ADAPTER_MAPS) {
>> @@ -2532,7 +2546,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
>>   		ret = -EINVAL;
>>   	}
>>   	spin_unlock_irqrestore(&adapter->maps_lock, flags);
>> -	if (ret)
>> +	if (ret && map->page)
>>   		unpin_user_page(map->page);
>>   out:
>>   	if (ret)
>> @@ -2546,6 +2560,7 @@ static int kvm_s390_adapter_unmap(struct kvm *kvm, unsigned int id, __u64 addr)
>>   	struct s390_map_info *map, *tmp, *map_to_free;
>>   	struct page *map_page_to_put = NULL;
>>   	u64 map_addr_to_mark = 0;
>> +	bool map_pinned = false;
>>   	unsigned long flags;
>>   	int found = 0, idx;
>>   
>> @@ -2560,6 +2575,7 @@ static int kvm_s390_adapter_unmap(struct kvm *kvm, unsigned int id, __u64 addr)
>>   			list_del(&map->list);
>>   			map_page_to_put = map->page;
>>   			map_addr_to_mark = map->guest_addr;
>> +			map_pinned = map->pinned;
>>   			map_to_free = map;
>>   			break;
>>   		}
>> @@ -2568,11 +2584,18 @@ static int kvm_s390_adapter_unmap(struct kvm *kvm, unsigned int id, __u64 addr)
>>   
>>   	if (found) {
>>   		kfree(map_to_free);
>> -		idx = srcu_read_lock(&kvm->srcu);
>> -		mark_page_dirty(kvm, map_addr_to_mark >> PAGE_SHIFT);
>> -		set_page_dirty_lock(map_page_to_put);
>> -		srcu_read_unlock(&kvm->srcu, idx);
>> -		unpin_user_page(map_page_to_put);
>> +		if (map_pinned) {
>> +			/*
>> +			 * Only long-term pinned pages need to be marked dirty
>> +			 * and released. Fallback entries exist only for
>> +			 * MAP/UNMAP symmetry.
>> +			 */
>> +			idx = srcu_read_lock(&kvm->srcu);
>> +			mark_page_dirty(kvm, map_addr_to_mark >> PAGE_SHIFT);
>> +			set_page_dirty_lock(map_page_to_put);
>> +			srcu_read_unlock(&kvm->srcu, idx);
>> +			unpin_user_page(map_page_to_put);
>> +		}
>>   	}
>>   
>>   	return found ? 0 : -ENOENT;
>> @@ -2598,11 +2621,13 @@ void kvm_s390_unmap_all_adapters(struct kvm *kvm)
>>   
>>   		list_for_each_entry_safe(map, tmp, &local_list, list) {
>>   			list_del(&map->list);
>> -			idx = srcu_read_lock(&kvm->srcu);
>> -			mark_page_dirty(kvm, map->guest_addr >> PAGE_SHIFT);
>> -			set_page_dirty_lock(map->page);
>> -			srcu_read_unlock(&kvm->srcu, idx);
>> -			unpin_user_page(map->page);
>> +			if (map->pinned) {
>> +				idx = srcu_read_lock(&kvm->srcu);
>> +				mark_page_dirty(kvm, map->guest_addr >> PAGE_SHIFT);
>> +				set_page_dirty_lock(map->page);
>> +				srcu_read_unlock(&kvm->srcu, idx);
>> +				unpin_user_page(map->page);
>> +			}
>>   			kfree(map);
>>   		}
>>   	}
>> @@ -2929,8 +2954,12 @@ static struct s390_map_info *get_map_info(struct s390_io_adapter *adapter,
>>   		return NULL;
>>   
>>   	list_for_each_entry(map, &adapter->maps, list) {
>> -		if (map->addr == addr)
>> -			return map;
>> +		if (map->addr == addr) {
>> +			if (map->pinned)
>> +				return map;
>> +			else
>> +				return NULL;
>> +		}
>>   	}
>>   	return NULL;
>>   }