On Mon, 2018-10-08 at 22:41 +0800, kvm-owner@xxxxxxxxxxxxxxx wrote:
> Case (!write_fault && writable) has been handled in hva_to_pfn_fast(),
> it is not necessary to try again if hva_to_pfn_fast() already failed.
>
> This patch removes this case in hva_to_pfn_slow().
>
> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>
> ---
>
> Hope my understanding is correct.
>
> ---
> virt/kvm/kvm_main.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 1f42f1d474b5..c8fb3a9d81fa 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1403,16 +1403,6 @@ static int hva_to_pfn_slow(unsigned long addr, bool *async, bool write_fault,
> if (npages != 1)
> return npages;
>
> - /* map read fault as writable if possible */
> - if (unlikely(!write_fault) && writable) {
> - struct page *wpage;
> -
> - if (__get_user_pages_fast(addr, 1, 1, &wpage) == 1) {
> - *writable = true;
> - put_page(page);
> - page = wpage;
> - }
> - }
This handles the scenario where __get_user_pages_fast() failed and
get_user_pages_unlocked() succeeded, obviously with !write_fault &&
writable. On a read fault, get_user_pages_unlocked() requests the
page with read permissions to ensure it doesn't incorrectly fail due
to invalid access permissions, i.e. doesn't speculatively request
write permissions for a read-only VMA. If that succeeds, we then
use __get_user_pages_fast() to test if the VMA is writable. If the
VMA is indeed writable, we can create the shadow PTE with write
permissions even though we're handling a read fault. Marking the
SPTE as writable means we don't take another page fault if/when the
guest writes the page, e.g. in a RMW scenario we take one page fault
instead of two.
> *pfn = page_to_pfn(page);
> return npages;
> }
