New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 845733 link

Starred by 3 users
Status: Fixed
Owner:
sonnyrao@chromium.org
Closed: May 2018
Cc:
dgreid@chromium.org
█ reve...@chromium.org
tbuck...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

DMABuf sharing to guest doesn't work on ARM

Project Member Reported by reve...@chromium.org, May 22 2018 
crosvm doesn't report any errors but guest can't access the memory.

Comment 1 by sonnyrao@chromium.org, May 23 2018 

What I see so far is that we're taking a page fault when we try to access the new memory region that's mapped into the guest with the DMAbuf and we're not faulting in the DMA buf at that time but instead returning it as an error from KVM_RUN

It looks like all of the calls to KVM_SET_USER_MEMORY_REGION succeed including the one for the DMA buf.

It looks like the generic kvm code in virt/kvm/kvm_main.c function hva_to_pfn() 
is looking for VM_PFNMAP on the vma found for the faulting address and doesn't find it so it returns an error.

The VM_PFNMAP flag means "Page-ranges managed without "struct page", just pure PFN "

The VMA that we are mapping doesn't have the VM_PFNMAP flag set but it does have the VM_MIXEDMAP flag set which means "Can contain "struct page" and pure PFN pages "

So I'm trying to determine what the situation is for x86 and whether we also get a fault there and fix it up, and what flags might be set on the VMA being mapped.

Comment 2 by sonnyrao@chromium.org, May 23 2018 

I've discovered a some upstream commits which looks like they may resolve this situation with VM_MIXEDMAP regions

92176a8ede577d KVM: MMU: prepare to support mapping of VM_IO and VM_PFNMAP frames
add6a0cd1c5ba5 KVM: MMU: try to fix up page faults before giving up

So I'm going to try these out

Comment 3 by sonnyrao@chromium.org, May 23 2018

Cc: dgreid@chromium.org
Status: Started (was: Assigned)
Ok those patches in #3 plus one other internal API change seem to fix it -- 
Series is here:

https://chromium-review.googlesource.com/#/c/chromiumos/third_party/kernel/+/1069945

Comment 4 by reve...@chromium.org, May 23 2018 

Awesome! I've updated my patches that enable to dmabufs to no longer be restricted to amd64 and added a CQ-DEPEND on the above series.
Project Member

Comment 5 by bugdroid1@chromium.org, May 24 2018

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/114d180f714f4740850400bb06b2a6a5cdc873f8

commit 114d180f714f4740850400bb06b2a6a5cdc873f8
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Thu May 24 07:22:54 2018

BACKPORT: KVM: MMU: prepare to support mapping of VM_IO and VM_PFNMAP frames

Handle VM_IO like VM_PFNMAP, as is common in the rest of Linux; extract
the formula to convert hva->pfn into a new function, which will soon
gain more capabilities.

BUG= chromium:845733 
TEST=run kvm with wayland dmabuf on kevin, no crashes

Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Radim Krm <rkrcmar@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from 92176a8ede577d0ff78ab3298e06701f67ad5f51)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
[SR: need pfn_t instead of kvm_pfn_t]
 Conflicts:
	virt/kvm/kvm_main.c

Change-Id: Ia6085753d8be2684ff0ea9b16061ab8101c921e7
Reviewed-on: https://chromium-review.googlesource.com/1069943
Commit-Ready: David Reveman <reveman@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/114d180f714f4740850400bb06b2a6a5cdc873f8/virt/kvm/kvm_main.c
Project Member

Comment 6 by bugdroid1@chromium.org, May 24 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/af4d8ae5820e653545d1b087183d12cb8c6725a8

commit af4d8ae5820e653545d1b087183d12cb8c6725a8
Author: Dominik Dingel <dingel@linux.vnet.ibm.com>
Date: Thu May 24 07:22:56 2018

UPSTREAM: mm: bring in additional flag for fixup_user_fault to signal unlock

During Jason's work with postcopy migration support for s390 a problem
regarding gmap faults was discovered.

The gmap code will call fixup_user_fault which will end up always in
handle_mm_fault.  Till now we never cared about retries, but as the
userfaultfd code kind of relies on it.  this needs some fix.

This patchset does not take care of the futex code.  I will now look
closer at this.

This patch (of 2):

With the introduction of userfaultfd, kvm on s390 needs fixup_user_fault
to pass in FAULT_FLAG_ALLOW_RETRY and give feedback if during the
faulting we ever unlocked mmap_sem.

This patch brings in the logic to handle retries as well as it cleans up
the current documentation.  fixup_user_fault was not having the same
semantics as filemap_fault.  It never indicated if a retry happened and
so a caller wasn't able to handle that case.  So we now changed the
behaviour to always retry a locked mmap_sem.

BUG= chromium:845733 
TEST=run kvm with wayland dmabuf on kevin, no crashes

Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Eric B Munson <emunson@akamai.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
(cherry picked from commit 4a9e1cda274893eca7d178d7dc265503ccb9d87a)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I02163622661117de4b2890f9e36d7d6c2daaddf5
Reviewed-on: https://chromium-review.googlesource.com/1069944
Commit-Ready: David Reveman <reveman@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/af4d8ae5820e653545d1b087183d12cb8c6725a8/include/linux/mm.h
[modify] https://crrev.com/af4d8ae5820e653545d1b087183d12cb8c6725a8/kernel/futex.c
[modify] https://crrev.com/af4d8ae5820e653545d1b087183d12cb8c6725a8/arch/s390/mm/pgtable.c
[modify] https://crrev.com/af4d8ae5820e653545d1b087183d12cb8c6725a8/mm/gup.c

