All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xen: avoid link error on ARM
@ 2019-03-04 20:47 Arnd Bergmann
  2019-03-04 21:19 ` Boris Ostrovsky
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-04 20:47 UTC (permalink / raw)
  To: Boris Ostrovsky, Juergen Gross
  Cc: Arnd Bergmann, Stefano Stabellini, Oleksandr Andrushchenko,
	Matthew Wilcox, Paul Durrant, Souptick Joarder, linux-kernel,
	xen-devel

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 built along with privcmd.o
but is always built-in, even when the latter is a loadable module.

xen_remap_vma_range() may not be the best name here, if someone
comes up with a better one, let me know.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/xen/Makefile  |  3 +++
 drivers/xen/mm.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 drivers/xen/privcmd.c | 30 +-----------------------------
 include/xen/xen-ops.h |  3 +++
 4 files changed, 48 insertions(+), 29 deletions(-)
 create mode 100644 drivers/xen/mm.c

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index ad3844d9f876..7124f9e749b4 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -29,6 +29,9 @@ obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
 obj-$(CONFIG_XEN_MCE_LOG)		+= mcelog.o
 obj-$(CONFIG_XEN_PCIDEV_BACKEND)	+= xen-pciback/
 obj-$(CONFIG_XEN_PRIVCMD)		+= xen-privcmd.o
+ifdef CONFIG_XEN_PRIVCMD
+obj-y					+= mm.o
+endif
 obj-$(CONFIG_XEN_STUB)			+= xen-stub.o
 obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY)	+= xen-acpi-memhotplug.o
 obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU)	+= xen-acpi-cpuhotplug.o
diff --git a/drivers/xen/mm.c b/drivers/xen/mm.c
new file mode 100644
index 000000000000..8ad0d4900588
--- /dev/null
+++ b/drivers/xen/mm.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/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/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
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-04 20:47 [PATCH] xen: avoid link error on ARM Arnd Bergmann
  2019-03-04 21:19 ` Boris Ostrovsky
@ 2019-03-04 21:19 ` Boris Ostrovsky
  2019-03-04 21:23   ` Arnd Bergmann
  2019-03-04 21:23   ` Arnd Bergmann
  2019-03-05  6:39 ` Juergen Gross
  2019-03-05  6:39 ` Juergen Gross
  3 siblings, 2 replies; 14+ messages in thread
From: Boris Ostrovsky @ 2019-03-04 21:19 UTC (permalink / raw)
  To: Arnd Bergmann, Juergen Gross
  Cc: Stefano Stabellini, Oleksandr Andrushchenko, Matthew Wilcox,
	Paul Durrant, Souptick Joarder, linux-kernel, xen-devel

On 3/4/19 3:47 PM, 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!

Can __sync_icache_dcache be exported in arm32, just like it is in arm64?

-boris



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-04 20:47 [PATCH] xen: avoid link error on ARM Arnd Bergmann
@ 2019-03-04 21:19 ` Boris Ostrovsky
  2019-03-04 21:19 ` Boris Ostrovsky
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Boris Ostrovsky @ 2019-03-04 21:19 UTC (permalink / raw)
  To: Arnd Bergmann, Juergen Gross
  Cc: Stefano Stabellini, Oleksandr Andrushchenko, linux-kernel,
	Matthew Wilcox, Paul Durrant, Souptick Joarder, xen-devel

On 3/4/19 3:47 PM, 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!

Can __sync_icache_dcache be exported in arm32, just like it is in arm64?

-boris



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-04 21:19 ` Boris Ostrovsky
@ 2019-03-04 21:23   ` Arnd Bergmann
  2019-03-04 21:23   ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-04 21:23 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Juergen Gross, Stefano Stabellini, Oleksandr Andrushchenko,
	Matthew Wilcox, Paul Durrant, Souptick Joarder,
	Linux Kernel Mailing List, xen-devel

On Mon, Mar 4, 2019 at 10:19 PM Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
>
> On 3/4/19 3:47 PM, 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!
>
> Can __sync_icache_dcache be exported in arm32, just like it is in arm64?

Russell wants all cache management operations to stay private to the
kernel, as they tend to get abused by other drivers:

https://lore.kernel.org/linux-arm-kernel/20181218100908.GL26090@n2100.armlinux.org.uk/

      Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-04 21:19 ` Boris Ostrovsky
  2019-03-04 21:23   ` Arnd Bergmann
@ 2019-03-04 21:23   ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-04 21:23 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Juergen Gross, Stefano Stabellini, Oleksandr Andrushchenko,
	Linux Kernel Mailing List, Matthew Wilcox, Paul Durrant,
	Souptick Joarder, xen-devel

On Mon, Mar 4, 2019 at 10:19 PM Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
>
> On 3/4/19 3:47 PM, 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!
>
> Can __sync_icache_dcache be exported in arm32, just like it is in arm64?

Russell wants all cache management operations to stay private to the
kernel, as they tend to get abused by other drivers:

https://lore.kernel.org/linux-arm-kernel/20181218100908.GL26090@n2100.armlinux.org.uk/

      Arnd

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-04 20:47 [PATCH] xen: avoid link error on ARM Arnd Bergmann
                   ` (2 preceding siblings ...)
  2019-03-05  6:39 ` Juergen Gross
@ 2019-03-05  6:39 ` Juergen Gross
  2019-03-05  8:34   ` Arnd Bergmann
  2019-03-05  8:34   ` Arnd Bergmann
  3 siblings, 2 replies; 14+ messages in thread
