* [PATCH] xen: set vma flag VM_PFNMAP in the privcmd mmap file_op @ 2010-11-11 15:55 Stefano Stabellini 2010-11-11 16:40 ` [Xen-devel] " Konrad Rzeszutek Wilk 0 siblings, 1 reply; 4+ messages in thread From: Stefano Stabellini @ 2010-11-11 15:55 UTC (permalink / raw) To: Jeremy Fitzhardinge Cc: linux-kernel, xen-devel, Konrad Rzeszutek Wilk, Ian Campbell Hi all, this patch fixes the dom0 kernel crash when creating a VM. Now I am able to create VMs successfully on 2.6.37 rc1, even though without disk or network access. --- xen: set vma flag VM_PFNMAP in the privcmd mmap file_op Set VM_PFNMAP in the privcmd mmap file_op, rather than later in xen_remap_domain_mfn_range when it is too late because vma_wants_writenotify has already been called and vm_page_prot has already been modified. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c index c237b81..2be1f36 100644 --- a/arch/x86/xen/mmu.c +++ b/arch/x86/xen/mmu.c @@ -2627,7 +2627,7 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma, prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP); - vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP; + BUG_ON(!(vma->vm_flags & (VM_PFNMAP | VM_RESERVED | VM_IO))); rmd.mfn = mfn; rmd.prot = prot; diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c index f80be7f..9aab216 100644 --- a/drivers/xen/xenfs/privcmd.c +++ b/drivers/xen/xenfs/privcmd.c @@ -385,7 +385,7 @@ static int privcmd_mmap(struct file *file, struct vm_area_struct *vma) return -ENOSYS; /* DONTCOPY is essential for Xen as copy_page_range is broken. */ - vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY; + vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY | VM_PFNMAP; vma->vm_ops = &privcmd_vm_ops; vma->vm_private_data = NULL; ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Xen-devel] [PATCH] xen: set vma flag VM_PFNMAP in the privcmd mmap file_op 2010-11-11 15:55 [PATCH] xen: set vma flag VM_PFNMAP in the privcmd mmap file_op Stefano Stabellini @ 2010-11-11 16:40 ` Konrad Rzeszutek Wilk 2010-11-11 17:21 ` Jeremy Fitzhardinge 0 siblings, 1 reply; 4+ messages in thread From: Konrad Rzeszutek Wilk @ 2010-11-11 16:40 UTC (permalink / raw) To: Stefano Stabellini Cc: Jeremy Fitzhardinge, Ian Campbell, xen-devel, linux-kernel On Thu, Nov 11, 2010 at 03:55:49PM +0000, Stefano Stabellini wrote: > Hi all, > this patch fixes the dom0 kernel crash when creating a VM. > Now I am able to create VMs successfully on 2.6.37 rc1, even though > without disk or network access. > > --- > > xen: set vma flag VM_PFNMAP in the privcmd mmap file_op > > Set VM_PFNMAP in the privcmd mmap file_op, rather than later in > xen_remap_domain_mfn_range when it is too late because > vma_wants_writenotify has already been called and vm_page_prot has So vma_wants_writenotify sets the invalid flags on vma->vm_flags? > already been modified. By whom? vma_wants_writenotify looks to just return 0 or 1 Ah, depending on that return value it sets vma->vm_page_prot. That looks odd, so if this: 1215 if (vma_wants_writenotify(vma)) 1216 vma->vm_page_prot = vm_get_page_prot(vm_flags & ~VM_SHARED); 1217 does not set the vma->vm_page_prot we never set the vm_page_prot? .. and it looks to not set that value earlier on either. So VM_PFNMAP inhibits the mmap code from setting the vm_page_prot. Is that what we want, not have vma->vm_page_prot set anything? Why? > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c > index c237b81..2be1f36 100644 > --- a/arch/x86/xen/mmu.c > +++ b/arch/x86/xen/mmu.c > @@ -2627,7 +2627,7 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma, > > prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP); > > - vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP; > + BUG_ON(!(vma->vm_flags & (VM_PFNMAP | VM_RESERVED | VM_IO))); > > rmd.mfn = mfn; > rmd.prot = prot; > diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c > index f80be7f..9aab216 100644 > --- a/drivers/xen/xenfs/privcmd.c > +++ b/drivers/xen/xenfs/privcmd.c > @@ -385,7 +385,7 @@ static int privcmd_mmap(struct file *file, struct vm_area_struct *vma) > return -ENOSYS; > > /* DONTCOPY is essential for Xen as copy_page_range is broken. */ > - vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY; > + vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY | VM_PFNMAP; > vma->vm_ops = &privcmd_vm_ops; > vma->vm_private_data = NULL; > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Xen-devel] [PATCH] xen: set vma flag VM_PFNMAP in the privcmd mmap file_op 2010-11-11 16:40 ` [Xen-devel] " Konrad Rzeszutek Wilk @ 2010-11-11 17:21 ` Jeremy Fitzhardinge 2010-11-11 17:42 ` Stefano Stabellini 0 siblings, 1 reply; 4+ messages in thread From: Jeremy Fitzhardinge @ 2010-11-11 17:21 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Stefano Stabellini, xen-devel, Ian Campbell, linux-kernel On 11/11/2010 08:40 AM, Konrad Rzeszutek Wilk wrote: > On Thu, Nov 11, 2010 at 03:55:49PM +0000, Stefano Stabellini wrote: >> Hi all, >> this patch fixes the dom0 kernel crash when creating a VM. >> Now I am able to create VMs successfully on 2.6.37 rc1, even though >> without disk or network access. >> >> --- >> >> xen: set vma flag VM_PFNMAP in the privcmd mmap file_op >> >> Set VM_PFNMAP in the privcmd mmap file_op, rather than later in >> xen_remap_domain_mfn_range when it is too late because >> vma_wants_writenotify has already been called and vm_page_prot has > So vma_wants_writenotify sets the invalid flags on vma->vm_flags? >> already been modified. > By whom? vma_wants_writenotify looks to just return 0 or 1 > > Ah, depending on that return value it sets vma->vm_page_prot. > That looks odd, so if this: > > 1215 if (vma_wants_writenotify(vma)) > 1216 vma->vm_page_prot = vm_get_page_prot(vm_flags & ~VM_SHARED); > 1217 > > does not set the vma->vm_page_prot we never set the vm_page_prot? > > .. and it looks to not set that value earlier on either. > > So VM_PFNMAP inhibits the mmap code from setting the vm_page_prot. > Is that what we want, not have vma->vm_page_prot set anything? Why? It is already set unconditionally earlier in mmap_region(). The writenotify stuff will knobble the mapping to be RO so we can track the first write with faults, but we don't want that in this case. The VM_PFNMAP flag is generally correct anyway, since it means that the mapping is of a random PFN which no corresponding struct page. J >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c >> index c237b81..2be1f36 100644 >> --- a/arch/x86/xen/mmu.c >> +++ b/arch/x86/xen/mmu.c >> @@ -2627,7 +2627,7 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma, >> >> prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP); >> >> - vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP; >> + BUG_ON(!(vma->vm_flags & (VM_PFNMAP | VM_RESERVED | VM_IO))); >> >> rmd.mfn = mfn; >> rmd.prot = prot; >> diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c >> index f80be7f..9aab216 100644 >> --- a/drivers/xen/xenfs/privcmd.c >> +++ b/drivers/xen/xenfs/privcmd.c >> @@ -385,7 +385,7 @@ static int privcmd_mmap(struct file *file, struct vm_area_struct *vma) >> return -ENOSYS; >> >> /* DONTCOPY is essential for Xen as copy_page_range is broken. */ >> - vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY; >> + vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY | VM_PFNMAP; >> vma->vm_ops = &privcmd_vm_ops; >> vma->vm_private_data = NULL; >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Xen-devel] [PATCH] xen: set vma flag VM_PFNMAP in the privcmd mmap file_op 2010-11-11 17:21 ` Jeremy Fitzhardinge @ 2010-11-11 17:42 ` Stefano Stabellini 0 siblings, 0 replies; 4+ messages in thread From: Stefano Stabellini @ 2010-11-11 17:42 UTC (permalink / raw) To: Jeremy Fitzhardinge Cc: Konrad Rzeszutek Wilk, Stefano Stabellini, xen-devel@lists.xensource.com, Ian Campbell, linux-kernel@vger.kernel.org On Thu, 11 Nov 2010, Jeremy Fitzhardinge wrote: > On 11/11/2010 08:40 AM, Konrad Rzeszutek Wilk wrote: > > On Thu, Nov 11, 2010 at 03:55:49PM +0000, Stefano Stabellini wrote: > >> Hi all, > >> this patch fixes the dom0 kernel crash when creating a VM. > >> Now I am able to create VMs successfully on 2.6.37 rc1, even though > >> without disk or network access. > >> > >> --- > >> > >> xen: set vma flag VM_PFNMAP in the privcmd mmap file_op > >> > >> Set VM_PFNMAP in the privcmd mmap file_op, rather than later in > >> xen_remap_domain_mfn_range when it is too late because > >> vma_wants_writenotify has already been called and vm_page_prot has > > So vma_wants_writenotify sets the invalid flags on vma->vm_flags? > >> already been modified. > > By whom? vma_wants_writenotify looks to just return 0 or 1 > > > > Ah, depending on that return value it sets vma->vm_page_prot. > > That looks odd, so if this: > > > > 1215 if (vma_wants_writenotify(vma)) > > 1216 vma->vm_page_prot = vm_get_page_prot(vm_flags & ~VM_SHARED); > > 1217 > > > > does not set the vma->vm_page_prot we never set the vm_page_prot? > > > > .. and it looks to not set that value earlier on either. > > > > So VM_PFNMAP inhibits the mmap code from setting the vm_page_prot. > > Is that what we want, not have vma->vm_page_prot set anything? Why? > > It is already set unconditionally earlier in mmap_region(). The > writenotify stuff will knobble the mapping to be RO so we can track the > first write with faults, but we don't want that in this case. > exactly. > The VM_PFNMAP flag is generally correct anyway, since it means that the > mapping is of a random PFN which no corresponding struct page. > I think so too. This is an update of the patch, with a more correct BUG_ON condition. --- xen: set vma flag VM_PFNMAP in the privcmd mmap file_op Set VM_PFNMAP in the privcmd mmap file_op, rather than later in xen_remap_domain_mfn_range when it is too late because vma_wants_writenotify has already been called and vm_page_prot has already been modified. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c index c237b81..f61b75d 100644 --- a/arch/x86/xen/mmu.c +++ b/arch/x86/xen/mmu.c @@ -2627,7 +2627,8 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma, prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP); - vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP; + BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_RESERVED | VM_IO)) == + (VM_PFNMAP | VM_RESERVED | VM_IO))); rmd.mfn = mfn; rmd.prot = prot; diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c index f80be7f..48f7b0f 100644 --- a/drivers/xen/xenfs/privcmd.c +++ b/drivers/xen/xenfs/privcmd.c @@ -384,8 +384,9 @@ static int privcmd_mmap(struct file *file, struct vm_area_struct *vma) if (xen_feature(XENFEAT_auto_translated_physmap)) return -ENOSYS; - /* DONTCOPY is essential for Xen as copy_page_range is broken. */ - vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY; + /* DONTCOPY is essential for Xen because copy_page_range doesn't know + * how to recreate these mappings */ + vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY | VM_PFNMAP; vma->vm_ops = &privcmd_vm_ops; vma->vm_private_data = NULL; ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-11 17:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-11 15:55 [PATCH] xen: set vma flag VM_PFNMAP in the privcmd mmap file_op Stefano Stabellini 2010-11-11 16:40 ` [Xen-devel] " Konrad Rzeszutek Wilk 2010-11-11 17:21 ` Jeremy Fitzhardinge 2010-11-11 17:42 ` Stefano Stabellini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.