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.
--
Thanks,
David