From: Juergen Gross @ 2019-03-05  6:39 UTC (permalink / raw)
  To: Arnd Bergmann, Boris Ostrovsky
  Cc: Stefano Stabellini, Oleksandr Andrushchenko, Matthew Wilcox,
	Paul Durrant, Souptick Joarder, linux-kernel, xen-devel

On 04/03/2019 21:47, 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 built along with privcmd.o
> but is always built-in, even when the latter is a loadable module.
> 
> xen_remap_vma_range() may not be the best name here, if someone
> comes up with a better one, let me know.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/xen/Makefile  |  3 +++
>  drivers/xen/mm.c      | 41 +++++++++++++++++++++++++++++++++++++++++
>  drivers/xen/privcmd.c | 30 +-----------------------------
>  include/xen/xen-ops.h |  3 +++
>  4 files changed, 48 insertions(+), 29 deletions(-)
>  create mode 100644 drivers/xen/mm.c
> 
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index ad3844d9f876..7124f9e749b4 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -29,6 +29,9 @@ obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
>  obj-$(CONFIG_XEN_MCE_LOG)		+= mcelog.o
>  obj-$(CONFIG_XEN_PCIDEV_BACKEND)	+= xen-pciback/
>  obj-$(CONFIG_XEN_PRIVCMD)		+= xen-privcmd.o
> +ifdef CONFIG_XEN_PRIVCMD
> +obj-y					+= mm.o
> +endif

Can we avoid that ifdef in the Makefile?

I'd rather have an architecture independant builtin driver added which
is always included for CONFIG_XEN. This would allow to move redundant
stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).

So: rename mm.c to xen-builtin.c, use:

obj-$(CONFIG_XEN) += xen-builtin.o


Juergen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-04 20:47 [PATCH] xen: avoid link error on ARM Arnd Bergmann
  2019-03-04 21:19 ` Boris Ostrovsky
  2019-03-04 21:19 ` Boris Ostrovsky
@ 2019-03-05  6:39 ` Juergen Gross
  2019-03-05  6:39 ` Juergen Gross
  3 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2019-03-05  6:39 UTC (permalink / raw)
  To: Arnd Bergmann, Boris Ostrovsky
  Cc: Stefano Stabellini, Oleksandr Andrushchenko, linux-kernel,
	Matthew Wilcox, Paul Durrant, Souptick Joarder, xen-devel

On 04/03/2019 21:47, 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 built along with privcmd.o
> but is always built-in, even when the latter is a loadable module.
> 
> xen_remap_vma_range() may not be the best name here, if someone
> comes up with a better one, let me know.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/xen/Makefile  |  3 +++
>  drivers/xen/mm.c      | 41 +++++++++++++++++++++++++++++++++++++++++
>  drivers/xen/privcmd.c | 30 +-----------------------------
>  include/xen/xen-ops.h |  3 +++
>  4 files changed, 48 insertions(+), 29 deletions(-)
>  create mode 100644 drivers/xen/mm.c
> 
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index ad3844d9f876..7124f9e749b4 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -29,6 +29,9 @@ obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
>  obj-$(CONFIG_XEN_MCE_LOG)		+= mcelog.o
>  obj-$(CONFIG_XEN_PCIDEV_BACKEND)	+= xen-pciback/
>  obj-$(CONFIG_XEN_PRIVCMD)		+= xen-privcmd.o
> +ifdef CONFIG_XEN_PRIVCMD
> +obj-y					+= mm.o
> +endif

