[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] x86/xen: safely map and unmap grant frames when in atomic context
On Wed, Jul 02, 2014 at 11:25:28AM +0100, David Vrabel wrote: > arch_gnttab_map_frames() and arch_gnttab_unmap_frames() are called in > atomic context but were calling alloc_vm_area() which might sleep. > > Also, if a driver attempts to allocate a grant ref from an interrupt > and the table needs expanding, then the CPU may already by in lazy MMU > mode and apply_to_page_range() will BUG when it tries to re-enable > lazy MMU mode. > > These two functions are only used in PV guests. > > Introduce arch_gnttab_init() to allocates the virtual address space in > advance. > > Avoid the use of apply_to_page_range() by using saving and using the > array of PTE addresses from the alloc_vm_area() call. > > Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx> With: a) I believe all the __init has to go as the code will be called during suspend/resume cycle. But you already know that :-) b) If you could also add in the description: "alloc_vm_area' pre-allocates the pagetable so there is no need to worry about having to do a PGD/PUD/PMD walk (like apply_to_page_range does) and we can instead do set_pte. that you can stick on it: Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> Thank you! > --- > arch/arm/xen/grant-table.c | 5 ++ > arch/x86/xen/grant-table.c | 147 > +++++++++++++++++++++++++++----------------- > drivers/xen/grant-table.c | 9 ++- > include/xen/grant_table.h | 1 + > 4 files changed, 105 insertions(+), 57 deletions(-) > > diff --git a/arch/arm/xen/grant-table.c b/arch/arm/xen/grant-table.c > index 859a9bb..91cf08b 100644 > --- a/arch/arm/xen/grant-table.c > +++ b/arch/arm/xen/grant-table.c > @@ -51,3 +51,8 @@ int arch_gnttab_map_status(uint64_t *frames, unsigned long > nr_gframes, > { > return -ENOSYS; > } > + > +int arch_gnttab_init(unsigned long nr_shared, unsigned long nr_status) > +{ > + return 0; > +} > diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c > index c985835..27301df 100644 > --- a/arch/x86/xen/grant-table.c > +++ b/arch/x86/xen/grant-table.c > @@ -36,6 +36,7 @@ > > #include <linux/sched.h> > #include <linux/mm.h> > +#include <linux/slab.h> > #include <linux/vmalloc.h> > > #include <xen/interface/xen.h> > @@ -44,87 +45,121 @@ > > #include <asm/pgtable.h> > > -static int map_pte_fn(pte_t *pte, struct page *pmd_page, > - unsigned long addr, void *data) > +static struct gnttab_vm_area { > + struct vm_struct *area; > + pte_t **ptes; > +} gnttab_shared_vm_area, gnttab_status_vm_area; > + > +int arch_gnttab_map_shared(unsigned long *frames, unsigned long nr_gframes, > + unsigned long max_nr_gframes, > + void **__shared) > { > - unsigned long **frames = (unsigned long **)data; > + void *shared = *__shared; > + unsigned long addr; > + unsigned long i; > > - set_pte_at(&init_mm, addr, pte, mfn_pte((*frames)[0], PAGE_KERNEL)); > - (*frames)++; > - return 0; > -} > + if (shared == NULL) > + *__shared = shared = gnttab_shared_vm_area.area->addr; > > -/* > - * This function is used to map shared frames to store grant status. It is > - * different from map_pte_fn above, the frames type here is uint64_t. > - */ > -static int map_pte_fn_status(pte_t *pte, struct page *pmd_page, > - unsigned long addr, void *data) > -{ > - uint64_t **frames = (uint64_t **)data; > + addr = (unsigned long)shared; > + > + for (i = 0; i < nr_gframes; i++) { > + set_pte_at(&init_mm, addr, gnttab_shared_vm_area.ptes[i], > + mfn_pte(frames[i], PAGE_KERNEL)); > + addr += PAGE_SIZE; > + } > > - set_pte_at(&init_mm, addr, pte, mfn_pte((*frames)[0], PAGE_KERNEL)); > - (*frames)++; > return 0; > } > > -static int unmap_pte_fn(pte_t *pte, struct page *pmd_page, > - unsigned long addr, void *data) > +int arch_gnttab_map_status(uint64_t *frames, unsigned long nr_gframes, > + unsigned long max_nr_gframes, > + grant_status_t **__shared) > { > + grant_status_t *shared = *__shared; > + unsigned long addr; > + unsigned long i; > + > + if (shared == NULL) > + *__shared = shared = gnttab_status_vm_area.area->addr; > + > + addr = (unsigned long)shared; > + > + for (i = 0; i < nr_gframes; i++) { > + set_pte_at(&init_mm, addr, gnttab_status_vm_area.ptes[i], > + mfn_pte(frames[i], PAGE_KERNEL)); > + addr += PAGE_SIZE; > + } > > - set_pte_at(&init_mm, addr, pte, __pte(0)); > return 0; > } > > -int arch_gnttab_map_shared(unsigned long *frames, unsigned long nr_gframes, > - unsigned long max_nr_gframes, > - void **__shared) > +void arch_gnttab_unmap(void *shared, unsigned long nr_gframes) > { > - int rc; > - void *shared = *__shared; > + pte_t **ptes; > + unsigned long addr; > + unsigned long i; > > - if (shared == NULL) { > - struct vm_struct *area = > - alloc_vm_area(PAGE_SIZE * max_nr_gframes, NULL); > - BUG_ON(area == NULL); > - shared = area->addr; > - *__shared = shared; > - } > + if (shared == gnttab_status_vm_area.area->addr) > + ptes = gnttab_status_vm_area.ptes; > + else > + ptes = gnttab_shared_vm_area.ptes; > > - rc = apply_to_page_range(&init_mm, (unsigned long)shared, > - PAGE_SIZE * nr_gframes, > - map_pte_fn, &frames); > - return rc; > + addr = (unsigned long)shared; > + > + for (i = 0; i < nr_gframes; i++) { > + set_pte_at(&init_mm, addr, ptes[i], __pte(0)); > + addr += PAGE_SIZE; > + } > } > > -int arch_gnttab_map_status(uint64_t *frames, unsigned long nr_gframes, > - unsigned long max_nr_gframes, > - grant_status_t **__shared) > +static int __init arch_gnttab_valloc(struct gnttab_vm_area *area, > + unsigned nr_frames) > { > - int rc; > - grant_status_t *shared = *__shared; > + area->ptes = kmalloc(sizeof(pte_t *) * nr_frames, GFP_KERNEL); > + if (area->ptes == NULL) > + return -ENOMEM; > > - if (shared == NULL) { > - /* No need to pass in PTE as we are going to do it > - * in apply_to_page_range anyhow. */ > - struct vm_struct *area = > - alloc_vm_area(PAGE_SIZE * max_nr_gframes, NULL); > - BUG_ON(area == NULL); > - shared = area->addr; > - *__shared = shared; > + area->area = alloc_vm_area(PAGE_SIZE * nr_frames, area->ptes); > + if (area->area == NULL) { > + kfree(area->ptes); > + return -ENOMEM; > } > > - rc = apply_to_page_range(&init_mm, (unsigned long)shared, > - PAGE_SIZE * nr_gframes, > - map_pte_fn_status, &frames); > - return rc; > + return 0; > } > > -void arch_gnttab_unmap(void *shared, unsigned long nr_gframes) > +static void __init arch_gnttab_vfree(struct gnttab_vm_area *area) > { > - apply_to_page_range(&init_mm, (unsigned long)shared, > - PAGE_SIZE * nr_gframes, unmap_pte_fn, NULL); > + free_vm_area(area->area); > + kfree(area->ptes); > } > + > +int __init arch_gnttab_init(unsigned long nr_shared, unsigned long nr_status) > +{ > + int ret; > + > + if (!xen_pv_domain()) > + return 0; > + > + ret = arch_gnttab_valloc(&gnttab_shared_vm_area, nr_shared); > + if (ret < 0) > + return ret; > + > + /* > + * Always allocate the space for the status frames in case > + * we're migrated to a host with V2 support. > + */ > + ret = arch_gnttab_valloc(&gnttab_status_vm_area, nr_status); > + if (ret < 0) > + goto err; > + > + return 0; > + err: > + arch_gnttab_vfree(&gnttab_shared_vm_area); > + return -ENOMEM; > +} > + > #ifdef CONFIG_XEN_PVH > #include <xen/balloon.h> > #include <xen/events.h> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c > index 5d4de88..eeba754 100644 > --- a/drivers/xen/grant-table.c > +++ b/drivers/xen/grant-table.c > @@ -1195,18 +1195,20 @@ static int gnttab_expand(unsigned int req_entries) > int gnttab_init(void) > { > int i; > + unsigned long max_nr_grant_frames; > unsigned int max_nr_glist_frames, nr_glist_frames; > unsigned int nr_init_grefs; > int ret; > > gnttab_request_version(); > + max_nr_grant_frames = gnttab_max_grant_frames(); > nr_grant_frames = 1; > > /* Determine the maximum number of frames required for the > * grant reference free list on the current hypervisor. > */ > BUG_ON(grefs_per_grant_frame == 0); > - max_nr_glist_frames = (gnttab_max_grant_frames() * > + max_nr_glist_frames = (max_nr_grant_frames * > grefs_per_grant_frame / RPP); > > gnttab_list = kmalloc(max_nr_glist_frames * sizeof(grant_ref_t *), > @@ -1223,6 +1225,11 @@ int gnttab_init(void) > } > } > > + ret = arch_gnttab_init(max_nr_grant_frames, > + nr_status_frames(max_nr_grant_frames)); > + if (ret < 0) > + goto ini_nomem; > + > if (gnttab_setup() < 0) { > ret = -ENODEV; > goto ini_nomem; > diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h > index a5af2a2..5c1aba1 100644 > --- a/include/xen/grant_table.h > +++ b/include/xen/grant_table.h > @@ -170,6 +170,7 @@ gnttab_set_unmap_op(struct gnttab_unmap_grant_ref *unmap, > phys_addr_t addr, > unmap->dev_bus_addr = 0; > } > > +int arch_gnttab_init(unsigned long nr_shared, unsigned long nr_status); > int arch_gnttab_map_shared(xen_pfn_t *frames, unsigned long nr_gframes, > unsigned long max_nr_gframes, > void **__shared); > -- > 1.7.10.4 > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |