Re: [PATCH] KVM: fix async page fault working for readonly mapping

2012-05-22 Thread Avi Kivity
On 05/22/2012 08:31 AM, Gleb Natapov wrote:
 On Mon, May 21, 2012 at 04:30:41PM -0300, Marcelo Tosatti wrote:
 On Mon, May 21, 2012 at 04:15:50PM +0800, Xiao Guangrong wrote:
  On 05/21/2012 04:08 PM, Gleb Natapov wrote:
  
   On Mon, May 21, 2012 at 02:45:45PM +0800, Xiao Guangrong wrote:
   If we map a readonly memory space from host to guest and the page is
   not currently mapped in the host, we will get a fault-pfn and async
   is not allowed, then the vm will crash
  
   Why would we want to map a readonly memory space from host to guest?
   We may want to do it to support memory semantics on read and mmio on
   write, but do not right now unless something changed while I was not
   looking.
  
  
  Some test cases in kvm-unit-tests and the benchmark i am writing for KVM
  need map the function on host to guest.
 
 Or ROM. Or read-only mappings of IVSHMEM (which don't exist yet).
 True. KVM should ignore writes to such areas, not kill a guest. Is this
 how the code works today?
 

Right now qemu maps ROM as RAM.  There is no way to tell kvm that
something is ROM (or ROMD).

There are two options for that:
- mprotect() the ROM, and teach kvm about read-only areas (this patch);
but that doesn't work if we have a read-only and a writable alias of the
same area
- add a flag indicating that an area is ROM or ROMD

I prefer the latter, because of the alias issue.

-- 
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] KVM: fix async page fault working for readonly mapping

2012-05-22 Thread Xiao Guangrong
On 05/22/2012 10:27 PM, Avi Kivity wrote:

 On 05/22/2012 08:31 AM, Gleb Natapov wrote:
 On Mon, May 21, 2012 at 04:30:41PM -0300, Marcelo Tosatti wrote:
 On Mon, May 21, 2012 at 04:15:50PM +0800, Xiao Guangrong wrote:
 On 05/21/2012 04:08 PM, Gleb Natapov wrote:

 On Mon, May 21, 2012 at 02:45:45PM +0800, Xiao Guangrong wrote:
 If we map a readonly memory space from host to guest and the page is
 not currently mapped in the host, we will get a fault-pfn and async
 is not allowed, then the vm will crash

 Why would we want to map a readonly memory space from host to guest?
 We may want to do it to support memory semantics on read and mmio on
 write, but do not right now unless something changed while I was not
 looking.


 Some test cases in kvm-unit-tests and the benchmark i am writing for KVM
 need map the function on host to guest.

 Or ROM. Or read-only mappings of IVSHMEM (which don't exist yet).
 True. KVM should ignore writes to such areas, not kill a guest. Is this
 how the code works today?

 
 Right now qemu maps ROM as RAM.  There is no way to tell kvm that
 something is ROM (or ROMD).
 
 There are two options for that:
 - mprotect() the ROM, and teach kvm about read-only areas (this patch);
 but that doesn't work if we have a read-only and a writable alias of the
 same area
 - add a flag indicating that an area is ROM or ROMD
 
 I prefer the latter, because of the alias issue.
 


I agree.

Will post a new patch to do it if other guys do not object it. :)

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] KVM: fix async page fault working for readonly mapping

2012-05-21 Thread Gleb Natapov
On Mon, May 21, 2012 at 02:45:45PM +0800, Xiao Guangrong wrote:
 If we map a readonly memory space from host to guest and the page is
 not currently mapped in the host, we will get a fault-pfn and async
 is not allowed, then the vm will crash
 