Can we avoid that ifdef in the Makefile?

I'd rather have an architecture independant builtin driver added which
is always included for CONFIG_XEN. This would allow to move redundant
stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).

So: rename mm.c to xen-builtin.c, use:

obj-$(CONFIG_XEN) += xen-builtin.o


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-05  6:39 ` Juergen Gross
  2019-03-05  8:34   ` Arnd Bergmann
@ 2019-03-05  8:34   ` Arnd Bergmann
  2019-03-05  9:05     ` Juergen Gross
  2019-03-05  9:05     ` Juergen Gross
  1 sibling, 2 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-05  8:34 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Boris Ostrovsky, Stefano Stabellini, Oleksandr Andrushchenko,
	Matthew Wilcox, Paul Durrant, Souptick Joarder,
	Linux Kernel Mailing List, xen-devel

On Tue, Mar 5, 2019 at 7:39 AM Juergen Gross <jgross@suse.com> wrote:

>
> Can we avoid that ifdef in the Makefile?
>
> I'd rather have an architecture independant builtin driver added which
> is always included for CONFIG_XEN. This would allow to move redundant
> stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).
>
> So: rename mm.c to xen-builtin.c, use:
>
> obj-$(CONFIG_XEN) += xen-builtin.o

Sure, I'm happy to change the naming and the Makefile logic. The way you
suggested sounds fine to me, but it will make the xen code slightly bigger
even if that code is not used. We could also have a silent Kconfig symbol
that turns this on and still avoid the ifdef:

obj-$(CONFIG_XEN_BUILTIN) += xen-builtin.o

(or using any other symbol name that makes more sense than that.

Let me know your preference and I'll resubmit.

      Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-05  6:39 ` Juergen Gross
@ 2019-03-05  8:34   ` Arnd Bergmann
  2019-03-05  8:34   ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-05  8:34 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Stefano Stabellini, Oleksandr Andrushchenko,
	Linux Kernel Mailing List, Matthew Wilcox, Paul Durrant,
	Souptick Joarder, xen-devel, Boris Ostrovsky

On Tue, Mar 5, 2019 at 7:39 AM Juergen Gross <jgross@suse.com> wrote:

>
> Can we avoid that ifdef in the Makefile?
>
> I'd rather have an architecture independant builtin driver added which
> is always included for CONFIG_XEN. This would allow to move redundant
> stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).
>
> So: rename mm.c to xen-builtin.c, use:
>
> obj-$(CONFIG_XEN) += xen-builtin.o

Sure, I'm happy to change the naming and the Makefile logic. The way you
suggested sounds fine to me, but it will make the xen code slightly bigger
even if that code is not used. We could also have a silent Kconfig symbol
that turns this on and still avoid the ifdef:

obj-$(CONFIG_XEN_BUILTIN) += xen-builtin.o

(or using any other symbol name that makes more sense than that.

Let me know your preference and I'll resubmit.

      Arnd

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-05  8:34   ` Arnd Bergmann
  2019-03-05  9:05     ` Juergen Gross
@ 2019-03-05  9:05     ` Juergen Gross
  2019-03-05  9:19       ` Arnd Bergmann
  2019-03-05  9:19       ` Arnd Bergmann
  1 sibling, 2 replies; 14+ messages in thread
From: Juergen Gross @ 2019-03-05  9:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Boris Ostrovsky, Stefano Stabellini, Oleksandr Andrushchenko,
	Matthew Wilcox, Paul Durrant, Souptick Joarder,
	Linux Kernel Mailing List, xen-devel

