All of lore.kernel.org
 help / color / mirror / Atom feed
From: JAEHOON KIM <jhkim@linux.ibm.com>
To: Christian Borntraeger <borntraeger@linux.ibm.com>,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	frankja@linux.ibm.com, imbrenda@linux.ibm.com, hca@linux.ibm.com,
	gor@linux.ibm.com, agordeev@linux.ibm.com
Cc: david@kernel.org, svens@linux.ibm.com, freimuth@linux.ibm.com,
	kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] KVM: s390: Fall back to short-term pinning in MAP ioctl
Date: Wed, 22 Jul 2026 13:05:40 -0500	[thread overview]
Message-ID: <ed15f41a-d2c3-45b9-a5f6-2e86bcdf290a@linux.ibm.com> (raw)
In-Reply-To: <2a080b3b-1a2f-489d-9d66-492e00c57f72@linux.ibm.com>

On 7/22/2026 12:07 PM, Christian Borntraeger wrote:
>
>
> 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.

I agree that option #2 is the cleaner approach as well. I'll also update the
documentation.

Thanks,
Jaehoon.


      reply	other threads:[~2026-07-22 18:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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
2026-07-22 18:05     ` JAEHOON KIM [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ed15f41a-d2c3-45b9-a5f6-2e86bcdf290a@linux.ibm.com \
    --to=jhkim@linux.ibm.com \
    --cc=agordeev@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=david@kernel.org \
    --cc=frankja@linux.ibm.com \
    --cc=freimuth@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mjrosato@linux.ibm.com \
    --cc=svens@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.