Why would we want to map a readonly memory space from host to guest?
We may want to do it to support memory semantics on read and mmio on
write, but do not right now unless something changed while I was not
looking.

 The reason is only writable vma can be allowed to be async in current
 code
 
 Signed-off-by: Xiao Guangrong xiaoguangr...@linux.vnet.ibm.com
 ---
  virt/kvm/kvm_main.c |   19 +--
  1 files changed, 17 insertions(+), 2 deletions(-)
 
 diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
 index 6bd34a6..b6c8962 100644
 --- a/virt/kvm/kvm_main.c
 +++ b/virt/kvm/kvm_main.c
 @@ -1052,6 +1052,21 @@ static inline int check_user_page_hwpoison(unsigned 
 long addr)
   return rc == -EHWPOISON;
  }
 
 +static bool vma_is_avalid(struct vm_area_struct *vma, bool write_fault)
 +{
 + if (write_fault) {
 + if (unlikely(!(vma-vm_flags  VM_WRITE)))
 + return false;
 +
 + return true;
 + }
 +
 + if (unlikely(!(vma-vm_flags  (VM_READ | VM_EXEC | VM_WRITE
 + return false;
 +
 + return true;
 +}
 +
  static pfn_t hva_to_pfn(struct kvm *kvm, unsigned long addr, bool atomic,
   bool *async, bool write_fault, bool *writable)
  {
 @@ -1075,7 +1090,6 @@ static pfn_t hva_to_pfn(struct kvm *kvm, unsigned long 
 addr, bool atomic,
 
   if (writable)
   *writable = write_fault;
 -
   if (async) {
   down_read(current-mm-mmap_sem);
   npages = get_user_page_nowait(current, current-mm,
 @@ -1122,8 +1136,9 @@ static pfn_t hva_to_pfn(struct kvm *kvm, unsigned long 
 addr, bool atomic,
   vma-vm_pgoff;
   BUG_ON(!kvm_is_mmio_pfn(pfn));
   } else {
 - if (async  (vma-vm_flags  VM_WRITE))
 + if (async  vma_is_avalid(vma, write_fault))
   *async = true;
 +
   pfn = get_fault_pfn();
   }
   up_read(current-mm-mmap_sem);
 -- 
 1.7.7.6
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] KVM: fix async page fault working for readonly mapping

2012-05-21 Thread Xiao Guangrong
On 05/21/2012 04:08 PM, Gleb Natapov wrote:

 On Mon, May 21, 2012 at 02:45:45PM +0800, Xiao Guangrong wrote:
 If we map a readonly memory space from host to guest and the page is
 not currently mapped in the host, we will get a fault-pfn and async
 is not allowed, then the vm will crash

 Why would we want to map a readonly memory space from host to guest?
 We may want to do it to support memory semantics on read and mmio on
 write, but do not right now unless something changed while I was not
 looking.


Some test cases in kvm-unit-tests and the benchmark i am writing for KVM
need map the function on host to guest.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] KVM: fix async page fault working for readonly mapping

2012-05-21 Thread Marcelo Tosatti
On Mon, May 21, 2012 at 04:15:50PM +0800, Xiao Guangrong wrote:
 On 05/21/2012 04:08 PM, Gleb Natapov wrote:
 
  On Mon, May 21, 2012 at 02:45:45PM +0800, Xiao Guangrong wrote:
  If we map a readonly memory space from host to guest and the page is
  not currently mapped in the host, we will get a fault-pfn and async
  is not allowed, then the vm will crash
 
  Why would we want to map a readonly memory space from host to guest?
  We may want to do it to support memory semantics on read and mmio on
  write, but do not right now unless something changed while I was not
  looking.
 
 
 Some test cases in kvm-unit-tests and the benchmark i am writing for KVM
 need map the function on host to guest.

Or ROM. Or read-only mappings of IVSHMEM (which don't exist yet).

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] KVM: fix async page fault working for readonly mapping

2012-05-21 Thread Gleb Natapov
On Mon, May 21, 2012 at 04:30:41PM -0300, Marcelo Tosatti wrote:
 On Mon, May 21, 2012 at 04:15:50PM +0800, Xiao Guangrong wrote:
  On 05/21/2012 04:08 PM, Gleb Natapov wrote:
  
   On Mon, May 21, 2012 at 02:45:45PM +0800, Xiao Guangrong wrote:
   If we map a readonly memory space from host to guest and the page is
   not currently mapped in the host, we will get a fault-pfn and async
   is not allowed, then the vm will crash
  
   Why would we want to map a readonly memory space from host to guest?
   We may want to do it to support memory semantics on read and mmio on
   write, but do not right now unless something changed while I was not
   looking.
  
  
  Some test cases in kvm-unit-tests and the benchmark i am writing for KVM
  need map the function on host to guest.
 
 Or ROM. Or read-only mappings of IVSHMEM (which don't exist yet).
True. KVM should ignore writes to such areas, not kill a guest. Is this
how the code works today?

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html