| Message ID | 20190305133057.3998926-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] xen: avoid link error on ARM | expand |
On 05/03/2019 14:30, Arnd Bergmann wrote: > Building the privcmd code as a loadable module on ARM, we get > a link error due to the private cache management functions: > > ERROR: "__sync_icache_dcache" [drivers/xen/xen-privcmd.ko] undefined! > > Move the code into a new file that is always built in when Xen > is enabled, as suggested by Juergen Gross. Additional code will > be moved into this file later. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > v2: rename mm.o to xen-builtin.o, make it unconditional > --- > drivers/xen/Makefile | 1 + > drivers/xen/privcmd.c | 30 +--------------------------- > drivers/xen/xen-builtin.c | 41 +++++++++++++++++++++++++++++++++++++++ > include/xen/xen-ops.h | 3 +++ > 4 files changed, 46 insertions(+), 29 deletions(-) > create mode 100644 drivers/xen/xen-builtin.c > > diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile > index ad3844d9f876..c3cbfcf30d38 100644 > --- a/drivers/xen/Makefile > +++ b/drivers/xen/Makefile > @@ -2,6 +2,7 @@ > obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o > obj-y += grant-table.o features.o balloon.o manage.o preempt.o time.o > obj-y += mem-reservation.o > +obj-y += xen-builtin.o > obj-y += events/ > obj-y += xenbus/ > > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c > index b24ddac1604b..290b6aca7e1d 100644 > --- a/drivers/xen/privcmd.c > +++ b/drivers/xen/privcmd.c > @@ -723,26 +723,6 @@ static long privcmd_ioctl_restrict(struct file *file, void __user *udata) > return 0; > } > > -struct remap_pfn { > - struct mm_struct *mm; > - struct page **pages; > - pgprot_t prot; > - unsigned long i; > -}; > - > -static int remap_pfn_fn(pte_t *ptep, pgtable_t token, unsigned long addr, > - void *data) > -{ > - struct remap_pfn *r = data; > - struct page *page = r->pages[r->i]; > - pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), r->prot)); > - > - set_pte_at(r->mm, addr, ptep, pte); > - r->i++; > - > - return 0; > -} > - > static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) > { > struct privcmd_data *data = file->private_data; > @@ -809,15 +789,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) > goto out; > > if (xen_feature(XENFEAT_auto_translated_physmap)) { > - struct remap_pfn r = { > - .mm = vma->vm_mm, > - .pages = vma->vm_private_data, > - .prot = vma->vm_page_prot, > - }; > - > - rc = apply_to_page_range(r.mm, kdata.addr, > - kdata.num << PAGE_SHIFT, > - remap_pfn_fn, &r); > + rc = xen_remap_vma_range(vma, kdata.addr, kdata.num << PAGE_SHIFT); > } else { > unsigned int domid = > (xdata.flags & XENMEM_rsrc_acq_caller_owned) ? > diff --git a/drivers/xen/xen-builtin.c b/drivers/xen/xen-builtin.c > new file mode 100644 > index 000000000000..8ad0d4900588 > --- /dev/null > +++ b/drivers/xen/xen-builtin.c > @@ -0,0 +1,41 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Architecture independent helper functions for memory management > + * > + * Written by Paul Durrant <paul.durrant@citrix.com> > + */ > +#include <linux/mm.h> > +#include <linux/export.h> Shouldn't you #include xen/xen-ops.h here as well? Juergen
On 3/5/19 8:30 AM, Arnd Bergmann wrote: > > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c > index b24ddac1604b..290b6aca7e1d 100644 > --- a/drivers/xen/privcmd.c > +++ b/drivers/xen/privcmd.c > @@ -723,26 +723,6 @@ static long privcmd_ioctl_restrict(struct file *file, void __user *udata) > return 0; > } > > -struct remap_pfn { > - struct mm_struct *mm; > - struct page **pages; > - pgprot_t prot; > - unsigned long i; > -}; > - > -static int remap_pfn_fn(pte_t *ptep, pgtable_t token, unsigned long addr, > - void *data) > -{ > - struct remap_pfn *r = data; > - struct page *page = r->pages[r->i]; > - pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), r->prot)); > - > - set_pte_at(r->mm, addr, ptep, pte); > - r->i++; > - > - return 0; > -} > - > static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) > { > struct privcmd_data *data = file->private_data; > @@ -809,15 +789,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) > goto out; > > if (xen_feature(XENFEAT_auto_translated_physmap)) { > - struct remap_pfn r = { > - .mm = vma->vm_mm, > - .pages = vma->vm_private_data, > - .prot = vma->vm_page_prot, > - }; > - > - rc = apply_to_page_range(r.mm, kdata.addr, > - kdata.num << PAGE_SHIFT, > - remap_pfn_fn, &r); > + rc = xen_remap_vma_range(vma, kdata.addr, kdata.num << PAGE_SHIFT); I wonder whether drivers/xen/xlate_mmu.c might be a good place for these routines. -boris
On 05/03/2019 14:43, Boris Ostrovsky wrote: > On 3/5/19 8:30 AM, Arnd Bergmann wrote: >> >> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c >> index b24ddac1604b..290b6aca7e1d 100644 >> --- a/drivers/xen/privcmd.c >> +++ b/drivers/xen/privcmd.c >> @@ -723,26 +723,6 @@ static long privcmd_ioctl_restrict(struct file *file, void __user *udata) >> return 0; >> } >> >> -struct remap_pfn { >> - struct mm_struct *mm; >> - struct page **pages; >> - pgprot_t prot; >> - unsigned long i; >> -}; >> - >> -static int remap_pfn_fn(pte_t *ptep, pgtable_t token, unsigned long addr, >> - void *data) >> -{ >> - struct remap_pfn *r = data; >> - struct page *page = r->pages[r->i]; >> - pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), r->prot)); >> - >> - set_pte_at(r->mm, addr, ptep, pte); >> - r->i++; >> - >> - return 0; >> -} >> - >> static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) >> { >> struct privcmd_data *data = file->private_data; >> @@ -809,15 +789,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) >> goto out; >> >> if (xen_feature(XENFEAT_auto_translated_physmap)) { >> - struct remap_pfn r = { >> - .mm = vma->vm_mm, >> - .pages = vma->vm_private_data, >> - .prot = vma->vm_page_prot, >> - }; >> - >> - rc = apply_to_page_range(r.mm, kdata.addr, >> - kdata.num << PAGE_SHIFT, >> - remap_pfn_fn, &r); >> + rc = xen_remap_vma_range(vma, kdata.addr, kdata.num << PAGE_SHIFT); > > I wonder whether drivers/xen/xlate_mmu.c might be a good place for these > routines. Hmm, probably. This would require a stub in the header to avoid problems in case of CONFIG_XEN_AUTO_XLATE not defined, though (the #ifdef is already there). I think this is the cleanest solution. Juergen
On Tue, Mar 5, 2019 at 3:57 PM Juergen Gross <jgross@suse.com> wrote: > On 05/03/2019 14:43, Boris Ostrovsky wrote: > > On 3/5/19 8:30 AM, Arnd Bergmann wrote: > >> @@ -809,15 +789,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) > >> goto out; > >> > >> if (xen_feature(XENFEAT_auto_translated_physmap)) { > >> - struct remap_pfn r = { > >> - .mm = vma->vm_mm, > >> - .pages = vma->vm_private_data, > >> - .prot = vma->vm_page_prot, > >> - }; > >> - > >> - rc = apply_to_page_range(r.mm, kdata.addr, > >> - kdata.num << PAGE_SHIFT, > >> - remap_pfn_fn, &r); > >> + rc = xen_remap_vma_range(vma, kdata.addr, kdata.num << PAGE_SHIFT); > > > > I wonder whether drivers/xen/xlate_mmu.c might be a good place for these > > routines. > > Hmm, probably. This would require a stub in the header to avoid > problems in case of CONFIG_XEN_AUTO_XLATE not defined, though > (the #ifdef is already there). > > I think this is the cleanest solution. Putting it into xlate_mmu.c was my first attempt, but I was not sure how to solve the dependency on CONFIG_XEN_AUTO_XLATE. So xen_feature(XENFEAT_auto_translated_physmap)) would be guaranteed to return false if CONFIG_XEN_AUTO_XLATE is disabled? Arnd
On 05/03/2019 17:49, Arnd Bergmann wrote: > On Tue, Mar 5, 2019 at 3:57 PM Juergen Gross <jgross@suse.com> wrote: >> On 05/03/2019 14:43, Boris Ostrovsky wrote: >>> On 3/5/19 8:30 AM, Arnd Bergmann wrote: > >>>> @@ -809,15 +789,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) >>>> goto out; >>>> >>>> if (xen_feature(XENFEAT_auto_translated_physmap)) { >>>> - struct remap_pfn r = { >>>> - .mm = vma->vm_mm, >>>> - .pages = vma->vm_private_data, >>>> - .prot = vma->vm_page_prot, >>>> - }; >>>> - >>>> - rc = apply_to_page_range(r.mm, kdata.addr, >>>> - kdata.num << PAGE_SHIFT, >>>> - remap_pfn_fn, &r); >>>> + rc = xen_remap_vma_range(vma, kdata.addr, kdata.num << PAGE_SHIFT); >>> >>> I wonder whether drivers/xen/xlate_mmu.c might be a good place for these >>> routines. >> >> Hmm, probably. This would require a stub in the header to avoid >> problems in case of CONFIG_XEN_AUTO_XLATE not defined, though >> (the #ifdef is already there). >> >> I think this is the cleanest solution. > > Putting it into xlate_mmu.c was my first attempt, but I was not sure how to > solve the dependency on CONFIG_XEN_AUTO_XLATE. So > xen_feature(XENFEAT_auto_translated_physmap)) would be guaranteed > to return false if CONFIG_XEN_AUTO_XLATE is disabled? Yes. Juergen
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index ad3844d9f876..c3cbfcf30d38 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o obj-y += grant-table.o features.o balloon.o manage.o preempt.o time.o obj-y += mem-reservation.o +obj-y += xen-builtin.o obj-y += events/ obj-y += xenbus/ diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index b24ddac1604b..290b6aca7e1d 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -723,26 +723,6 @@ static long privcmd_ioctl_restrict(struct file *file, void __user *udata) return 0; } -struct remap_pfn { - struct mm_struct *mm; - struct page **pages; - pgprot_t prot; - unsigned long i; -}; - -static int remap_pfn_fn(pte_t *ptep, pgtable_t token, unsigned long addr, - void *data) -{ - struct remap_pfn *r = data; - struct page *page = r->pages[r->i]; - pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), r->prot)); - - set_pte_at(r->mm, addr, ptep, pte); - r->i++; - - return 0; -} - static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) { struct privcmd_data *data = file->private_data; @@ -809,15 +789,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata) goto out; if (xen_feature(XENFEAT_auto_translated_physmap)) { - struct remap_pfn r = { - .mm = vma->vm_mm, - .pages = vma->vm_private_data, - .prot = vma->vm_page_prot, - }; - - rc = apply_to_page_range(r.mm, kdata.addr, - kdata.num << PAGE_SHIFT, - remap_pfn_fn, &r); + rc = xen_remap_vma_range(vma, kdata.addr, kdata.num << PAGE_SHIFT); } else { unsigned int domid = (xdata.flags & XENMEM_rsrc_acq_caller_owned) ? diff --git a/drivers/xen/xen-builtin.c b/drivers/xen/xen-builtin.c new file mode 100644 index 000000000000..8ad0d4900588 --- /dev/null +++ b/drivers/xen/xen-builtin.c @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Architecture independent helper functions for memory management + * + * Written by Paul Durrant <paul.durrant@citrix.com> + */ +#include <linux/mm.h> +#include <linux/export.h> + +struct remap_pfn { + struct mm_struct *mm; + struct page **pages; + pgprot_t prot; + unsigned long i; +}; + +static int remap_pfn_fn(pte_t *ptep, pgtable_t token, unsigned long addr, + void *data) +{ + struct remap_pfn *r = data; + struct page *page = r->pages[r->i]; + pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), r->prot)); + + set_pte_at(r->mm, addr, ptep, pte); + r->i++; + + return 0; +} + +/* Used by the privcmd module, but has to be built-in on ARM */ +int xen_remap_vma_range(struct vm_area_struct *vma, unsigned long addr, unsigned long len) +{ + struct remap_pfn r = { + .mm = vma->vm_mm, + .pages = vma->vm_private_data, + .prot = vma->vm_page_prot, + }; + + return apply_to_page_range(vma->vm_mm, addr, len, remap_pfn_fn, &r); +} +EXPORT_SYMBOL_GPL(xen_remap_vma_range); diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h index 4969817124a8..98b30c1613b2 100644 --- a/include/xen/xen-ops.h +++ b/include/xen/xen-ops.h @@ -109,6 +109,9 @@ static inline int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma, } #endif +int xen_remap_vma_range(struct vm_area_struct *vma, unsigned long addr, + unsigned long len); + /* * xen_remap_domain_gfn_array() - map an array of foreign frames by gfn * @vma: VMA to map the pages into
Building the privcmd code as a loadable module on ARM, we get a link error due to the private cache management functions: ERROR: "__sync_icache_dcache" [drivers/xen/xen-privcmd.ko] undefined! Move the code into a new file that is always built in when Xen is enabled, as suggested by Juergen Gross. Additional code will be moved into this file later. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- v2: rename mm.o to xen-builtin.o, make it unconditional --- drivers/xen/Makefile | 1 + drivers/xen/privcmd.c | 30 +--------------------------- drivers/xen/xen-builtin.c | 41 +++++++++++++++++++++++++++++++++++++++ include/xen/xen-ops.h | 3 +++ 4 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 drivers/xen/xen-builtin.c -- 2.20.0