Comment 7 by sonnyrao@chromium.org, May 24 2018

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 15 2018

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/10e524e99053dd8dc15061743e6bada85f74617e

commit 10e524e99053dd8dc15061743e6bada85f74617e
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Fri Jun 15 17:54:23 2018

BACKPORT: KVM: MMU: prepare to support mapping of VM_IO and VM_PFNMAP frames

Handle VM_IO like VM_PFNMAP, as is common in the rest of Linux; extract
the formula to convert hva->pfn into a new function, which will soon
gain more capabilities.

BUG= chromium:845733 
TEST=build/boot on hana with USE=kvm_host

Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Radim Krm <rkrcmar@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from 92176a8ede577d0ff78ab3298e06701f67ad5f51)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
[SR: need pfn_t instead of kvm_pfn_t]
 Conflicts:
	virt/kvm/kvm_main.c

Change-Id: Ia6085753d8be2684ff0ea9b16061ab8101c921e7
Reviewed-on: https://chromium-review.googlesource.com/1069943
Commit-Ready: David Reveman <reveman@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1088123
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/10e524e99053dd8dc15061743e6bada85f74617e/virt/kvm/kvm_main.c
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 15 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0ffa062e398d856904c4ca90cc13a50411519270

commit 0ffa062e398d856904c4ca90cc13a50411519270
Author: Dominik Dingel <dingel@linux.vnet.ibm.com>
Date: Fri Jun 15 17:54:24 2018

UPSTREAM: mm: bring in additional flag for fixup_user_fault to signal unlock

During Jason's work with postcopy migration support for s390 a problem
regarding gmap faults was discovered.

The gmap code will call fixup_user_fault which will end up always in
handle_mm_fault.  Till now we never cared about retries, but as the
userfaultfd code kind of relies on it.  this needs some fix.

This patchset does not take care of the futex code.  I will now look
closer at this.

This patch (of 2):

With the introduction of userfaultfd, kvm on s390 needs fixup_user_fault
to pass in FAULT_FLAG_ALLOW_RETRY and give feedback if during the
faulting we ever unlocked mmap_sem.

This patch brings in the logic to handle retries as well as it cleans up
the current documentation.  fixup_user_fault was not having the same
semantics as filemap_fault.  It never indicated if a retry happened and
so a caller wasn't able to handle that case.  So we now changed the
behaviour to always retry a locked mmap_sem.

BUG= chromium:845733 
TEST=build/boot on hana with USE=kvm_host

Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Eric B Munson <emunson@akamai.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
(cherry picked from commit 4a9e1cda274893eca7d178d7dc265503ccb9d87a)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I02163622661117de4b2890f9e36d7d6c2daaddf5
Reviewed-on: https://chromium-review.googlesource.com/1069944
Commit-Ready: David Reveman <reveman@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1088124
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/0ffa062e398d856904c4ca90cc13a50411519270/include/linux/mm.h
[modify] https://crrev.com/0ffa062e398d856904c4ca90cc13a50411519270/kernel/futex.c
[modify] https://crrev.com/0ffa062e398d856904c4ca90cc13a50411519270/arch/s390/mm/pgtable.c
[modify] https://crrev.com/0ffa062e398d856904c4ca90cc13a50411519270/mm/gup.c
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 15 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2973fc130567271b30947f66fa70ab5c04e890f5

commit 2973fc130567271b30947f66fa70ab5c04e890f5
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Fri Jun 15 17:54:26 2018

UPSTREAM: KVM: MMU: try to fix up page faults before giving up

The vGPU folks would like to trap the first access to a BAR by setting
vm_ops on the VMAs produced by mmap-ing a VFIO device.  The fault handler
then can use remap_pfn_range to place some non-reserved pages in the VMA.

This kind of VM_PFNMAP mapping is not handled by KVM, but follow_pfn
and fixup_user_fault together help supporting it.  The patch also supports
VM_MIXEDMAP vmas where the pfns are not reserved and thus subject to
reference counting.

BUG= chromium:845733 
TEST=build/boot on hana with USE=kvm_host

Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Radim Krm <rkrcmar@redhat.com>
Tested-by: Neo Jia <cjia@nvidia.com>
Reported-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit add6a0cd1c5ba51b201e1361b05a5df817083618)
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

Change-Id: I6d28ebd115ba07b0ec017a7c59eb815c0a6ed5e6
Reviewed-on: https://chromium-review.googlesource.com/1069945
Commit-Ready: David Reveman <reveman@chromium.org>
Tested-by: David Reveman <reveman@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1088125
Commit-Ready: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/2973fc130567271b30947f66fa70ab5c04e890f5/virt/kvm/kvm_main.c
[modify] https://crrev.com/2973fc130567271b30947f66fa70ab5c04e890f5/mm/gup.c
►

Sign in to add a comment