On 05/03/2019 09:34, Arnd Bergmann wrote:
> On Tue, Mar 5, 2019 at 7:39 AM Juergen Gross <jgross@suse.com> wrote:
> 
>>
>> Can we avoid that ifdef in the Makefile?
>>
>> I'd rather have an architecture independant builtin driver added which
>> is always included for CONFIG_XEN. This would allow to move redundant
>> stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).
>>
>> So: rename mm.c to xen-builtin.c, use:
>>
>> obj-$(CONFIG_XEN) += xen-builtin.o
> 
> Sure, I'm happy to change the naming and the Makefile logic. The way you
> suggested sounds fine to me, but it will make the xen code slightly bigger
> even if that code is not used. We could also have a silent Kconfig symbol
> that turns this on and still avoid the ifdef:
> 
> obj-$(CONFIG_XEN_BUILTIN) += xen-builtin.o

That was my first thought.

But looking through arch/[arm|x86]/xen/enlighten.c I found several
global variables defined the same way. I'd like to merge those, too.

So my preference is a common source for all this stuff.

I'll send a followup patch to move the mentioned variables into the new
source.


Juergen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-05  8:34   ` Arnd Bergmann
@ 2019-03-05  9:05     ` Juergen Gross
  2019-03-05  9:05     ` Juergen Gross
  1 sibling, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2019-03-05  9:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stefano Stabellini, Oleksandr Andrushchenko,
	Linux Kernel Mailing List, Matthew Wilcox, Paul Durrant,
	Souptick Joarder, xen-devel, Boris Ostrovsky

On 05/03/2019 09:34, Arnd Bergmann wrote:
> On Tue, Mar 5, 2019 at 7:39 AM Juergen Gross <jgross@suse.com> wrote:
> 
>>
>> Can we avoid that ifdef in the Makefile?
>>
>> I'd rather have an architecture independant builtin driver added which
>> is always included for CONFIG_XEN. This would allow to move redundant
>> stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).
>>
>> So: rename mm.c to xen-builtin.c, use:
>>
>> obj-$(CONFIG_XEN) += xen-builtin.o
> 
> Sure, I'm happy to change the naming and the Makefile logic. The way you
> suggested sounds fine to me, but it will make the xen code slightly bigger
> even if that code is not used. We could also have a silent Kconfig symbol
> that turns this on and still avoid the ifdef:
> 
> obj-$(CONFIG_XEN_BUILTIN) += xen-builtin.o

That was my first thought.

But looking through arch/[arm|x86]/xen/enlighten.c I found several
global variables defined the same way. I'd like to merge those, too.

So my preference is a common source for all this stuff.

I'll send a followup patch to move the mentioned variables into the new
source.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-05  9:05     ` Juergen Gross
  2019-03-05  9:19       ` Arnd Bergmann
@ 2019-03-05  9:19       ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-05  9:19 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Boris Ostrovsky, Stefano Stabellini, Oleksandr Andrushchenko,
	Matthew Wilcox, Paul Durrant, Souptick Joarder,
	Linux Kernel Mailing List, xen-devel

On Tue, Mar 5, 2019 at 10:05 AM Juergen Gross <jgross@suse.com> wrote:
>
> On 05/03/2019 09:34, Arnd Bergmann wrote:
> > On Tue, Mar 5, 2019 at 7:39 AM Juergen Gross <jgross@suse.com> wrote:
> >
> >>
> >> Can we avoid that ifdef in the Makefile?
> >>
> >> I'd rather have an architecture independant builtin driver added which
> >> is always included for CONFIG_XEN. This would allow to move redundant
> >> stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).
> >>
> >> So: rename mm.c to xen-builtin.c, use:
> >>
> >> obj-$(CONFIG_XEN) += xen-builtin.o
> >
> > Sure, I'm happy to change the naming and the Makefile logic. The way you
> > suggested sounds fine to me, but it will make the xen code slightly bigger
> > even if that code is not used. We could also have a silent Kconfig symbol
> > that turns this on and still avoid the ifdef:
> >
> > obj-$(CONFIG_XEN_BUILTIN) += xen-builtin.o
>
> That was my first thought.
>
> But looking through arch/[arm|x86]/xen/enlighten.c I found several
> global variables defined the same way. I'd like to merge those, too.
>
> So my preference is a common source for all this stuff.

Ok, makes sense. I've prepared that patch now and will send it
after some more build testing.

        Arnd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xen: avoid link error on ARM
  2019-03-05  9:05     ` Juergen Gross
@ 2019-03-05  9:19       ` Arnd Bergmann
  2019-03-05  9:19       ` Arnd Bergmann
  1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-05  9:19 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Stefano Stabellini, Oleksandr Andrushchenko,
	Linux Kernel Mailing List, Matthew Wilcox, Paul Durrant,
	Souptick Joarder, xen-devel, Boris Ostrovsky

