On 08/31/2017 02:13 PM, David Hildenbrand wrote:
> On 31.08.2017 13:44, Christian Borntraeger wrote:
>>
>>
>> On 08/30/2017 06:06 PM, David Hildenbrand wrote:
>>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>>> ---
>>> arch/s390/kvm/vsie.c | 19 +++++--------------
>>> include/linux/kvm_host.h | 1 +
>>> virt/kvm/kvm_main.c | 4 ++--
>>> 3 files changed, 8 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>> index b5eec30eb37d..a21763b4c229 100644
>>> --- a/arch/s390/kvm/vsie.c
>>> +++ b/arch/s390/kvm/vsie.c
>>> @@ -445,17 +445,12 @@ static int map_prefix(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>> static int pin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t *hpa)
>>> {
>>> struct page *page;
>>> - hva_t hva;
>>> - int rc;
>>>
>>> - hva = gfn_to_hva(kvm, gpa_to_gfn(gpa));
>>> - if (kvm_is_error_hva(hva))
>>> - return -EINVAL;
>>> - rc = get_user_pages_fast(hva, 1, 1, &page);
>>> - if (rc < 0)
>>> - return rc;
>>> - else if (rc != 1)
>>> + page = gfn_to_page(kvm, gpa_to_gfn(gpa));
>>> + if (PTR_ERR(page) == -ENOMEM)
>>> return -ENOMEM;
>>
>> Can this actually happen? Reading gfn_to_page does not seem
>> to have a path that returns ENOMEM.
>>
>
> Boils down to what get_user_pages_fast() guarantees (which is called by
> gfn_to_hva() now). It can call __get_user_pages(). And there I can at
> least spot a -ENOMEM potentially coming out of faultin_page().
> This check essentially makes sure that the behavior of pin_guest_page()
> doesn't change (this function is specified to return either -ENOMEM or
> -EINVAL). So unless there is a very good reason, I think we should keep
> it that way.
I might be misreading the code, but it looks like that
gfn_to_page will will call
kvm_pfn_to_page and that function will return KVM_ERR_PTR_BAD_PAGE if something
fails (which is (ERR_PTR(-ENOENT)). But it will never return ERR_PTR(-ENOMEM)