On Tue, Mar 5, 2019 at 10:05 AM Juergen Gross <jgross@suse.com> wrote:
>
> On 05/03/2019 09:34, Arnd Bergmann wrote:
> > On Tue, Mar 5, 2019 at 7:39 AM Juergen Gross <jgross@suse.com> wrote:
> >
> >>
> >> Can we avoid that ifdef in the Makefile?
> >>
> >> I'd rather have an architecture independant builtin driver added which
> >> is always included for CONFIG_XEN. This would allow to move redundant
> >> stuff from arch/*/xen/ into it (e.g. xen_vcpu_id).
> >>
> >> So: rename mm.c to xen-builtin.c, use:
> >>
> >> obj-$(CONFIG_XEN) += xen-builtin.o
> >
> > Sure, I'm happy to change the naming and the Makefile logic. The way you
> > suggested sounds fine to me, but it will make the xen code slightly bigger
> > even if that code is not used. We could also have a silent Kconfig symbol
> > that turns this on and still avoid the ifdef:
> >
> > obj-$(CONFIG_XEN_BUILTIN) += xen-builtin.o
>
> That was my first thought.
>
> But looking through arch/[arm|x86]/xen/enlighten.c I found several
> global variables defined the same way. I'd like to merge those, too.
>
> So my preference is a common source for all this stuff.

Ok, makes sense. I've prepared that patch now and will send it
after some more build testing.

        Arnd

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] xen: avoid link error on ARM
@ 2019-03-04 20:47 Arnd Bergmann
  0 siblings, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-03-04 20:47 UTC (permalink / raw)
  To: Boris Ostrovsky, Juergen Gross
  Cc: Stefano Stabellini, Arnd Bergmann, Oleksandr Andrushchenko,
	linux-kernel, Matthew Wilcox, Paul Durrant, Souptick Joarder,
	xen-devel

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 built along with privcmd.o
but is always built-in, even when the latter is a loadable module.

xen_remap_vma_range() may not be the best name here, if someone
comes up with a better one, let me know.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/xen/Makefile  |  3 +++
 drivers/xen/mm.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 drivers/xen/privcmd.c | 30 +-----------------------------
 include/xen/xen-ops.h |  3 +++
 4 files changed, 48 insertions(+), 29 deletions(-)
 create mode 100644 drivers/xen/mm.c

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index ad3844d9f876..7124f9e749b4 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -29,6 +29,9 @@ obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
 obj-$(CONFIG_XEN_MCE_LOG)		+= mcelog.o
 obj-$(CONFIG_XEN_PCIDEV_BACKEND)	+= xen-pciback/
 obj-$(CONFIG_XEN_PRIVCMD)		+= xen-privcmd.o
+ifdef CONFIG_XEN_PRIVCMD
+obj-y					+= mm.o
+endif
 obj-$(CONFIG_XEN_STUB)			+= xen-stub.o
 obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY)	+= xen-acpi-memhotplug.o
 obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU)	+= xen-acpi-cpuhotplug.o
diff --git a/drivers/xen/mm.c b/drivers/xen/mm.c
new file mode 100644
index 000000000000..8ad0d4900588
--- /dev/null
+++ b/drivers/xen/mm.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/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/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
-- 
2.20.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-03-05  9:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 20:47 [PATCH] xen: avoid link error on ARM Arnd Bergmann
2019-03-04 21:19 ` Boris Ostrovsky
2019-03-04 21:19 ` Boris Ostrovsky
2019-03-04 21:23   ` Arnd Bergmann
2019-03-04 21:23   ` Arnd Bergmann
2019-03-05  6:39 ` Juergen Gross
2019-03-05  6:39 ` Juergen Gross
2019-03-05  8:34   ` Arnd Bergmann
2019-03-05  8:34   ` Arnd Bergmann
2019-03-05  9:05     ` Juergen Gross
2019-03-05  9:05     ` Juergen Gross
2019-03-05  9:19       ` Arnd Bergmann
2019-03-05  9:19       ` Arnd Bergmann
  -- strict thread matches above, loose matches on Subject: below --
2019-03-04 20:47 Arnd Bergmann

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.