2012-08-06 14:25:40

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 00/23] Introduce Xen support on ARM

Hi all,
this patch series implements Xen support for ARMv7 with virtualization
extensions. It allows a Linux guest to boot as dom0 and
as domU on Xen on ARM. PV console, disk and network frontends and
backends are all working correctly.

It has been tested on a Versatile Express Cortex A15 emulator, using the
latest Xen ARM developement branch
(git://xenbits.xen.org/people/ianc/xen-unstable.git arm-for-4.3) plus
the "ARM hypercall ABI: 64 bit ready" patch series
(http://marc.info/?l=xen-devel&m=134426267205408), and a simple ad-hoc
tool to build guest domains (marc.info/?l=xen-devel&m=134089788016546).

The patch marked with [HACK] shouldn't be applied and is part of the
series only because it is needed to create domUs.

I am also attaching to this email the dts'es that I am currently using
for dom0 and domU: vexpress-v2p-ca15-tc1.dts (that includes
vexpress-v2m-rs1-rtsm.dtsi) is the dts used for dom0 and it is passed to
Linux by Xen, while vexpress-virt.dts is the dts used for other domUs
and it is appended in binary form to the guest kernel image. I am not
sure where they are supposed to live yet, so I am just attaching them
here so that people can actually try out this series if they want to.

Comments are very welcome!


Changes in v2:
- fix up many comments and commit messages;
- remove the early_printk patches: rely on the emulated serial for now;
- remove the xen_guest_init patch: without any PV early_printk, we don't
need any early call to xen_guest_init, we can rely on core_initcall
alone;
- define an HYPERCALL macro for 5 arguments hypercall wrappers, even if
at the moment is unused;
- use ldm instead of pop in the hypercall wrappers;
- return -ENOSYS rather than -1 from the unimplemented grant_table
functions;
- remove the pvclock ifdef in the Xen headers;
- remove include linux/types.h from xen/interface/xen.h;
- replace pr_info with pr_debug in xen_guest_init;
- add a new patch to introduce xen_ulong_t and use it top replace all
the occurences of unsigned long in the public Xen interface;
- explicitely size all the pointers to 64 bit on ARM, so that the
hypercall ABI is "64 bit ready";
- clean up xenbus_init;
- make pci.o depend on CONFIG_PCI and acpi.o depend on CONFIG_ACPI;
- mark Xen guest support on ARM as EXPERIMENTAL;
- introduce GRANT_TABLE_PHYSADDR;
- remove unneeded initialization of boot_max_nr_grant_frames;
- add a new patch to clear IRQ_NOAUTOEN and IRQ_NOREQUEST in events.c;
- return -EINVAL from xen_remap_domain_mfn_range if
auto_translated_physmap;
- retain binary compatibility in xen_add_to_physmap: use a union to
introduce foreign_domid.



Ian Campbell (1):
[HACK] xen/arm: implement xen_remap_domain_mfn_range

Stefano Stabellini (22):
arm: initial Xen support
xen/arm: hypercalls
xen/arm: page.h definitions
xen/arm: sync_bitops
xen/arm: empty implementation of grant_table arch specific functions
xen: missing includes
xen/arm: Xen detection and shared_info page mapping
xen/arm: Introduce xen_pfn_t for pfn and mfn types
xen/arm: Introduce xen_ulong_t for unsigned long
xen/arm: compile and run xenbus
xen: do not compile manage, balloon, pci, acpi and cpu_hotplug on ARM
xen/arm: introduce CONFIG_XEN on ARM
xen/arm: get privilege status
xen/arm: initialize grant_table on ARM
xen/arm: receive Xen events on ARM
xen: clear IRQ_NOAUTOEN and IRQ_NOREQUEST
xen/arm: implement alloc/free_xenballooned_pages with alloc_pages/kfree
xen: allow privcmd for HVM guests
xen/arm: compile blkfront and blkback
xen/arm: compile netback
xen: update xen_add_to_physmap interface
arm/v2m: initialize arch_timers even if v2m_timer is not present

arch/arm/Kconfig | 10 ++
arch/arm/Makefile | 1 +
arch/arm/include/asm/hypervisor.h | 6 +
arch/arm/include/asm/sync_bitops.h | 27 +++
arch/arm/include/asm/xen/events.h | 18 ++
arch/arm/include/asm/xen/hypercall.h | 69 ++++++++
arch/arm/include/asm/xen/hypervisor.h | 19 +++
arch/arm/include/asm/xen/interface.h | 72 +++++++++
arch/arm/include/asm/xen/page.h | 79 +++++++++
arch/arm/mach-vexpress/v2m.c | 11 +-
arch/arm/xen/Makefile | 1 +
arch/arm/xen/enlighten.c | 237 ++++++++++++++++++++++++++++
arch/arm/xen/grant-table.c | 53 ++++++
arch/arm/xen/hypercall.S | 106 +++++++++++++
arch/ia64/include/asm/xen/interface.h | 6 +-
arch/x86/include/asm/xen/interface.h | 8 +
arch/x86/xen/enlighten.c | 1 +
arch/x86/xen/irq.c | 1 +
arch/x86/xen/mmu.c | 3 +
arch/x86/xen/xen-ops.h | 1 -
drivers/block/xen-blkback/blkback.c | 1 +
drivers/net/xen-netback/netback.c | 1 +
drivers/net/xen-netfront.c | 1 +
drivers/tty/hvc/hvc_xen.c | 2 +
drivers/xen/Makefile | 11 +-
drivers/xen/events.c | 18 ++-
drivers/xen/grant-table.c | 1 +
drivers/xen/privcmd.c | 20 +--
drivers/xen/xenbus/xenbus_comms.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 62 +++++---
drivers/xen/xenbus/xenbus_probe_frontend.c | 1 +
drivers/xen/xenbus/xenbus_xs.c | 1 +
drivers/xen/xenfs/super.c | 7 +
include/xen/events.h | 2 +
include/xen/interface/features.h | 3 +
include/xen/interface/grant_table.h | 4 +-
include/xen/interface/io/protocols.h | 3 +
include/xen/interface/memory.h | 32 +++--
include/xen/interface/physdev.h | 4 +-
include/xen/interface/platform.h | 4 +-
include/xen/interface/version.h | 2 +-
include/xen/interface/xen.h | 13 +-
include/xen/privcmd.h | 3 +-
include/xen/xen.h | 2 +-
44 files changed, 857 insertions(+), 72 deletions(-)



A branch based on 3.5-rc7 is available here:

git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git 3.5-rc7-arm-2

Cheers,

Stefano


Attachments:
vexpress-virt.dts (2.53 kB)
vexpress-v2p-ca15-tc1.dts (2.79 kB)
vexpress-v2m-rs1-rtsm.dtsi (4.27 kB)
Download all attachments

2012-08-06 14:35:21

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 10/23] xen/arm: compile and run xenbus

bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
an error.

If Linux is running as an HVM domain and is running as Dom0, use
xenstored_local_init to initialize the xenstore page and event channel.

Changes in v2:

- refactor xenbus_init.

Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/xen/xenbus/xenbus_comms.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
drivers/xen/xenbus/xenbus_xs.c | 1 +
3 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
index 52fe7ad..c5aa55c 100644
--- a/drivers/xen/xenbus/xenbus_comms.c
+++ b/drivers/xen/xenbus/xenbus_comms.c
@@ -224,7 +224,7 @@ int xb_init_comms(void)
int err;
err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
0, "xenbus", &xb_waitq);
- if (err <= 0) {
+ if (err < 0) {
printk(KERN_ERR "XENBUS request irq failed %i\n", err);
return err;
}
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index b793723..a67ccc0 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
return err;
}

+enum xenstore_init {
+ UNKNOWN,
+ PV,
+ HVM,
+ LOCAL,
+};
static int __init xenbus_init(void)
{
int err = 0;
+ enum xenstore_init usage = UNKNOWN;
+ uint64_t v = 0;

if (!xen_domain())
return -ENODEV;

xenbus_ring_ops_init();

- if (xen_hvm_domain()) {
- uint64_t v = 0;
- err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
- if (err)
- goto out_error;
- xen_store_evtchn = (int)v;
- err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
- if (err)
- goto out_error;
- xen_store_mfn = (unsigned long)v;
- xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
- } else {
- xen_store_evtchn = xen_start_info->store_evtchn;
- xen_store_mfn = xen_start_info->store_mfn;
- if (xen_store_evtchn)
- xenstored_ready = 1;
- else {
+ if (xen_pv_domain())
+ usage = PV;
+ if (xen_hvm_domain())
+ usage = HVM;
+ if (xen_hvm_domain() && xen_initial_domain())
+ usage = LOCAL;
+ if (xen_pv_domain() && !xen_start_info->store_evtchn)
+ usage = LOCAL;
+ if (xen_pv_domain() && xen_start_info->store_evtchn)
+ xenstored_ready = 1;
+
+ switch (usage) {
+ case LOCAL:
err = xenstored_local_init();
if (err)
goto out_error;
- }
- xen_store_interface = mfn_to_virt(xen_store_mfn);
+ xen_store_interface = mfn_to_virt(xen_store_mfn);
+ break;
+ case PV:
+ xen_store_evtchn = xen_start_info->store_evtchn;
+ xen_store_mfn = xen_start_info->store_mfn;
+ xen_store_interface = mfn_to_virt(xen_store_mfn);
+ break;
+ case HVM:
+ err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
+ if (err)
+ goto out_error;
+ xen_store_evtchn = (int)v;
+ err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
+ if (err)
+ goto out_error;
+ xen_store_mfn = (unsigned long)v;
+ xen_store_interface =
+ ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
+ break;
+ default:
+ pr_warn("Xenstore state unknown\n");
+ break;
}

/* Initialize the interface to xenstore. */
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index d1c217b..f7feb3d 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -44,6 +44,7 @@
#include <linux/rwsem.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <asm/xen/hypervisor.h>
#include <xen/xenbus.h>
#include <xen/xen.h>
#include "xenbus_comms.h"
--
1.7.2.5

2012-08-06 14:35:28

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 09/23] xen/arm: Introduce xen_ulong_t for unsigned long

All the original Xen headers have xen_ulong_t as unsigned long type, however
when they have been imported in Linux, xen_ulong_t has been replaced with
unsigned long. That might work for x86 and ia64 but it does not for arm.
Bring back xen_ulong_t and let each architecture define xen_ulong_t as they
see fit.

Also explicitly size pointers (__DEFINE_GUEST_HANDLE) to 64 bit.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/xen/interface.h | 8 ++++++--
arch/ia64/include/asm/xen/interface.h | 1 +
arch/x86/include/asm/xen/interface.h | 1 +
include/xen/interface/memory.h | 12 ++++++------
include/xen/interface/physdev.h | 4 ++--
include/xen/interface/version.h | 2 +-
include/xen/interface/xen.h | 6 +++---
7 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
index f904dcc..1d3030c 100644
--- a/arch/arm/include/asm/xen/interface.h
+++ b/arch/arm/include/asm/xen/interface.h
@@ -9,8 +9,11 @@

#include <linux/types.h>

+#define uint64_aligned_t uint64_t __attribute__((aligned(8)))
+
#define __DEFINE_GUEST_HANDLE(name, type) \
- typedef type * __guest_handle_ ## name
+ typedef struct { union { type *p; uint64_aligned_t q; }; } \
+ __guest_handle_ ## name

#define DEFINE_GUEST_HANDLE_STRUCT(name) \
__DEFINE_GUEST_HANDLE(name, struct name)
@@ -21,13 +24,14 @@
do { \
if (sizeof(hnd) == 8) \
*(uint64_t *)&(hnd) = 0; \
- (hnd) = val; \
+ (hnd).p = val; \
} while (0)

#ifndef __ASSEMBLY__
/* Explicitly size integers that represent pfns in the interface with
* Xen so that we can have one ABI that works for 32 and 64 bit guests. */
typedef uint64_t xen_pfn_t;
+typedef uint64_t xen_ulong_t;
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
index 686464e..7c83445 100644
--- a/arch/ia64/include/asm/xen/interface.h
+++ b/arch/ia64/include/asm/xen/interface.h
@@ -71,6 +71,7 @@
* with Xen so that we could have one ABI that works for 32 and 64 bit
* guests. */
typedef unsigned long xen_pfn_t;
+typedef unsigned long xen_ulong_t;
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
index 555f94d..28fc621 100644
--- a/arch/x86/include/asm/xen/interface.h
+++ b/arch/x86/include/asm/xen/interface.h
@@ -51,6 +51,7 @@
* with Xen so that on ARM we can have one ABI that works for 32 and 64
* bit guests. */
typedef unsigned long xen_pfn_t;
+typedef unsigned long xen_ulong_t;
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index abbbff0..b5c3098 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -34,7 +34,7 @@ struct xen_memory_reservation {
GUEST_HANDLE(xen_pfn_t) extent_start;

/* Number of extents, and size/alignment of each (2^extent_order pages). */
- unsigned long nr_extents;
+ xen_ulong_t nr_extents;
unsigned int extent_order;

/*
@@ -92,7 +92,7 @@ struct xen_memory_exchange {
* command will be non-zero.
* 5. THIS FIELD MUST BE INITIALISED TO ZERO BY THE CALLER!
*/
- unsigned long nr_exchanged;
+ xen_ulong_t nr_exchanged;
};

DEFINE_GUEST_HANDLE_STRUCT(xen_memory_exchange);
@@ -148,8 +148,8 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mfn_list);
*/
#define XENMEM_machphys_mapping 12
struct xen_machphys_mapping {
- unsigned long v_start, v_end; /* Start and end virtual addresses. */
- unsigned long max_mfn; /* Maximum MFN that can be looked up. */
+ xen_ulong_t v_start, v_end; /* Start and end virtual addresses. */
+ xen_ulong_t max_mfn; /* Maximum MFN that can be looked up. */
};
DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mapping_t);

@@ -169,7 +169,7 @@ struct xen_add_to_physmap {
unsigned int space;

/* Index into source mapping space. */
- unsigned long idx;
+ xen_ulong_t idx;

/* GPFN where the source mapping page should appear. */
xen_pfn_t gpfn;
@@ -186,7 +186,7 @@ struct xen_translate_gpfn_list {
domid_t domid;

/* Length of list. */
- unsigned long nr_gpfns;
+ xen_ulong_t nr_gpfns;

/* List of GPFNs to translate. */
GUEST_HANDLE(ulong) gpfn_list;
diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h
index 9ce788d..bc3ae14 100644
--- a/include/xen/interface/physdev.h
+++ b/include/xen/interface/physdev.h
@@ -56,7 +56,7 @@ struct physdev_eoi {
#define PHYSDEVOP_pirq_eoi_gmfn_v2 28
struct physdev_pirq_eoi_gmfn {
/* IN */
- unsigned long gmfn;
+ xen_ulong_t gmfn;
};

/*
@@ -108,7 +108,7 @@ struct physdev_set_iobitmap {
#define PHYSDEVOP_apic_write 9
struct physdev_apic {
/* IN */
- unsigned long apic_physbase;
+ xen_ulong_t apic_physbase;
uint32_t reg;
/* IN or OUT */
uint32_t value;
diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
index e8b6519..30280c9 100644
--- a/include/xen/interface/version.h
+++ b/include/xen/interface/version.h
@@ -45,7 +45,7 @@ struct xen_changeset_info {

#define XENVER_platform_parameters 5
struct xen_platform_parameters {
- unsigned long virt_start;
+ xen_ulong_t virt_start;
};

#define XENVER_get_features 6
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 42834a3..ec32115 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -274,9 +274,9 @@ DEFINE_GUEST_HANDLE_STRUCT(mmu_update);
* NB. The fields are natural register size for this architecture.
*/
struct multicall_entry {
- unsigned long op;
- long result;
- unsigned long args[6];
+ xen_ulong_t op;
+ xen_ulong_t result;
+ xen_ulong_t args[6];
};
DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);

--
1.7.2.5

2012-08-06 14:35:33

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 06/23] xen: missing includes

Changes in v2:
- remove pvclock hack;
- remove include linux/types.h from xen/interface/xen.h.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/x86/include/asm/xen/interface.h | 2 ++
drivers/tty/hvc/hvc_xen.c | 2 ++
drivers/xen/grant-table.c | 1 +
drivers/xen/xenbus/xenbus_probe_frontend.c | 1 +
include/xen/interface/xen.h | 1 -
include/xen/privcmd.h | 1 +
6 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
index cbf0c9d..a93db16 100644
--- a/arch/x86/include/asm/xen/interface.h
+++ b/arch/x86/include/asm/xen/interface.h
@@ -121,6 +121,8 @@ struct arch_shared_info {
#include "interface_64.h"
#endif

+#include <asm/pvclock-abi.h>
+
#ifndef __ASSEMBLY__
/*
* The following is all CPU context. Note that the fpu_ctxt block is filled
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 944eaeb..dc07f56 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -21,6 +21,7 @@
#include <linux/console.h>
#include <linux/delay.h>
#include <linux/err.h>
+#include <linux/irq.h>
#include <linux/init.h>
#include <linux/types.h>
#include <linux/list.h>
@@ -35,6 +36,7 @@
#include <xen/page.h>
#include <xen/events.h>
#include <xen/interface/io/console.h>
+#include <xen/interface/sched.h>
#include <xen/hvc-console.h>
#include <xen/xenbus.h>

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 0bfc1ef..1d0d95e 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -47,6 +47,7 @@
#include <xen/interface/memory.h>
#include <xen/hvc-console.h>
#include <asm/xen/hypercall.h>
+#include <asm/xen/interface.h>

#include <asm/pgtable.h>
#include <asm/sync_bitops.h>
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
index a31b54d..3159a37 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -21,6 +21,7 @@
#include <xen/xenbus.h>
#include <xen/events.h>
#include <xen/page.h>
+#include <xen/xen.h>

#include <xen/platform_pci.h>

diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index a890804..3871e47 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -10,7 +10,6 @@
#define __XEN_PUBLIC_XEN_H__

#include <asm/xen/interface.h>
-#include <asm/pvclock-abi.h>

/*
* XEN "SYSTEM CALLS" (a.k.a. HYPERCALLS).
diff --git a/include/xen/privcmd.h b/include/xen/privcmd.h
index 17857fb..4d58881 100644
--- a/include/xen/privcmd.h
+++ b/include/xen/privcmd.h
@@ -35,6 +35,7 @@

#include <linux/types.h>
#include <linux/compiler.h>
+#include <xen/interface/xen.h>

typedef unsigned long xen_pfn_t;

--
1.7.2.5

2012-08-06 14:35:34

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 01/23] arm: initial Xen support

- Basic hypervisor.h and interface.h definitions.
- Skeleton enlighten.c, set xen_start_info to an empty struct.
- Make xen_initial_domain dependent on the SIF_PRIVILIGED_BIT.

The new code only compiles when CONFIG_XEN is set, that is going to be
added to arch/arm/Kconfig in patch #11 "xen/arm: introduce CONFIG_XEN on
ARM".

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/Makefile | 1 +
arch/arm/include/asm/hypervisor.h | 6 +++
arch/arm/include/asm/xen/hypervisor.h | 19 ++++++++++
arch/arm/include/asm/xen/interface.h | 64 +++++++++++++++++++++++++++++++++
arch/arm/xen/Makefile | 1 +
arch/arm/xen/enlighten.c | 35 ++++++++++++++++++
include/xen/xen.h | 2 +-
7 files changed, 127 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/include/asm/hypervisor.h
create mode 100644 arch/arm/include/asm/xen/hypervisor.h
create mode 100644 arch/arm/include/asm/xen/interface.h
create mode 100644 arch/arm/xen/Makefile
create mode 100644 arch/arm/xen/enlighten.c

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 0298b00..70aaa82 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -246,6 +246,7 @@ endif
core-$(CONFIG_FPE_NWFPE) += arch/arm/nwfpe/
core-$(CONFIG_FPE_FASTFPE) += $(FASTFPE_OBJ)
core-$(CONFIG_VFP) += arch/arm/vfp/
+core-$(CONFIG_XEN) += arch/arm/xen/

# If we have a machine-specific directory, then include it in the build.
core-y += arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
diff --git a/arch/arm/include/asm/hypervisor.h b/arch/arm/include/asm/hypervisor.h
new file mode 100644
index 0000000..b90d9e5
--- /dev/null
+++ b/arch/arm/include/asm/hypervisor.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_ARM_HYPERVISOR_H
+#define _ASM_ARM_HYPERVISOR_H
+
+#include <asm/xen/hypervisor.h>
+
+#endif
diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h
new file mode 100644
index 0000000..d7ab99a
--- /dev/null
+++ b/arch/arm/include/asm/xen/hypervisor.h
@@ -0,0 +1,19 @@
+#ifndef _ASM_ARM_XEN_HYPERVISOR_H
+#define _ASM_ARM_XEN_HYPERVISOR_H
+
+extern struct shared_info *HYPERVISOR_shared_info;
+extern struct start_info *xen_start_info;
+
+/* Lazy mode for batching updates / context switch */
+enum paravirt_lazy_mode {
+ PARAVIRT_LAZY_NONE,
+ PARAVIRT_LAZY_MMU,
+ PARAVIRT_LAZY_CPU,
+};
+
+static inline enum paravirt_lazy_mode paravirt_get_lazy_mode(void)
+{
+ return PARAVIRT_LAZY_NONE;
+}
+
+#endif /* _ASM_ARM_XEN_HYPERVISOR_H */
diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
new file mode 100644
index 0000000..ab99270
--- /dev/null
+++ b/arch/arm/include/asm/xen/interface.h
@@ -0,0 +1,64 @@
+/******************************************************************************
+ * Guest OS interface to ARM Xen.
+ *
+ * Stefano Stabellini <[email protected]>, Citrix, 2012
+ */
+
+#ifndef _ASM_ARM_XEN_INTERFACE_H
+#define _ASM_ARM_XEN_INTERFACE_H
+
+#include <linux/types.h>
+
+#define __DEFINE_GUEST_HANDLE(name, type) \
+ typedef type * __guest_handle_ ## name
+
+#define DEFINE_GUEST_HANDLE_STRUCT(name) \
+ __DEFINE_GUEST_HANDLE(name, struct name)
+#define DEFINE_GUEST_HANDLE(name) __DEFINE_GUEST_HANDLE(name, name)
+#define GUEST_HANDLE(name) __guest_handle_ ## name
+
+#define set_xen_guest_handle(hnd, val) \
+ do { \
+ if (sizeof(hnd) == 8) \
+ *(uint64_t *)&(hnd) = 0; \
+ (hnd) = val; \
+ } while (0)
+
+#ifndef __ASSEMBLY__
+/* Guest handles for primitive C types. */
+__DEFINE_GUEST_HANDLE(uchar, unsigned char);
+__DEFINE_GUEST_HANDLE(uint, unsigned int);
+__DEFINE_GUEST_HANDLE(ulong, unsigned long);
+DEFINE_GUEST_HANDLE(char);
+DEFINE_GUEST_HANDLE(int);
+DEFINE_GUEST_HANDLE(long);
+DEFINE_GUEST_HANDLE(void);
+DEFINE_GUEST_HANDLE(uint64_t);
+DEFINE_GUEST_HANDLE(uint32_t);
+
+/* Maximum number of virtual CPUs in multi-processor guests. */
+#define MAX_VIRT_CPUS 1
+
+struct arch_vcpu_info { };
+struct arch_shared_info { };
+
+/* XXX: Move pvclock definitions some place arch independent */
+struct pvclock_vcpu_time_info {
+ u32 version;
+ u32 pad0;
+ u64 tsc_timestamp;
+ u64 system_time;
+ u32 tsc_to_system_mul;
+ s8 tsc_shift;
+ u8 flags;
+ u8 pad[2];
+} __attribute__((__packed__)); /* 32 bytes */
+
+struct pvclock_wall_clock {
+ u32 version;
+ u32 sec;
+ u32 nsec;
+} __attribute__((__packed__));
+#endif
+
+#endif /* _ASM_ARM_XEN_INTERFACE_H */
diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
new file mode 100644
index 0000000..0bad594
--- /dev/null
+++ b/arch/arm/xen/Makefile
@@ -0,0 +1 @@
+obj-y := enlighten.o
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
new file mode 100644
index 0000000..d27c2a6
--- /dev/null
+++ b/arch/arm/xen/enlighten.c
@@ -0,0 +1,35 @@
+#include <xen/xen.h>
+#include <xen/interface/xen.h>
+#include <xen/interface/memory.h>
+#include <xen/platform_pci.h>
+#include <asm/xen/hypervisor.h>
+#include <asm/xen/hypercall.h>
+#include <linux/module.h>
+
+struct start_info _xen_start_info;
+struct start_info *xen_start_info = &_xen_start_info;
+EXPORT_SYMBOL_GPL(xen_start_info);
+
+enum xen_domain_type xen_domain_type = XEN_NATIVE;
+EXPORT_SYMBOL_GPL(xen_domain_type);
+
+struct shared_info xen_dummy_shared_info;
+struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
+
+DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
+
+/* XXX: to be removed */
+__read_mostly int xen_have_vector_callback;
+EXPORT_SYMBOL_GPL(xen_have_vector_callback);
+
+int xen_platform_pci_unplug = XEN_UNPLUG_ALL;
+EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
+
+int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
+ unsigned long addr,
+ unsigned long mfn, int nr,
+ pgprot_t prot, unsigned domid)
+{
+ return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
diff --git a/include/xen/xen.h b/include/xen/xen.h
index a164024..2c0d3a5 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -23,7 +23,7 @@ extern enum xen_domain_type xen_domain_type;
#include <xen/interface/xen.h>
#include <asm/xen/hypervisor.h>

-#define xen_initial_domain() (xen_pv_domain() && \
+#define xen_initial_domain() (xen_domain() && \
xen_start_info->flags & SIF_INITDOMAIN)
#else /* !CONFIG_XEN_DOM0 */
#define xen_initial_domain() (0)
--
1.7.2.5

2012-08-06 14:35:32

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 02/23] xen/arm: hypercalls

Use r12 to pass the hypercall number to the hypervisor.

We need a register to pass the hypercall number because we might not
know it at compile time and HVC only takes an immediate argument.

Among the available registers r12 seems to be the best choice because it
is defined as "intra-procedure call scratch register".

Use the ISS to pass an hypervisor specific tag.

Changes in v2:
- define an HYPERCALL macro for 5 arguments hypercall wrappers, even if
at the moment is unused;
- use ldm instead of pop;
- fix up comments.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/hypercall.S | 106 ++++++++++++++++++++++++++++++++++
3 files changed, 157 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/include/asm/xen/hypercall.h
create mode 100644 arch/arm/xen/hypercall.S

diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
new file mode 100644
index 0000000..4ac0624
--- /dev/null
+++ b/arch/arm/include/asm/xen/hypercall.h
@@ -0,0 +1,50 @@
+/******************************************************************************
+ * hypercall.h
+ *
+ * Linux-specific hypervisor handling.
+ *
+ * Stefano Stabellini <[email protected]>, Citrix, 2012
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef _ASM_ARM_XEN_HYPERCALL_H
+#define _ASM_ARM_XEN_HYPERCALL_H
+
+#include <xen/interface/xen.h>
+
+long privcmd_call(unsigned call, unsigned long a1,
+ unsigned long a2, unsigned long a3,
+ unsigned long a4, unsigned long a5);
+int HYPERVISOR_xen_version(int cmd, void *arg);
+int HYPERVISOR_console_io(int cmd, int count, char *str);
+int HYPERVISOR_grant_table_op(unsigned int cmd, void *uop, unsigned int count);
+int HYPERVISOR_sched_op(int cmd, void *arg);
+int HYPERVISOR_event_channel_op(int cmd, void *arg);
+unsigned long HYPERVISOR_hvm_op(int op, void *arg);
+int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
+int HYPERVISOR_physdev_op(int cmd, void *arg);
+
+#endif /* _ASM_ARM_XEN_HYPERCALL_H */
diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
index 0bad594..b9d6acc 100644
--- a/arch/arm/xen/Makefile
+++ b/arch/arm/xen/Makefile
@@ -1 +1 @@
-obj-y := enlighten.o
+obj-y := enlighten.o hypercall.o
diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
new file mode 100644
index 0000000..074f5ed
--- /dev/null
+++ b/arch/arm/xen/hypercall.S
@@ -0,0 +1,106 @@
+/******************************************************************************
+ * hypercall.S
+ *
+ * Xen hypercall wrappers
+ *
+ * Stefano Stabellini <[email protected]>, Citrix, 2012
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/*
+ * The Xen hypercall calling convention is very similar to the ARM
+ * procedure calling convention: the first paramter is passed in r0, the
+ * second in r1, the third in r2 and the fourth in r3. Considering that
+ * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
+ * in r4, differently from the procedure calling convention of using the
+ * stack for that case.
+ *
+ * The hypercall number is passed in r12.
+ *
+ * The return value is in r0.
+ *
+ * The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
+ * hypercall tag.
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <xen/interface/xen.h>
+
+
+/* HVC 0xEA1 */
+#ifdef CONFIG_THUMB2_KERNEL
+#define xen_hvc .word 0xf7e08ea1
+#else
+#define xen_hvc .word 0xe140ea71
+#endif
+
+#define HYPERCALL_SIMPLE(hypercall) \
+ENTRY(HYPERVISOR_##hypercall) \
+ mov r12, #__HYPERVISOR_##hypercall; \
+ xen_hvc; \
+ mov pc, lr; \
+ENDPROC(HYPERVISOR_##hypercall)
+
+#define HYPERCALL0 HYPERCALL_SIMPLE
+#define HYPERCALL1 HYPERCALL_SIMPLE
+#define HYPERCALL2 HYPERCALL_SIMPLE
+#define HYPERCALL3 HYPERCALL_SIMPLE
+#define HYPERCALL4 HYPERCALL_SIMPLE
+
+#define HYPERCALL5(hypercall) \
+ENTRY(HYPERVISOR_##hypercall) \
+ stmdb sp!, {r4} \
+ ldr r4, [sp, #4] \
+ mov r12, #__HYPERVISOR_##hypercall; \
+ xen_hvc \
+ ldm sp!, {r4} \
+ mov pc, lr \
+ENDPROC(HYPERVISOR_##hypercall)
+
+ .text
+
+HYPERCALL2(xen_version);
+HYPERCALL3(console_io);
+HYPERCALL3(grant_table_op);
+HYPERCALL2(sched_op);
+HYPERCALL2(event_channel_op);
+HYPERCALL2(hvm_op);
+HYPERCALL2(memory_op);
+HYPERCALL2(physdev_op);
+
+ENTRY(privcmd_call)
+ stmdb sp!, {r4}
+ mov r12, r0
+ mov r0, r1
+ mov r1, r2
+ mov r2, r3
+ ldr r3, [sp, #8]
+ ldr r4, [sp, #4]
+ xen_hvc
+ ldm sp!, {r4}
+ mov pc, lr
+ENDPROC(privcmd_call);
--
1.7.2.5

2012-08-06 14:36:13

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 08/23] xen/arm: Introduce xen_pfn_t for pfn and mfn types

All the original Xen headers have xen_pfn_t as mfn and pfn type, however
when they have been imported in Linux, xen_pfn_t has been replaced with
unsigned long. That might work for x86 and ia64 but it does not for arm.
Bring back xen_pfn_t and let each architecture define xen_pfn_t as they
see fit.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/xen/interface.h | 4 ++++
arch/ia64/include/asm/xen/interface.h | 5 ++++-
arch/x86/include/asm/xen/interface.h | 5 +++++
include/xen/interface/grant_table.h | 4 ++--
include/xen/interface/memory.h | 6 +++---
include/xen/interface/platform.h | 4 ++--
include/xen/interface/xen.h | 6 +++---
include/xen/privcmd.h | 2 --
8 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
index ab99270..f904dcc 100644
--- a/arch/arm/include/asm/xen/interface.h
+++ b/arch/arm/include/asm/xen/interface.h
@@ -25,6 +25,9 @@
} while (0)

#ifndef __ASSEMBLY__
+/* Explicitly size integers that represent pfns in the interface with
+ * Xen so that we can have one ABI that works for 32 and 64 bit guests. */
+typedef uint64_t xen_pfn_t;
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
@@ -35,6 +38,7 @@ DEFINE_GUEST_HANDLE(long);
DEFINE_GUEST_HANDLE(void);
DEFINE_GUEST_HANDLE(uint64_t);
DEFINE_GUEST_HANDLE(uint32_t);
+DEFINE_GUEST_HANDLE(xen_pfn_t);

/* Maximum number of virtual CPUs in multi-processor guests. */
#define MAX_VIRT_CPUS 1
diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
index 09d5f7f..686464e 100644
--- a/arch/ia64/include/asm/xen/interface.h
+++ b/arch/ia64/include/asm/xen/interface.h
@@ -67,6 +67,10 @@
#define set_xen_guest_handle(hnd, val) do { (hnd).p = val; } while (0)

#ifndef __ASSEMBLY__
+/* Explicitly size integers that represent pfns in the public interface
+ * with Xen so that we could have one ABI that works for 32 and 64 bit
+ * guests. */
+typedef unsigned long xen_pfn_t;
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
@@ -79,7 +83,6 @@ DEFINE_GUEST_HANDLE(void);
DEFINE_GUEST_HANDLE(uint64_t);
DEFINE_GUEST_HANDLE(uint32_t);

-typedef unsigned long xen_pfn_t;
DEFINE_GUEST_HANDLE(xen_pfn_t);
#define PRI_xen_pfn "lx"
#endif
diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
index a93db16..555f94d 100644
--- a/arch/x86/include/asm/xen/interface.h
+++ b/arch/x86/include/asm/xen/interface.h
@@ -47,6 +47,10 @@
#endif

#ifndef __ASSEMBLY__
+/* Explicitly size integers that represent pfns in the public interface
+ * with Xen so that on ARM we can have one ABI that works for 32 and 64
+ * bit guests. */
+typedef unsigned long xen_pfn_t;
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
@@ -57,6 +61,7 @@ DEFINE_GUEST_HANDLE(long);
DEFINE_GUEST_HANDLE(void);
DEFINE_GUEST_HANDLE(uint64_t);
DEFINE_GUEST_HANDLE(uint32_t);
+DEFINE_GUEST_HANDLE(xen_pfn_t);
#endif

#ifndef HYPERVISOR_VIRT_START
diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h
index a17d844..7da811b 100644
--- a/include/xen/interface/grant_table.h
+++ b/include/xen/interface/grant_table.h
@@ -338,7 +338,7 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_dump_table);
#define GNTTABOP_transfer 4
struct gnttab_transfer {
/* IN parameters. */
- unsigned long mfn;
+ xen_pfn_t mfn;
domid_t domid;
grant_ref_t ref;
/* OUT parameters. */
@@ -375,7 +375,7 @@ struct gnttab_copy {
struct {
union {
grant_ref_t ref;
- unsigned long gmfn;
+ xen_pfn_t gmfn;
} u;
domid_t domid;
uint16_t offset;
diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index eac3ce1..abbbff0 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -31,7 +31,7 @@ struct xen_memory_reservation {
* OUT: GMFN bases of extents that were allocated
* (NB. This command also updates the mach_to_phys translation table)
*/
- GUEST_HANDLE(ulong) extent_start;
+ GUEST_HANDLE(xen_pfn_t) extent_start;

/* Number of extents, and size/alignment of each (2^extent_order pages). */
unsigned long nr_extents;
@@ -130,7 +130,7 @@ struct xen_machphys_mfn_list {
* any large discontiguities in the machine address space, 2MB gaps in
* the machphys table will be represented by an MFN base of zero.
*/
- GUEST_HANDLE(ulong) extent_start;
+ GUEST_HANDLE(xen_pfn_t) extent_start;

/*
* Number of extents written to the above array. This will be smaller
@@ -172,7 +172,7 @@ struct xen_add_to_physmap {
unsigned long idx;

/* GPFN where the source mapping page should appear. */
- unsigned long gpfn;
+ xen_pfn_t gpfn;
};
DEFINE_GUEST_HANDLE_STRUCT(xen_add_to_physmap);

diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 486653f..0bea470 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -54,7 +54,7 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
#define XENPF_add_memtype 31
struct xenpf_add_memtype {
/* IN variables. */
- unsigned long mfn;
+ xen_pfn_t mfn;
uint64_t nr_mfns;
uint32_t type;
/* OUT variables. */
@@ -84,7 +84,7 @@ struct xenpf_read_memtype {
/* IN variables. */
uint32_t reg;
/* OUT variables. */
- unsigned long mfn;
+ xen_pfn_t mfn;
uint64_t nr_mfns;
uint32_t type;
};
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 3871e47..42834a3 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -188,7 +188,7 @@ struct mmuext_op {
unsigned int cmd;
union {
/* [UN]PIN_TABLE, NEW_BASEPTR, NEW_USER_BASEPTR */
- unsigned long mfn;
+ xen_pfn_t mfn;
/* INVLPG_LOCAL, INVLPG_ALL, SET_LDT */
unsigned long linear_addr;
} arg1;
@@ -428,11 +428,11 @@ struct start_info {
unsigned long nr_pages; /* Total pages allocated to this domain. */
unsigned long shared_info; /* MACHINE address of shared info struct. */
uint32_t flags; /* SIF_xxx flags. */
- unsigned long store_mfn; /* MACHINE page number of shared page. */
+ xen_pfn_t store_mfn; /* MACHINE page number of shared page. */
uint32_t store_evtchn; /* Event channel for store communication. */
union {
struct {
- unsigned long mfn; /* MACHINE page number of console page. */
+ xen_pfn_t mfn; /* MACHINE page number of console page. */
uint32_t evtchn; /* Event channel for console page. */
} domU;
struct {
diff --git a/include/xen/privcmd.h b/include/xen/privcmd.h
index 4d58881..45c1aa1 100644
--- a/include/xen/privcmd.h
+++ b/include/xen/privcmd.h
@@ -37,8 +37,6 @@
#include <linux/compiler.h>
#include <xen/interface/xen.h>

-typedef unsigned long xen_pfn_t;
-
struct privcmd_hypercall {
__u64 op;
__u64 arg[5];
--
1.7.2.5

2012-08-06 14:36:12

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 04/23] xen/arm: sync_bitops

sync_bitops functions are equivalent to the SMP implementation of the
original functions, independently from CONFIG_SMP being defined.

We need them because _set_bit etc are not SMP safe if !CONFIG_SMP. But
under Xen you might be communicating with a completely external entity
who might be on another CPU (e.g. two uniprocessor guests communicating
via event channels and grant tables). So we need a variant of the bit
ops which are SMP safe even on a UP kernel.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/sync_bitops.h | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/include/asm/sync_bitops.h

diff --git a/arch/arm/include/asm/sync_bitops.h b/arch/arm/include/asm/sync_bitops.h
new file mode 100644
index 0000000..63479ee
--- /dev/null
+++ b/arch/arm/include/asm/sync_bitops.h
@@ -0,0 +1,27 @@
+#ifndef __ASM_SYNC_BITOPS_H__
+#define __ASM_SYNC_BITOPS_H__
+
+#include <asm/bitops.h>
+#include <asm/system.h>
+
+/* sync_bitops functions are equivalent to the SMP implementation of the
+ * original functions, independently from CONFIG_SMP being defined.
+ *
+ * We need them because _set_bit etc are not SMP safe if !CONFIG_SMP. But
+ * under Xen you might be communicating with a completely external entity
+ * who might be on another CPU (e.g. two uniprocessor guests communicating
+ * via event channels and grant tables). So we need a variant of the bit
+ * ops which are SMP safe even on a UP kernel.
+ */
+
+#define sync_set_bit(nr, p) _set_bit(nr, p)
+#define sync_clear_bit(nr, p) _clear_bit(nr, p)
+#define sync_change_bit(nr, p) _change_bit(nr, p)
+#define sync_test_and_set_bit(nr, p) _test_and_set_bit(nr, p)
+#define sync_test_and_clear_bit(nr, p) _test_and_clear_bit(nr, p)
+#define sync_test_and_change_bit(nr, p) _test_and_change_bit(nr, p)
+#define sync_test_bit(nr, addr) test_bit(nr, addr)
+#define sync_cmpxchg cmpxchg
+
+
+#endif
--
1.7.2.5

2012-08-06 14:36:42

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 07/23] xen/arm: Xen detection and shared_info page mapping

Check for a "/xen" node in the device tree, if it is present set
xen_domain_type to XEN_HVM_DOMAIN and continue initialization.

Map the real shared info page using XENMEM_add_to_physmap with
XENMAPSPACE_shared_info.

Changes in v2:

- replace pr_info with pr_debug.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/xen/enlighten.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index d27c2a6..102d823 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -5,6 +5,9 @@
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>

struct start_info _xen_start_info;
struct start_info *xen_start_info = &_xen_start_info;
@@ -33,3 +36,52 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
return -ENOSYS;
}
EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
+
+/*
+ * == Xen Device Tree format ==
+ * - /xen node;
+ * - compatible "arm,xen";
+ * - one interrupt for Xen event notifications;
+ * - one memory region to map the grant_table.
+ */
+static int __init xen_guest_init(void)
+{
+ struct xen_add_to_physmap xatp;
+ static struct shared_info *shared_info_page = 0;
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "arm,xen");
+ if (!node) {
+ pr_debug("No Xen support\n");
+ return 0;
+ }
+ xen_domain_type = XEN_HVM_DOMAIN;
+
+ if (!shared_info_page)
+ shared_info_page = (struct shared_info *)
+ get_zeroed_page(GFP_KERNEL);
+ if (!shared_info_page) {
+ pr_err("not enough memory\n");
+ return -ENOMEM;
+ }
+ xatp.domid = DOMID_SELF;
+ xatp.idx = 0;
+ xatp.space = XENMAPSPACE_shared_info;
+ xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
+ if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
+ BUG();
+
+ HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
+
+ /* xen_vcpu is a pointer to the vcpu_info struct in the shared_info
+ * page, we use it in the event channel upcall and in some pvclock
+ * related functions. We don't need the vcpu_info placement
+ * optimizations because we don't use any pv_mmu or pv_irq op on
+ * HVM.
+ * The shared info contains exactly 1 CPU (the boot CPU). The guest
+ * is required to use VCPUOP_register_vcpu_info to place vcpu info
+ * for secondary CPUs as they are brought up. */
+ per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
+ return 0;
+}
+core_initcall(xen_guest_init);
--
1.7.2.5

2012-08-06 14:36:57

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 05/23] xen/arm: empty implementation of grant_table arch specific functions

Changes in v2:

- return -ENOSYS rather than -1.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/grant-table.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/xen/grant-table.c

diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
index b9d6acc..4384103 100644
--- a/arch/arm/xen/Makefile
+++ b/arch/arm/xen/Makefile
@@ -1 +1 @@
-obj-y := enlighten.o hypercall.o
+obj-y := enlighten.o hypercall.o grant-table.o
diff --git a/arch/arm/xen/grant-table.c b/arch/arm/xen/grant-table.c
new file mode 100644
index 0000000..dbd1330
--- /dev/null
+++ b/arch/arm/xen/grant-table.c
@@ -0,0 +1,53 @@
+/******************************************************************************
+ * grant_table.c
+ * ARM specific part
+ *
+ * Granting foreign access to our memory reservation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <xen/interface/xen.h>
+#include <xen/page.h>
+#include <xen/grant_table.h>
+
+int arch_gnttab_map_shared(unsigned long *frames, unsigned long nr_gframes,
+ unsigned long max_nr_gframes,
+ void **__shared)
+{
+ return -ENOSYS;
+}
+
+void arch_gnttab_unmap(void *shared, unsigned long nr_gframes)
+{
+ return;
+}
+
+int arch_gnttab_map_status(uint64_t *frames, unsigned long nr_gframes,
+ unsigned long max_nr_gframes,
+ grant_status_t **__shared)
+{
+ return -ENOSYS;
+}
--
1.7.2.5

2012-08-06 14:38:05

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 03/23] xen/arm: page.h definitions

ARM Xen guests always use paging in hardware, like PV on HVM guests in
the X86 world.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/xen/page.h | 79 +++++++++++++++++++++++++++++++++++++++
1 files changed, 79 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/include/asm/xen/page.h

diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
new file mode 100644
index 0000000..fe78331
--- /dev/null
+++ b/arch/arm/include/asm/xen/page.h
@@ -0,0 +1,79 @@
+#ifndef _ASM_ARM_XEN_PAGE_H
+#define _ASM_ARM_XEN_PAGE_H
+
+#include <asm/page.h>
+#include <asm/pgtable.h>
+
+#include <linux/pfn.h>
+#include <linux/types.h>
+
+#include <xen/interface/grant_table.h>
+
+#define pfn_to_mfn(pfn) (pfn)
+#define phys_to_machine_mapping_valid (1)
+#define mfn_to_pfn(mfn) (mfn)
+#define mfn_to_virt(m) (__va(mfn_to_pfn(m) << PAGE_SHIFT))
+
+#define pte_mfn pte_pfn
+#define mfn_pte pfn_pte
+
+/* Xen machine address */
+typedef struct xmaddr {
+ phys_addr_t maddr;
+} xmaddr_t;
+
+/* Xen pseudo-physical address */
+typedef struct xpaddr {
+ phys_addr_t paddr;
+} xpaddr_t;
+
+#define XMADDR(x) ((xmaddr_t) { .maddr = (x) })
+#define XPADDR(x) ((xpaddr_t) { .paddr = (x) })
+
+static inline xmaddr_t phys_to_machine(xpaddr_t phys)
+{
+ unsigned offset = phys.paddr & ~PAGE_MASK;
+ return XMADDR(PFN_PHYS(pfn_to_mfn(PFN_DOWN(phys.paddr))) | offset);
+}
+
+static inline xpaddr_t machine_to_phys(xmaddr_t machine)
+{
+ unsigned offset = machine.maddr & ~PAGE_MASK;
+ return XPADDR(PFN_PHYS(mfn_to_pfn(PFN_DOWN(machine.maddr))) | offset);
+}
+/* VIRT <-> MACHINE conversion */
+#define virt_to_machine(v) (phys_to_machine(XPADDR(__pa(v))))
+#define virt_to_pfn(v) (PFN_DOWN(__pa(v)))
+#define virt_to_mfn(v) (pfn_to_mfn(virt_to_pfn(v)))
+#define mfn_to_virt(m) (__va(mfn_to_pfn(m) << PAGE_SHIFT))
+
+static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
+{
+ /* XXX: assuming it is mapped in the kernel 1:1 */
+ return virt_to_machine(vaddr);
+}
+
+/* XXX: this shouldn't be here */
+static inline pte_t *lookup_address(unsigned long address, unsigned int *level)
+{
+ BUG();
+ return NULL;
+}
+
+static inline int m2p_add_override(unsigned long mfn, struct page *page,
+ struct gnttab_map_grant_ref *kmap_op)
+{
+ return 0;
+}
+
+static inline int m2p_remove_override(struct page *page, bool clear_pte)
+{
+ return 0;
+}
+
+static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
+{
+ BUG();
+ return false;
+}
+#endif /* _ASM_ARM_XEN_PAGE_H */
--
1.7.2.5

2012-08-06 14:55:47

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 17/23] xen/arm: implement alloc/free_xenballooned_pages with alloc_pages/kfree

Only until we get the balloon driver to work.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/xen/enlighten.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 87b17f0..c244583 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -140,3 +140,21 @@ static int __init xen_init_events(void)
return 0;
}
postcore_initcall(xen_init_events);
+
+/* XXX: only until balloon is properly working */
+int alloc_xenballooned_pages(int nr_pages, struct page **pages, bool highmem)
+{
+ *pages = alloc_pages(highmem ? GFP_HIGHUSER : GFP_KERNEL,
+ get_order(nr_pages));
+ if (*pages == NULL)
+ return -ENOMEM;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(alloc_xenballooned_pages);
+
+void free_xenballooned_pages(int nr_pages, struct page **pages)
+{
+ kfree(*pages);
+ *pages = NULL;
+}
+EXPORT_SYMBOL_GPL(free_xenballooned_pages);
--
1.7.2.5

2012-08-06 14:55:54

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 19/23] xen/arm: compile blkfront and blkback

Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/block/xen-blkback/blkback.c | 1 +
include/xen/interface/io/protocols.h | 3 +++
2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 73f196c..63dd5b9 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -42,6 +42,7 @@

#include <xen/events.h>
#include <xen/page.h>
+#include <xen/xen.h>
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
#include "common.h"
diff --git a/include/xen/interface/io/protocols.h b/include/xen/interface/io/protocols.h
index 01fc8ae..0eafaf2 100644
--- a/include/xen/interface/io/protocols.h
+++ b/include/xen/interface/io/protocols.h
@@ -5,6 +5,7 @@
#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi"
#define XEN_IO_PROTO_ABI_IA64 "ia64-abi"
#define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi"
+#define XEN_IO_PROTO_ABI_ARM "arm-abi"

#if defined(__i386__)
# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
@@ -14,6 +15,8 @@
# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
#elif defined(__powerpc64__)
# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
+#elif defined(__arm__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_ARM
#else
# error arch fixup needed here
#endif
--
1.7.2.5

2012-08-06 14:56:04

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 22/23] arm/v2m: initialize arch_timers even if v2m_timer is not present

Signed-off-by: Stefano Stabellini <[email protected]>
CC: Russell King <[email protected]>
---
arch/arm/mach-vexpress/v2m.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index fde26ad..dee1451 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -637,16 +637,17 @@ static void __init v2m_dt_timer_init(void)
node = of_find_compatible_node(NULL, NULL, "arm,sp810");
v2m_sysctl_init(of_iomap(node, 0));

- err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
- if (WARN_ON(err))
- return;
- node = of_find_node_by_path(path);
- v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0));
if (arch_timer_of_register() != 0)
twd_local_timer_of_register();

if (arch_timer_sched_clock_init() != 0)
versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000);
+
+ err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
+ if (WARN_ON(err))
+ return;
+ node = of_find_node_by_path(path);
+ v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0));
}

static struct sys_timer v2m_dt_timer = {
--
1.7.2.5

2012-08-06 14:56:07

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 16/23] xen: clear IRQ_NOAUTOEN and IRQ_NOREQUEST

Reset the IRQ_NOAUTOEN and IRQ_NOREQUEST flags that are enabled by
default on ARM. If IRQ_NOAUTOEN is set, __setup_irq doesn't call
irq_startup, that is responsible for calling irq_unmask at startup time.
As a result event channels remain masked.

Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/xen/events.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 5ecb596..8ffb7b7 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -836,6 +836,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)
struct irq_info *info = info_for_irq(irq);
WARN_ON(info == NULL || info->type != IRQT_EVTCHN);
}
+ irq_clear_status_flags(irq, IRQ_NOREQUEST|IRQ_NOAUTOEN);

out:
mutex_unlock(&irq_mapping_update_lock);
--
1.7.2.5

2012-08-06 14:56:17

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 12/23] xen/arm: introduce CONFIG_XEN on ARM

Changes in v2:

- mark Xen guest support on ARM as EXPERIMENTAL.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/Kconfig | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a91009c..f14664b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1855,6 +1855,16 @@ config DEPRECATED_PARAM_STRUCT
This was deprecated in 2001 and announced to live on for 5 years.
Some old boot loaders still use this way.

+config XEN_DOM0
+ def_bool y
+
+config XEN
+ bool "Xen guest support on ARM (EXPERIMENTAL)"
+ depends on EXPERIMENTAL && ARM && OF
+ select XEN_DOM0
+ help
+ Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
+
endmenu

menu "Boot options"
--
1.7.2.5

2012-08-06 14:56:25

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 15/23] xen/arm: receive Xen events on ARM

Compile events.c on ARM.
Parse, map and enable the IRQ to get event notifications from the device
tree (node "/xen").

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/xen/events.h | 18 ++++++++++++++++++
arch/arm/xen/enlighten.c | 33 +++++++++++++++++++++++++++++++++
arch/x86/xen/enlighten.c | 1 +
arch/x86/xen/irq.c | 1 +
arch/x86/xen/xen-ops.h | 1 -
drivers/xen/events.c | 17 ++++++++++++++---
include/xen/events.h | 2 ++
7 files changed, 69 insertions(+), 4 deletions(-)
create mode 100644 arch/arm/include/asm/xen/events.h

diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
new file mode 100644
index 0000000..94b4e90
--- /dev/null
+++ b/arch/arm/include/asm/xen/events.h
@@ -0,0 +1,18 @@
+#ifndef _ASM_ARM_XEN_EVENTS_H
+#define _ASM_ARM_XEN_EVENTS_H
+
+#include <asm/ptrace.h>
+
+enum ipi_vector {
+ XEN_PLACEHOLDER_VECTOR,
+
+ /* Xen IPIs go here */
+ XEN_NR_IPIS,
+};
+
+static inline int xen_irqs_disabled(struct pt_regs *regs)
+{
+ return raw_irqs_disabled_flags(regs->ARM_cpsr);
+}
+
+#endif /* _ASM_ARM_XEN_EVENTS_H */
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index e5e92d5..87b17f0 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -1,4 +1,5 @@
#include <xen/xen.h>
+#include <xen/events.h>
#include <xen/grant_table.h>
#include <xen/hvm.h>
#include <xen/interface/xen.h>
@@ -9,6 +10,8 @@
#include <xen/xenbus.h>
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_irq.h>
@@ -33,6 +36,8 @@ EXPORT_SYMBOL_GPL(xen_have_vector_callback);
int xen_platform_pci_unplug = XEN_UNPLUG_ALL;
EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);

+static __read_mostly int xen_events_irq = -1;
+
int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
unsigned long addr,
unsigned long mfn, int nr,
@@ -66,6 +71,9 @@ static int __init xen_guest_init(void)
if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res))
return 0;
xen_hvm_resume_frames = res.start >> PAGE_SHIFT;
+ xen_events_irq = irq_of_parse_and_map(node, 0);
+ pr_info("Xen support found, events_irq=%d gnttab_frame_pfn=%lx\n",
+ xen_events_irq, xen_hvm_resume_frames);
xen_domain_type = XEN_HVM_DOMAIN;

xen_setup_features();
@@ -107,3 +115,28 @@ static int __init xen_guest_init(void)
return 0;
}
core_initcall(xen_guest_init);
+
+static irqreturn_t xen_arm_callback(int irq, void *arg)
+{
+ xen_hvm_evtchn_do_upcall();
+ return IRQ_HANDLED;
+}
+
+static int __init xen_init_events(void)
+{
+ if (!xen_domain() || xen_events_irq < 0)
+ return -ENODEV;
+
+ xen_init_IRQ();
+
+ if (request_percpu_irq(xen_events_irq, xen_arm_callback,
+ "events", xen_vcpu)) {
+ pr_err("Error requesting IRQ %d\n", xen_events_irq);
+ return -EINVAL;
+ }
+
+ enable_percpu_irq(xen_events_irq, 0);
+
+ return 0;
+}
+postcore_initcall(xen_init_events);
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index ff962d4..9f8b0ef 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -33,6 +33,7 @@
#include <linux/memblock.h>

#include <xen/xen.h>
+#include <xen/events.h>
#include <xen/interface/xen.h>
#include <xen/interface/version.h>
#include <xen/interface/physdev.h>
diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index 1573376..01a4dc0 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -5,6 +5,7 @@
#include <xen/interface/xen.h>
#include <xen/interface/sched.h>
#include <xen/interface/vcpu.h>
+#include <xen/events.h>

#include <asm/xen/hypercall.h>
#include <asm/xen/hypervisor.h>
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 202d4c1..2368295 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -35,7 +35,6 @@ void xen_set_pat(u64);

char * __init xen_memory_setup(void);
void __init xen_arch_setup(void);
-void __init xen_init_IRQ(void);
void xen_enable_sysenter(void);
void xen_enable_syscall(void);
void xen_vcpu_restore(void);
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 7595581..5ecb596 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -31,14 +31,16 @@
#include <linux/irqnr.h>
#include <linux/pci.h>

+#ifdef CONFIG_X86
#include <asm/desc.h>
#include <asm/ptrace.h>
#include <asm/irq.h>
#include <asm/idle.h>
#include <asm/io_apic.h>
-#include <asm/sync_bitops.h>
#include <asm/xen/page.h>
#include <asm/xen/pci.h>
+#endif
+#include <asm/sync_bitops.h>
#include <asm/xen/hypercall.h>
#include <asm/xen/hypervisor.h>

@@ -50,6 +52,9 @@
#include <xen/interface/event_channel.h>
#include <xen/interface/hvm/hvm_op.h>
#include <xen/interface/hvm/params.h>
+#include <xen/interface/physdev.h>
+#include <xen/interface/sched.h>
+#include <asm/hw_irq.h>

/*
* This lock protects updates to the following mapping and reference-count
@@ -1374,7 +1379,9 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);

+#ifdef CONFIG_X86
exit_idle();
+#endif
irq_enter();

__xen_evtchn_do_upcall();
@@ -1783,9 +1790,9 @@ void xen_callback_vector(void)
void xen_callback_vector(void) {}
#endif

-void __init xen_init_IRQ(void)
+void xen_init_IRQ(void)
{
- int i, rc;
+ int i;

evtchn_to_irq = kcalloc(NR_EVENT_CHANNELS, sizeof(*evtchn_to_irq),
GFP_KERNEL);
@@ -1801,6 +1808,7 @@ void __init xen_init_IRQ(void)

pirq_needs_eoi = pirq_needs_eoi_flag;

+#ifdef CONFIG_X86
if (xen_hvm_domain()) {
xen_callback_vector();
native_init_IRQ();
@@ -1808,6 +1816,7 @@ void __init xen_init_IRQ(void)
* __acpi_register_gsi can point at the right function */
pci_xen_hvm_init();
} else {
+ int rc;
struct physdev_pirq_eoi_gmfn eoi_gmfn;

irq_ctx_init(smp_processor_id());
@@ -1823,4 +1832,6 @@ void __init xen_init_IRQ(void)
} else
pirq_needs_eoi = pirq_check_eoi_map;
}
+#endif
}
+EXPORT_SYMBOL_GPL(xen_init_IRQ);
diff --git a/include/xen/events.h b/include/xen/events.h
index 04399b2..c6bfe01 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -109,4 +109,6 @@ int xen_irq_from_gsi(unsigned gsi);
/* Determine whether to ignore this IRQ if it is passed to a guest. */
int xen_test_irq_shared(int irq);

+/* initialize Xen IRQ subsystem */
+void xen_init_IRQ(void);
#endif /* _XEN_EVENTS_H */
--
1.7.2.5

2012-08-06 14:56:33

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 13/23] xen/arm: get privilege status

Use Xen features to figure out if we are privileged.

XENFEAT_dom0 was introduced by 23735 in xen-unstable.hg.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/xen/enlighten.c | 7 +++++++
include/xen/interface/features.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 102d823..ac3a2d6 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -1,6 +1,7 @@
#include <xen/xen.h>
#include <xen/interface/xen.h>
#include <xen/interface/memory.h>
+#include <xen/features.h>
#include <xen/platform_pci.h>
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
@@ -57,6 +58,12 @@ static int __init xen_guest_init(void)
}
xen_domain_type = XEN_HVM_DOMAIN;

+ xen_setup_features();
+ if (xen_feature(XENFEAT_dom0))
+ xen_start_info->flags |= SIF_INITDOMAIN|SIF_PRIVILEGED;
+ else
+ xen_start_info->flags &= ~(SIF_INITDOMAIN|SIF_PRIVILEGED);
+
if (!shared_info_page)
shared_info_page = (struct shared_info *)
get_zeroed_page(GFP_KERNEL);
diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
index b6ca39a..131a6cc 100644
--- a/include/xen/interface/features.h
+++ b/include/xen/interface/features.h
@@ -50,6 +50,9 @@
/* x86: pirq can be used by HVM guests */
#define XENFEAT_hvm_pirqs 10

+/* operation as Dom0 is supported */
+#define XENFEAT_dom0 11
+
#define XENFEAT_NR_SUBMAPS 1

#endif /* __XEN_PUBLIC_FEATURES_H__ */
--
1.7.2.5

2012-08-06 14:56:38

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 20/23] xen/arm: compile netback

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/include/asm/xen/hypercall.h | 19 +++++++++++++++++++
drivers/net/xen-netback/netback.c | 1 +
drivers/net/xen-netfront.c | 1 +
3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
index 4ac0624..8a82325 100644
--- a/arch/arm/include/asm/xen/hypercall.h
+++ b/arch/arm/include/asm/xen/hypercall.h
@@ -47,4 +47,23 @@ unsigned long HYPERVISOR_hvm_op(int op, void *arg);
int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
int HYPERVISOR_physdev_op(int cmd, void *arg);

+static inline void
+MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
+ unsigned int new_val, unsigned long flags)
+{
+ BUG();
+}
+
+static inline void
+MULTI_mmu_update(struct multicall_entry *mcl, struct mmu_update *req,
+ int count, int *success_count, domid_t domid)
+{
+ BUG();
+}
+
+static inline int
+HYPERVISOR_multicall(void *call_list, int nr_calls)
+{
+ BUG();
+}
#endif /* _ASM_ARM_XEN_HYPERCALL_H */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index f4a6fca..ab4f81c 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -40,6 +40,7 @@

#include <net/tcp.h>

+#include <xen/xen.h>
#include <xen/events.h>
#include <xen/interface/memory.h>

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 3089990..bf4ba2b 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -43,6 +43,7 @@
#include <linux/slab.h>
#include <net/ip.h>

+#include <asm/xen/page.h>
#include <xen/xen.h>
#include <xen/xenbus.h>
#include <xen/events.h>
--
1.7.2.5

2012-08-06 14:56:48

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 21/23] xen: update xen_add_to_physmap interface

Update struct xen_add_to_physmap to be in sync with Xen's version of the
structure.
The size field was introduced by:

changeset: 24164:707d27fe03e7
user: Jean Guyader <[email protected]>
date: Fri Nov 18 13:42:08 2011 +0000
summary: mm: New XENMEM space, XENMAPSPACE_gmfn_range

According to the comment:

"This new field .size is located in the 16 bits padding between .domid
and .space in struct xen_add_to_physmap to stay compatible with older
versions."

Changes in v2:

- remove erroneous comment in the commit message.

Signed-off-by: Stefano Stabellini <[email protected]>
---
include/xen/interface/memory.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index b5c3098..b66d04c 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -163,6 +163,9 @@ struct xen_add_to_physmap {
/* Which domain to change the mapping for. */
domid_t domid;

+ /* Number of pages to go through for gmfn_range */
+ uint16_t size;
+
/* Source mapping space. */
#define XENMAPSPACE_shared_info 0 /* shared info page */
#define XENMAPSPACE_grant_table 1 /* grant table page */
--
1.7.2.5

2012-08-06 14:56:42

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 18/23] xen: allow privcmd for HVM guests

This patch removes the "return -ENOSYS" for auto_translated_physmap
guests from privcmd_mmap, thus it allows ARM guests to issue privcmd
mmap calls. However privcmd mmap calls are still going to fail for HVM
and hybrid guests on x86 because the xen_remap_domain_mfn_range
implementation is currently PV only.

Changes in v2:

- better commit message;
- return -EINVAL from xen_remap_domain_mfn_range if
auto_translated_physmap.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/x86/xen/mmu.c | 3 +++
drivers/xen/privcmd.c | 4 ----
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 3a73785..885a223 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2310,6 +2310,9 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
unsigned long range;
int err = 0;

+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ return -EINVAL;
+
prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP);

BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_RESERVED | VM_IO)) ==
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index ccee0f1..85226cb 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -380,10 +380,6 @@ static struct vm_operations_struct privcmd_vm_ops = {

static int privcmd_mmap(struct file *file, struct vm_area_struct *vma)
{
- /* Unsupported for auto-translate guests. */
- if (xen_feature(XENFEAT_auto_translated_physmap))
- return -ENOSYS;
-
/* 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;
--
1.7.2.5

2012-08-06 14:56:55

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 23/23] [HACK] xen/arm: implement xen_remap_domain_mfn_range

From: Ian Campbell <[email protected]>

Do not apply!

This is a simple, hacky implementation of xen_remap_domain_mfn_range,
using XENMAPSPACE_gmfn_foreign.

It should use same interface as hybrid x86.

Changes in v2:

- retain binary compatibility in xen_add_to_physmap: use a union.

Signed-off-by: Ian Campbell <[email protected]>
Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/xen/enlighten.c | 79 +++++++++++++++++++++++++++++++++++++++-
drivers/xen/privcmd.c | 16 +++++----
drivers/xen/xenfs/super.c | 7 ++++
include/xen/interface/memory.h | 15 ++++++--
4 files changed, 105 insertions(+), 12 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index c244583..20ca1e4 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -16,6 +16,10 @@
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
+#include <linux/mm.h>
+#include <linux/ioport.h>
+
+#include <asm/pgtable.h>

struct start_info _xen_start_info;
struct start_info *xen_start_info = &_xen_start_info;
@@ -38,12 +42,85 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);

static __read_mostly int xen_events_irq = -1;

+#define FOREIGN_MAP_BUFFER 0x90000000UL
+#define FOREIGN_MAP_BUFFER_SIZE 0x10000000UL
+struct resource foreign_map_resource = {
+ .start = FOREIGN_MAP_BUFFER,
+ .end = FOREIGN_MAP_BUFFER + FOREIGN_MAP_BUFFER_SIZE,
+ .name = "Xen foreign map buffer",
+ .flags = 0,
+};
+
+static unsigned long foreign_map_buffer_pfn = FOREIGN_MAP_BUFFER >> PAGE_SHIFT;
+
+struct remap_data {
+ struct mm_struct *mm;
+ unsigned long mfn;
+ pgprot_t prot;
+};
+
+static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
+ unsigned long addr, void *data)
+{
+ struct remap_data *rmd = data;
+ pte_t pte = pfn_pte(rmd->mfn, rmd->prot);
+
+ if (rmd->mfn < 0x90010)
+ pr_crit("%s: ptep %p addr %#lx => %#x / %#lx\n",
+ __func__, ptep, addr, pte_val(pte), rmd->mfn);
+
+ set_pte_at(rmd->mm, addr, ptep, pte);
+
+ rmd->mfn++;
+ return 0;
+}
+
int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
unsigned long addr,
unsigned long mfn, int nr,
pgprot_t prot, unsigned domid)
{
- return -ENOSYS;
+ int i, rc = 0;
+ struct remap_data rmd = {
+ .mm = vma->vm_mm,
+ .prot = prot,
+ };
+ struct xen_add_to_physmap xatp = {
+ .domid = DOMID_SELF,
+ .space = XENMAPSPACE_gmfn_foreign,
+
+ .foreign_domid = domid,
+ };
+
+ if (foreign_map_buffer_pfn + nr > ((FOREIGN_MAP_BUFFER +
+ FOREIGN_MAP_BUFFER_SIZE)>>PAGE_SHIFT)) {
+ pr_crit("RAM out of foreign map buffers...\n");
+ return -EBUSY;
+ }
+
+ for (i = 0; i < nr; i++) {
+ xatp.idx = mfn + i;
+ xatp.gpfn = foreign_map_buffer_pfn + i;
+ rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
+ if (rc != 0) {
+ pr_crit("foreign map add_to_physmap failed, err=%d\n", rc);
+ goto out;
+ }
+ }
+
+ rmd.mfn = foreign_map_buffer_pfn;
+ rc = apply_to_page_range(vma->vm_mm,
+ addr,
+ (unsigned long)nr << PAGE_SHIFT,
+ remap_area_mfn_pte_fn, &rmd);
+ if (rc != 0) {
+ pr_crit("apply_to_page_range failed rc=%d\n", rc);
+ goto out;
+ }
+
+ foreign_map_buffer_pfn += nr;
+out:
+ return rc;
}
EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 85226cb..3e15c22 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -20,6 +20,8 @@
#include <linux/pagemap.h>
#include <linux/seq_file.h>
#include <linux/miscdevice.h>
+#include <linux/resource.h>
+#include <linux/ioport.h>

#include <asm/pgalloc.h>
#include <asm/pgtable.h>
@@ -196,9 +198,6 @@ static long privcmd_ioctl_mmap(void __user *udata)
LIST_HEAD(pagelist);
struct mmap_mfn_state state;

- if (!xen_initial_domain())
- return -EPERM;
-
if (copy_from_user(&mmapcmd, udata, sizeof(mmapcmd)))
return -EFAULT;

@@ -286,9 +285,6 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
LIST_HEAD(pagelist);
struct mmap_batch_state state;

- if (!xen_initial_domain())
- return -EPERM;
-
if (copy_from_user(&m, udata, sizeof(m)))
return -EFAULT;

@@ -365,6 +361,11 @@ static long privcmd_ioctl(struct file *file,
return ret;
}

+static void privcmd_close(struct vm_area_struct *vma)
+{
+ /* TODO: unmap VMA */
+}
+
static int privcmd_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
printk(KERN_DEBUG "privcmd_fault: vma=%p %lx-%lx, pgoff=%lx, uv=%p\n",
@@ -375,7 +376,8 @@ static int privcmd_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
}

static struct vm_operations_struct privcmd_vm_ops = {
- .fault = privcmd_fault
+ .fault = privcmd_fault,
+ .close = privcmd_close,
};

static int privcmd_mmap(struct file *file, struct vm_area_struct *vma)
diff --git a/drivers/xen/xenfs/super.c b/drivers/xen/xenfs/super.c
index a84b53c..edbe22f 100644
--- a/drivers/xen/xenfs/super.c
+++ b/drivers/xen/xenfs/super.c
@@ -12,6 +12,7 @@
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/magic.h>
+#include <linux/ioport.h>

#include <xen/xen.h>

@@ -80,6 +81,8 @@ static const struct file_operations capabilities_file_ops = {
.llseek = default_llseek,
};

+extern struct resource foreign_map_resource;
+
static int xenfs_fill_super(struct super_block *sb, void *data, int silent)
{
static struct tree_descr xenfs_files[] = {
@@ -100,6 +103,10 @@ static int xenfs_fill_super(struct super_block *sb, void *data, int silent)
&xsd_kva_file_ops, NULL, S_IRUSR|S_IWUSR);
xenfs_create_file(sb, sb->s_root, "xsd_port",
&xsd_port_file_ops, NULL, S_IRUSR|S_IWUSR);
+ rc = request_resource(&iomem_resource, &foreign_map_resource);
+ if (rc < 0)
+ pr_crit("failed to register foreign map resource\n");
+ rc = 0; /* ignore */
}

return rc;
diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index b66d04c..dd2ffe0 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -163,12 +163,19 @@ struct xen_add_to_physmap {
/* Which domain to change the mapping for. */
domid_t domid;

- /* Number of pages to go through for gmfn_range */
- uint16_t size;
+ union {
+ /* Number of pages to go through for gmfn_range */
+ uint16_t size;
+ /* IFF gmfn_foreign */
+ domid_t foreign_domid;
+ };

/* Source mapping space. */
-#define XENMAPSPACE_shared_info 0 /* shared info page */
-#define XENMAPSPACE_grant_table 1 /* grant table page */
+#define XENMAPSPACE_shared_info 0 /* shared info page */
+#define XENMAPSPACE_grant_table 1 /* grant table page */
+#define XENMAPSPACE_gmfn 2 /* GMFN */
+#define XENMAPSPACE_gmfn_range 3 /* GMFN range */
+#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
unsigned int space;

/* Index into source mapping space. */
--
1.7.2.5

2012-08-06 14:57:01

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 11/23] xen: do not compile manage, balloon, pci, acpi and cpu_hotplug on ARM

Changes in v2:

- make pci.o depend on CONFIG_PCI and acpi.o depend on CONFIG_ACPI.

Signed-off-by: Stefano Stabellini <[email protected]>
---
drivers/xen/Makefile | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index fc34886..bee02b2 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -1,11 +1,17 @@
-obj-y += grant-table.o features.o events.o manage.o balloon.o
+ifneq ($(CONFIG_ARM),y)
+obj-y += manage.o balloon.o
+obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o
+endif
+obj-y += grant-table.o features.o events.o
obj-y += xenbus/

nostackp := $(call cc-option, -fno-stack-protector)
CFLAGS_features.o := $(nostackp)

+obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
+dom0-$(CONFIG_PCI) := pci.o
+dom0-$(CONFIG_ACPI) := acpi.o
obj-$(CONFIG_BLOCK) += biomerge.o
-obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o
obj-$(CONFIG_XEN_XENCOMM) += xencomm.o
obj-$(CONFIG_XEN_BALLOON) += xen-balloon.o
obj-$(CONFIG_XEN_SELFBALLOONING) += xen-selfballoon.o
@@ -17,7 +23,6 @@ obj-$(CONFIG_XEN_SYS_HYPERVISOR) += sys-hypervisor.o
obj-$(CONFIG_XEN_PVHVM) += platform-pci.o
obj-$(CONFIG_XEN_TMEM) += tmem.o
obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o
-obj-$(CONFIG_XEN_DOM0) += pci.o acpi.o
obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
--
1.7.2.5

2012-08-06 14:58:47

by Stefano Stabellini

[permalink] [raw]
Subject: [PATCH v2 14/23] xen/arm: initialize grant_table on ARM

Initialize the grant table mapping at the address specified at index 0
in the DT under the /xen node.
After the grant table is initialized, call xenbus_probe (if not dom0).

Changes in v2:

- introduce GRANT_TABLE_PHYSADDR;
- remove unneeded initialization of boot_max_nr_grant_frames.

Signed-off-by: Stefano Stabellini <[email protected]>
---
arch/arm/xen/enlighten.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index ac3a2d6..e5e92d5 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -1,8 +1,12 @@
#include <xen/xen.h>
+#include <xen/grant_table.h>
+#include <xen/hvm.h>
#include <xen/interface/xen.h>
#include <xen/interface/memory.h>
+#include <xen/interface/hvm/params.h>
#include <xen/features.h>
#include <xen/platform_pci.h>
+#include <xen/xenbus.h>
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
#include <linux/module.h>
@@ -45,17 +49,23 @@ EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
* - one interrupt for Xen event notifications;
* - one memory region to map the grant_table.
*/
+
+#define GRANT_TABLE_PHYSADDR 0
static int __init xen_guest_init(void)
{
struct xen_add_to_physmap xatp;
static struct shared_info *shared_info_page = 0;
struct device_node *node;
+ struct resource res;

node = of_find_compatible_node(NULL, NULL, "arm,xen");
if (!node) {
pr_debug("No Xen support\n");
return 0;
}
+ if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res))
+ return 0;
+ xen_hvm_resume_frames = res.start >> PAGE_SHIFT;
xen_domain_type = XEN_HVM_DOMAIN;

xen_setup_features();
@@ -89,6 +99,11 @@ static int __init xen_guest_init(void)
* is required to use VCPUOP_register_vcpu_info to place vcpu info
* for secondary CPUs as they are brought up. */
per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
+
+ gnttab_init();
+ if (!xen_initial_domain())
+ xenbus_probe(NULL);
+
return 0;
}
core_initcall(xen_guest_init);
--
1.7.2.5

2012-08-06 16:23:07

by David Vrabel

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH v2 07/23] xen/arm: Xen detection and shared_info page mapping

On 06/08/12 15:27, Stefano Stabellini wrote:
> Check for a "/xen" node in the device tree, if it is present set
> xen_domain_type to XEN_HVM_DOMAIN and continue initialization.
>
> Map the real shared info page using XENMEM_add_to_physmap with
> XENMAPSPACE_shared_info.
>
> Changes in v2:
>
> - replace pr_info with pr_debug.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/xen/enlighten.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index d27c2a6..102d823 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -5,6 +5,9 @@
> #include <asm/xen/hypervisor.h>
> #include <asm/xen/hypercall.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
>
> struct start_info _xen_start_info;
> struct start_info *xen_start_info = &_xen_start_info;
> @@ -33,3 +36,52 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> return -ENOSYS;
> }
> EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
> +
> +/*
> + * == Xen Device Tree format ==
> + * - /xen node;
> + * - compatible "arm,xen";
> + * - one interrupt for Xen event notifications;
> + * - one memory region to map the grant_table.
> + */

These needs to be documented in Documentation/devicetree/bindings/ and
should be sent to the devicetree-discuss mailing list for review.

The node should be called 'hypervisor' I think.

The first word of the compatible string is the vendor/organization that
defined the binding so should be "xen" here. This does give a odd
looking "xen,xen" but we'll have to live with that.

I'd suggest that the DT provided by the hypervisor or tools give the
hypercall ABI version in the compatible string as well. e.g.,

hypervisor {
compatible = "xen,xen-4.3", "xen,xen"
};

I missed the Xen patch that adds this node for dom0. Can you point me
to it?

David

> +static int __init xen_guest_init(void)
> +{
> + struct xen_add_to_physmap xatp;
> + static struct shared_info *shared_info_page = 0;
> + struct device_node *node;
> +
> + node = of_find_compatible_node(NULL, NULL, "arm,xen");
> + if (!node) {
> + pr_debug("No Xen support\n");
> + return 0;
> + }
> + xen_domain_type = XEN_HVM_DOMAIN;
> +
> + if (!shared_info_page)
> + shared_info_page = (struct shared_info *)
> + get_zeroed_page(GFP_KERNEL);
> + if (!shared_info_page) {
> + pr_err("not enough memory\n");
> + return -ENOMEM;
> + }
> + xatp.domid = DOMID_SELF;
> + xatp.idx = 0;
> + xatp.space = XENMAPSPACE_shared_info;
> + xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
> + if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
> + BUG();
> +
> + HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> +
> + /* xen_vcpu is a pointer to the vcpu_info struct in the shared_info
> + * page, we use it in the event channel upcall and in some pvclock
> + * related functions. We don't need the vcpu_info placement
> + * optimizations because we don't use any pv_mmu or pv_irq op on
> + * HVM.
> + * The shared info contains exactly 1 CPU (the boot CPU). The guest
> + * is required to use VCPUOP_register_vcpu_info to place vcpu info
> + * for secondary CPUs as they are brought up. */
> + per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> + return 0;
> +}
> +core_initcall(xen_guest_init);

2012-08-07 18:20:12

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 01/23] arm: initial Xen support

On Mon, Aug 06, 2012 at 03:27:04PM +0100, Stefano Stabellini wrote:
> - Basic hypervisor.h and interface.h definitions.
> - Skeleton enlighten.c, set xen_start_info to an empty struct.
> - Make xen_initial_domain dependent on the SIF_PRIVILIGED_BIT.
>
> The new code only compiles when CONFIG_XEN is set, that is going to be
> added to arch/arm/Kconfig in patch #11 "xen/arm: introduce CONFIG_XEN on
> ARM".

You can add my Ack, but do one change pls:

> +/* XXX: Move pvclock definitions some place arch independent */

Just use 'TODO'

> +struct pvclock_vcpu_time_info {
> + u32 version;
> + u32 pad0;
> + u64 tsc_timestamp;
> + u64 system_time;
> + u32 tsc_to_system_mul;
> + s8 tsc_shift;
> + u8 flags;
> + u8 pad[2];
> +} __attribute__((__packed__)); /* 32 bytes */
> +
> +struct pvclock_wall_clock {
> + u32 version;
> + u32 sec;
> + u32 nsec;
> +} __attribute__((__packed__));

Mention the size and why it is OK to have it be a weird
size while the one above is nicely padded.

> +#endif
> +
> +#endif /* _ASM_ARM_XEN_INTERFACE_H */
> diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
> new file mode 100644
> index 0000000..0bad594
> --- /dev/null
> +++ b/arch/arm/xen/Makefile
> @@ -0,0 +1 @@
> +obj-y := enlighten.o
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> new file mode 100644
> index 0000000..d27c2a6
> --- /dev/null
> +++ b/arch/arm/xen/enlighten.c
> @@ -0,0 +1,35 @@
> +#include <xen/xen.h>
> +#include <xen/interface/xen.h>
> +#include <xen/interface/memory.h>
> +#include <xen/platform_pci.h>
> +#include <asm/xen/hypervisor.h>
> +#include <asm/xen/hypercall.h>
> +#include <linux/module.h>
> +
> +struct start_info _xen_start_info;
> +struct start_info *xen_start_info = &_xen_start_info;
> +EXPORT_SYMBOL_GPL(xen_start_info);
> +
> +enum xen_domain_type xen_domain_type = XEN_NATIVE;
> +EXPORT_SYMBOL_GPL(xen_domain_type);
> +
> +struct shared_info xen_dummy_shared_info;
> +struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
> +
> +DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
> +
> +/* XXX: to be removed */

s/XXX/TODO/ here, and mention pls why it needs to be removed.

> +__read_mostly int xen_have_vector_callback;
> +EXPORT_SYMBOL_GPL(xen_have_vector_callback);
> +
> +int xen_platform_pci_unplug = XEN_UNPLUG_ALL;
> +EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);

2012-08-07 18:23:08

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 03/23] xen/arm: page.h definitions

On Mon, Aug 06, 2012 at 03:27:06PM +0100, Stefano Stabellini wrote:
> ARM Xen guests always use paging in hardware, like PV on HVM guests in
> the X86 world.
>
> Signed-off-by: Stefano Stabellini <[email protected]>

Ack.. with one nitpick

> +/* XXX: this shouldn't be here */

.. but its here b/c the frontend drivers are using it (its rolled in
headers)- even though we won't hit the code path. So for right now
just punt with this.

> +static inline pte_t *lookup_address(unsigned long address, unsigned int *level)
> +{
> + BUG();
> + return NULL;
> +}
> +
> +static inline int m2p_add_override(unsigned long mfn, struct page *page,
> + struct gnttab_map_grant_ref *kmap_op)
> +{
> + return 0;
> +}
> +
> +static inline int m2p_remove_override(struct page *page, bool clear_pte)
> +{
> + return 0;
> +}
> +
> +static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
> +{
> + BUG();
> + return false;
> +}
> +#endif /* _ASM_ARM_XEN_PAGE_H */
> --
> 1.7.2.5

2012-08-07 18:23:16

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 04/23] xen/arm: sync_bitops

On Mon, Aug 06, 2012 at 03:27:07PM +0100, Stefano Stabellini wrote:
> sync_bitops functions are equivalent to the SMP implementation of the
> original functions, independently from CONFIG_SMP being defined.
>
> We need them because _set_bit etc are not SMP safe if !CONFIG_SMP. But
> under Xen you might be communicating with a completely external entity
> who might be on another CPU (e.g. two uniprocessor guests communicating
> via event channels and grant tables). So we need a variant of the bit
> ops which are SMP safe even on a UP kernel.

Ack from me.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/include/asm/sync_bitops.h | 27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/include/asm/sync_bitops.h
>
> diff --git a/arch/arm/include/asm/sync_bitops.h b/arch/arm/include/asm/sync_bitops.h
> new file mode 100644
> index 0000000..63479ee
> --- /dev/null
> +++ b/arch/arm/include/asm/sync_bitops.h
> @@ -0,0 +1,27 @@
> +#ifndef __ASM_SYNC_BITOPS_H__
> +#define __ASM_SYNC_BITOPS_H__
> +
> +#include <asm/bitops.h>
> +#include <asm/system.h>
> +
> +/* sync_bitops functions are equivalent to the SMP implementation of the
> + * original functions, independently from CONFIG_SMP being defined.
> + *
> + * We need them because _set_bit etc are not SMP safe if !CONFIG_SMP. But
> + * under Xen you might be communicating with a completely external entity
> + * who might be on another CPU (e.g. two uniprocessor guests communicating
> + * via event channels and grant tables). So we need a variant of the bit
> + * ops which are SMP safe even on a UP kernel.
> + */
> +
> +#define sync_set_bit(nr, p) _set_bit(nr, p)
> +#define sync_clear_bit(nr, p) _clear_bit(nr, p)
> +#define sync_change_bit(nr, p) _change_bit(nr, p)
> +#define sync_test_and_set_bit(nr, p) _test_and_set_bit(nr, p)
> +#define sync_test_and_clear_bit(nr, p) _test_and_clear_bit(nr, p)
> +#define sync_test_and_change_bit(nr, p) _test_and_change_bit(nr, p)
> +#define sync_test_bit(nr, addr) test_bit(nr, addr)
> +#define sync_cmpxchg cmpxchg
> +
> +
> +#endif
> --
> 1.7.2.5

2012-08-07 18:23:20

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 05/23] xen/arm: empty implementation of grant_table arch specific functions

On Mon, Aug 06, 2012 at 03:27:08PM +0100, Stefano Stabellini wrote:
> Changes in v2:
>
> - return -ENOSYS rather than -1.

Ack.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/xen/Makefile | 2 +-
> arch/arm/xen/grant-table.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 54 insertions(+), 1 deletions(-)
> create mode 100644 arch/arm/xen/grant-table.c
>
> diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
> index b9d6acc..4384103 100644
> --- a/arch/arm/xen/Makefile
> +++ b/arch/arm/xen/Makefile
> @@ -1 +1 @@
> -obj-y := enlighten.o hypercall.o
> +obj-y := enlighten.o hypercall.o grant-table.o
> diff --git a/arch/arm/xen/grant-table.c b/arch/arm/xen/grant-table.c
> new file mode 100644
> index 0000000..dbd1330
> --- /dev/null
> +++ b/arch/arm/xen/grant-table.c
> @@ -0,0 +1,53 @@
> +/******************************************************************************
> + * grant_table.c
> + * ARM specific part
> + *
> + * Granting foreign access to our memory reservation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation; or, when distributed
> + * separately from the Linux kernel or incorporated into other
> + * software packages, subject to the following license:
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this source file (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use, copy, modify,
> + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
> + * and to permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include <xen/interface/xen.h>
> +#include <xen/page.h>
> +#include <xen/grant_table.h>
> +
> +int arch_gnttab_map_shared(unsigned long *frames, unsigned long nr_gframes,
> + unsigned long max_nr_gframes,
> + void **__shared)
> +{
> + return -ENOSYS;
> +}
> +
> +void arch_gnttab_unmap(void *shared, unsigned long nr_gframes)
> +{
> + return;
> +}
> +
> +int arch_gnttab_map_status(uint64_t *frames, unsigned long nr_gframes,
> + unsigned long max_nr_gframes,
> + grant_status_t **__shared)
> +{
> + return -ENOSYS;
> +}
> --
> 1.7.2.5

2012-08-07 18:24:41

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 06/23] xen: missing includes

On Mon, Aug 06, 2012 at 03:27:09PM +0100, Stefano Stabellini wrote:
> Changes in v2:
> - remove pvclock hack;
> - remove include linux/types.h from xen/interface/xen.h.

I think I can take in my tree now right by itself right? Or do
you want to keep this in your patchqueue? If so, Ack from me.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/x86/include/asm/xen/interface.h | 2 ++
> drivers/tty/hvc/hvc_xen.c | 2 ++
> drivers/xen/grant-table.c | 1 +
> drivers/xen/xenbus/xenbus_probe_frontend.c | 1 +
> include/xen/interface/xen.h | 1 -
> include/xen/privcmd.h | 1 +
> 6 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> index cbf0c9d..a93db16 100644
> --- a/arch/x86/include/asm/xen/interface.h
> +++ b/arch/x86/include/asm/xen/interface.h
> @@ -121,6 +121,8 @@ struct arch_shared_info {
> #include "interface_64.h"
> #endif
>
> +#include <asm/pvclock-abi.h>
> +
> #ifndef __ASSEMBLY__
> /*
> * The following is all CPU context. Note that the fpu_ctxt block is filled
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 944eaeb..dc07f56 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -21,6 +21,7 @@
> #include <linux/console.h>
> #include <linux/delay.h>
> #include <linux/err.h>
> +#include <linux/irq.h>
> #include <linux/init.h>
> #include <linux/types.h>
> #include <linux/list.h>
> @@ -35,6 +36,7 @@
> #include <xen/page.h>
> #include <xen/events.h>
> #include <xen/interface/io/console.h>
> +#include <xen/interface/sched.h>
> #include <xen/hvc-console.h>
> #include <xen/xenbus.h>
>
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 0bfc1ef..1d0d95e 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -47,6 +47,7 @@
> #include <xen/interface/memory.h>
> #include <xen/hvc-console.h>
> #include <asm/xen/hypercall.h>
> +#include <asm/xen/interface.h>
>
> #include <asm/pgtable.h>
> #include <asm/sync_bitops.h>
> diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
> index a31b54d..3159a37 100644
> --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> @@ -21,6 +21,7 @@
> #include <xen/xenbus.h>
> #include <xen/events.h>
> #include <xen/page.h>
> +#include <xen/xen.h>
>
> #include <xen/platform_pci.h>
>
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index a890804..3871e47 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -10,7 +10,6 @@
> #define __XEN_PUBLIC_XEN_H__
>
> #include <asm/xen/interface.h>
> -#include <asm/pvclock-abi.h>
>
> /*
> * XEN "SYSTEM CALLS" (a.k.a. HYPERCALLS).
> diff --git a/include/xen/privcmd.h b/include/xen/privcmd.h
> index 17857fb..4d58881 100644
> --- a/include/xen/privcmd.h
> +++ b/include/xen/privcmd.h
> @@ -35,6 +35,7 @@
>
> #include <linux/types.h>
> #include <linux/compiler.h>
> +#include <xen/interface/xen.h>
>
> typedef unsigned long xen_pfn_t;
>
> --
> 1.7.2.5

2012-08-07 18:27:38

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 07/23] xen/arm: Xen detection and shared_info page mapping

On Mon, Aug 06, 2012 at 03:27:10PM +0100, Stefano Stabellini wrote:
> Check for a "/xen" node in the device tree, if it is present set
> xen_domain_type to XEN_HVM_DOMAIN and continue initialization.
>
> Map the real shared info page using XENMEM_add_to_physmap with
> XENMAPSPACE_shared_info.
>
> Changes in v2:
>
> - replace pr_info with pr_debug.

I second what David mentioned. The other thing is that you are going to
need to rebase this on top of v3.5-rc1, as Olaf's patches have changed
the shared_info_page a bit.

>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/xen/enlighten.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index d27c2a6..102d823 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -5,6 +5,9 @@
> #include <asm/xen/hypervisor.h>
> #include <asm/xen/hypercall.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
>
> struct start_info _xen_start_info;
> struct start_info *xen_start_info = &_xen_start_info;
> @@ -33,3 +36,52 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> return -ENOSYS;
> }
> EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
> +
> +/*
> + * == Xen Device Tree format ==
> + * - /xen node;
> + * - compatible "arm,xen";
> + * - one interrupt for Xen event notifications;
> + * - one memory region to map the grant_table.
> + */
> +static int __init xen_guest_init(void)
> +{
> + struct xen_add_to_physmap xatp;
> + static struct shared_info *shared_info_page = 0;
> + struct device_node *node;
> +
> + node = of_find_compatible_node(NULL, NULL, "arm,xen");
> + if (!node) {
> + pr_debug("No Xen support\n");
> + return 0;
> + }
> + xen_domain_type = XEN_HVM_DOMAIN;
> +
> + if (!shared_info_page)
> + shared_info_page = (struct shared_info *)
> + get_zeroed_page(GFP_KERNEL);
> + if (!shared_info_page) {
> + pr_err("not enough memory\n");
> + return -ENOMEM;
> + }
> + xatp.domid = DOMID_SELF;
> + xatp.idx = 0;
> + xatp.space = XENMAPSPACE_shared_info;
> + xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
> + if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
> + BUG();
> +
> + HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> +
> + /* xen_vcpu is a pointer to the vcpu_info struct in the shared_info
> + * page, we use it in the event channel upcall and in some pvclock
> + * related functions. We don't need the vcpu_info placement
> + * optimizations because we don't use any pv_mmu or pv_irq op on
> + * HVM.
> + * The shared info contains exactly 1 CPU (the boot CPU). The guest
> + * is required to use VCPUOP_register_vcpu_info to place vcpu info
> + * for secondary CPUs as they are brought up. */
> + per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
> + return 0;
> +}
> +core_initcall(xen_guest_init);
> --
> 1.7.2.5

2012-08-07 18:28:05

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 08/23] xen/arm: Introduce xen_pfn_t for pfn and mfn types

On Mon, Aug 06, 2012 at 03:27:11PM +0100, Stefano Stabellini wrote:
> All the original Xen headers have xen_pfn_t as mfn and pfn type, however
> when they have been imported in Linux, xen_pfn_t has been replaced with
> unsigned long. That might work for x86 and ia64 but it does not for arm.
> Bring back xen_pfn_t and let each architecture define xen_pfn_t as they
> see fit.

Ack.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/include/asm/xen/interface.h | 4 ++++
> arch/ia64/include/asm/xen/interface.h | 5 ++++-
> arch/x86/include/asm/xen/interface.h | 5 +++++
> include/xen/interface/grant_table.h | 4 ++--
> include/xen/interface/memory.h | 6 +++---
> include/xen/interface/platform.h | 4 ++--
> include/xen/interface/xen.h | 6 +++---
> include/xen/privcmd.h | 2 --
> 8 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
> index ab99270..f904dcc 100644
> --- a/arch/arm/include/asm/xen/interface.h
> +++ b/arch/arm/include/asm/xen/interface.h
> @@ -25,6 +25,9 @@
> } while (0)
>
> #ifndef __ASSEMBLY__
> +/* Explicitly size integers that represent pfns in the interface with
> + * Xen so that we can have one ABI that works for 32 and 64 bit guests. */
> +typedef uint64_t xen_pfn_t;
> /* Guest handles for primitive C types. */
> __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> __DEFINE_GUEST_HANDLE(uint, unsigned int);
> @@ -35,6 +38,7 @@ DEFINE_GUEST_HANDLE(long);
> DEFINE_GUEST_HANDLE(void);
> DEFINE_GUEST_HANDLE(uint64_t);
> DEFINE_GUEST_HANDLE(uint32_t);
> +DEFINE_GUEST_HANDLE(xen_pfn_t);
>
> /* Maximum number of virtual CPUs in multi-processor guests. */
> #define MAX_VIRT_CPUS 1
> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> index 09d5f7f..686464e 100644
> --- a/arch/ia64/include/asm/xen/interface.h
> +++ b/arch/ia64/include/asm/xen/interface.h
> @@ -67,6 +67,10 @@
> #define set_xen_guest_handle(hnd, val) do { (hnd).p = val; } while (0)
>
> #ifndef __ASSEMBLY__
> +/* Explicitly size integers that represent pfns in the public interface
> + * with Xen so that we could have one ABI that works for 32 and 64 bit
> + * guests. */
> +typedef unsigned long xen_pfn_t;
> /* Guest handles for primitive C types. */
> __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> __DEFINE_GUEST_HANDLE(uint, unsigned int);
> @@ -79,7 +83,6 @@ DEFINE_GUEST_HANDLE(void);
> DEFINE_GUEST_HANDLE(uint64_t);
> DEFINE_GUEST_HANDLE(uint32_t);
>
> -typedef unsigned long xen_pfn_t;
> DEFINE_GUEST_HANDLE(xen_pfn_t);
> #define PRI_xen_pfn "lx"
> #endif
> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> index a93db16..555f94d 100644
> --- a/arch/x86/include/asm/xen/interface.h
> +++ b/arch/x86/include/asm/xen/interface.h
> @@ -47,6 +47,10 @@
> #endif
>
> #ifndef __ASSEMBLY__
> +/* Explicitly size integers that represent pfns in the public interface
> + * with Xen so that on ARM we can have one ABI that works for 32 and 64
> + * bit guests. */
> +typedef unsigned long xen_pfn_t;
> /* Guest handles for primitive C types. */
> __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> __DEFINE_GUEST_HANDLE(uint, unsigned int);
> @@ -57,6 +61,7 @@ DEFINE_GUEST_HANDLE(long);
> DEFINE_GUEST_HANDLE(void);
> DEFINE_GUEST_HANDLE(uint64_t);
> DEFINE_GUEST_HANDLE(uint32_t);
> +DEFINE_GUEST_HANDLE(xen_pfn_t);
> #endif
>
> #ifndef HYPERVISOR_VIRT_START
> diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h
> index a17d844..7da811b 100644
> --- a/include/xen/interface/grant_table.h
> +++ b/include/xen/interface/grant_table.h
> @@ -338,7 +338,7 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_dump_table);
> #define GNTTABOP_transfer 4
> struct gnttab_transfer {
> /* IN parameters. */
> - unsigned long mfn;
> + xen_pfn_t mfn;
> domid_t domid;
> grant_ref_t ref;
> /* OUT parameters. */
> @@ -375,7 +375,7 @@ struct gnttab_copy {
> struct {
> union {
> grant_ref_t ref;
> - unsigned long gmfn;
> + xen_pfn_t gmfn;
> } u;
> domid_t domid;
> uint16_t offset;
> diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> index eac3ce1..abbbff0 100644
> --- a/include/xen/interface/memory.h
> +++ b/include/xen/interface/memory.h
> @@ -31,7 +31,7 @@ struct xen_memory_reservation {
> * OUT: GMFN bases of extents that were allocated
> * (NB. This command also updates the mach_to_phys translation table)
> */
> - GUEST_HANDLE(ulong) extent_start;
> + GUEST_HANDLE(xen_pfn_t) extent_start;
>
> /* Number of extents, and size/alignment of each (2^extent_order pages). */
> unsigned long nr_extents;
> @@ -130,7 +130,7 @@ struct xen_machphys_mfn_list {
> * any large discontiguities in the machine address space, 2MB gaps in
> * the machphys table will be represented by an MFN base of zero.
> */
> - GUEST_HANDLE(ulong) extent_start;
> + GUEST_HANDLE(xen_pfn_t) extent_start;
>
> /*
> * Number of extents written to the above array. This will be smaller
> @@ -172,7 +172,7 @@ struct xen_add_to_physmap {
> unsigned long idx;
>
> /* GPFN where the source mapping page should appear. */
> - unsigned long gpfn;
> + xen_pfn_t gpfn;
> };
> DEFINE_GUEST_HANDLE_STRUCT(xen_add_to_physmap);
>
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 486653f..0bea470 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -54,7 +54,7 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> #define XENPF_add_memtype 31
> struct xenpf_add_memtype {
> /* IN variables. */
> - unsigned long mfn;
> + xen_pfn_t mfn;
> uint64_t nr_mfns;
> uint32_t type;
> /* OUT variables. */
> @@ -84,7 +84,7 @@ struct xenpf_read_memtype {
> /* IN variables. */
> uint32_t reg;
> /* OUT variables. */
> - unsigned long mfn;
> + xen_pfn_t mfn;
> uint64_t nr_mfns;
> uint32_t type;
> };
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index 3871e47..42834a3 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -188,7 +188,7 @@ struct mmuext_op {
> unsigned int cmd;
> union {
> /* [UN]PIN_TABLE, NEW_BASEPTR, NEW_USER_BASEPTR */
> - unsigned long mfn;
> + xen_pfn_t mfn;
> /* INVLPG_LOCAL, INVLPG_ALL, SET_LDT */
> unsigned long linear_addr;
> } arg1;
> @@ -428,11 +428,11 @@ struct start_info {
> unsigned long nr_pages; /* Total pages allocated to this domain. */
> unsigned long shared_info; /* MACHINE address of shared info struct. */
> uint32_t flags; /* SIF_xxx flags. */
> - unsigned long store_mfn; /* MACHINE page number of shared page. */
> + xen_pfn_t store_mfn; /* MACHINE page number of shared page. */
> uint32_t store_evtchn; /* Event channel for store communication. */
> union {
> struct {
> - unsigned long mfn; /* MACHINE page number of console page. */
> + xen_pfn_t mfn; /* MACHINE page number of console page. */
> uint32_t evtchn; /* Event channel for console page. */
> } domU;
> struct {
> diff --git a/include/xen/privcmd.h b/include/xen/privcmd.h
> index 4d58881..45c1aa1 100644
> --- a/include/xen/privcmd.h
> +++ b/include/xen/privcmd.h
> @@ -37,8 +37,6 @@
> #include <linux/compiler.h>
> #include <xen/interface/xen.h>
>
> -typedef unsigned long xen_pfn_t;
> -
> struct privcmd_hypercall {
> __u64 op;
> __u64 arg[5];
> --
> 1.7.2.5

2012-08-07 18:28:24

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 09/23] xen/arm: Introduce xen_ulong_t for unsigned long

On Mon, Aug 06, 2012 at 03:27:12PM +0100, Stefano Stabellini wrote:
> All the original Xen headers have xen_ulong_t as unsigned long type, however
> when they have been imported in Linux, xen_ulong_t has been replaced with
> unsigned long. That might work for x86 and ia64 but it does not for arm.
> Bring back xen_ulong_t and let each architecture define xen_ulong_t as they
> see fit.
>
> Also explicitly size pointers (__DEFINE_GUEST_HANDLE) to 64 bit.

Looks ok to me.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/include/asm/xen/interface.h | 8 ++++++--
> arch/ia64/include/asm/xen/interface.h | 1 +
> arch/x86/include/asm/xen/interface.h | 1 +
> include/xen/interface/memory.h | 12 ++++++------
> include/xen/interface/physdev.h | 4 ++--
> include/xen/interface/version.h | 2 +-
> include/xen/interface/xen.h | 6 +++---
> 7 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
> index f904dcc..1d3030c 100644
> --- a/arch/arm/include/asm/xen/interface.h
> +++ b/arch/arm/include/asm/xen/interface.h
> @@ -9,8 +9,11 @@
>
> #include <linux/types.h>
>
> +#define uint64_aligned_t uint64_t __attribute__((aligned(8)))
> +
> #define __DEFINE_GUEST_HANDLE(name, type) \
> - typedef type * __guest_handle_ ## name
> + typedef struct { union { type *p; uint64_aligned_t q; }; } \
> + __guest_handle_ ## name
>
> #define DEFINE_GUEST_HANDLE_STRUCT(name) \
> __DEFINE_GUEST_HANDLE(name, struct name)
> @@ -21,13 +24,14 @@
> do { \
> if (sizeof(hnd) == 8) \
> *(uint64_t *)&(hnd) = 0; \
> - (hnd) = val; \
> + (hnd).p = val; \
> } while (0)
>
> #ifndef __ASSEMBLY__
> /* Explicitly size integers that represent pfns in the interface with
> * Xen so that we can have one ABI that works for 32 and 64 bit guests. */
> typedef uint64_t xen_pfn_t;
> +typedef uint64_t xen_ulong_t;
> /* Guest handles for primitive C types. */
> __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> __DEFINE_GUEST_HANDLE(uint, unsigned int);
> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> index 686464e..7c83445 100644
> --- a/arch/ia64/include/asm/xen/interface.h
> +++ b/arch/ia64/include/asm/xen/interface.h
> @@ -71,6 +71,7 @@
> * with Xen so that we could have one ABI that works for 32 and 64 bit
> * guests. */
> typedef unsigned long xen_pfn_t;
> +typedef unsigned long xen_ulong_t;
> /* Guest handles for primitive C types. */
> __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> __DEFINE_GUEST_HANDLE(uint, unsigned int);
> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> index 555f94d..28fc621 100644
> --- a/arch/x86/include/asm/xen/interface.h
> +++ b/arch/x86/include/asm/xen/interface.h
> @@ -51,6 +51,7 @@
> * with Xen so that on ARM we can have one ABI that works for 32 and 64
> * bit guests. */
> typedef unsigned long xen_pfn_t;
> +typedef unsigned long xen_ulong_t;
> /* Guest handles for primitive C types. */
> __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> __DEFINE_GUEST_HANDLE(uint, unsigned int);
> diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> index abbbff0..b5c3098 100644
> --- a/include/xen/interface/memory.h
> +++ b/include/xen/interface/memory.h
> @@ -34,7 +34,7 @@ struct xen_memory_reservation {
> GUEST_HANDLE(xen_pfn_t) extent_start;
>
> /* Number of extents, and size/alignment of each (2^extent_order pages). */
> - unsigned long nr_extents;
> + xen_ulong_t nr_extents;
> unsigned int extent_order;
>
> /*
> @@ -92,7 +92,7 @@ struct xen_memory_exchange {
> * command will be non-zero.
> * 5. THIS FIELD MUST BE INITIALISED TO ZERO BY THE CALLER!
> */
> - unsigned long nr_exchanged;
> + xen_ulong_t nr_exchanged;
> };
>
> DEFINE_GUEST_HANDLE_STRUCT(xen_memory_exchange);
> @@ -148,8 +148,8 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mfn_list);
> */
> #define XENMEM_machphys_mapping 12
> struct xen_machphys_mapping {
> - unsigned long v_start, v_end; /* Start and end virtual addresses. */
> - unsigned long max_mfn; /* Maximum MFN that can be looked up. */
> + xen_ulong_t v_start, v_end; /* Start and end virtual addresses. */
> + xen_ulong_t max_mfn; /* Maximum MFN that can be looked up. */
> };
> DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mapping_t);
>
> @@ -169,7 +169,7 @@ struct xen_add_to_physmap {
> unsigned int space;
>
> /* Index into source mapping space. */
> - unsigned long idx;
> + xen_ulong_t idx;
>
> /* GPFN where the source mapping page should appear. */
> xen_pfn_t gpfn;
> @@ -186,7 +186,7 @@ struct xen_translate_gpfn_list {
> domid_t domid;
>
> /* Length of list. */
> - unsigned long nr_gpfns;
> + xen_ulong_t nr_gpfns;
>
> /* List of GPFNs to translate. */
> GUEST_HANDLE(ulong) gpfn_list;
> diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h
> index 9ce788d..bc3ae14 100644
> --- a/include/xen/interface/physdev.h
> +++ b/include/xen/interface/physdev.h
> @@ -56,7 +56,7 @@ struct physdev_eoi {
> #define PHYSDEVOP_pirq_eoi_gmfn_v2 28
> struct physdev_pirq_eoi_gmfn {
> /* IN */
> - unsigned long gmfn;
> + xen_ulong_t gmfn;
> };
>
> /*
> @@ -108,7 +108,7 @@ struct physdev_set_iobitmap {
> #define PHYSDEVOP_apic_write 9
> struct physdev_apic {
> /* IN */
> - unsigned long apic_physbase;
> + xen_ulong_t apic_physbase;
> uint32_t reg;
> /* IN or OUT */
> uint32_t value;
> diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
> index e8b6519..30280c9 100644
> --- a/include/xen/interface/version.h
> +++ b/include/xen/interface/version.h
> @@ -45,7 +45,7 @@ struct xen_changeset_info {
>
> #define XENVER_platform_parameters 5
> struct xen_platform_parameters {
> - unsigned long virt_start;
> + xen_ulong_t virt_start;
> };
>
> #define XENVER_get_features 6
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index 42834a3..ec32115 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -274,9 +274,9 @@ DEFINE_GUEST_HANDLE_STRUCT(mmu_update);
> * NB. The fields are natural register size for this architecture.
> */
> struct multicall_entry {
> - unsigned long op;
> - long result;
> - unsigned long args[6];
> + xen_ulong_t op;
> + xen_ulong_t result;
> + xen_ulong_t args[6];
> };
> DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
>
> --
> 1.7.2.5

2012-08-07 18:31:43

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

On Mon, Aug 06, 2012 at 03:27:13PM +0100, Stefano Stabellini wrote:
> bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
> an error.
>
> If Linux is running as an HVM domain and is running as Dom0, use
> xenstored_local_init to initialize the xenstore page and event channel.
>
> Changes in v2:
>
> - refactor xenbus_init.

Thank you. Lets also CC our friend at NSA who has been doing some work
in that area. Daniel are you OK with this change - will it still make
PV initial domain with with the MiniOS XenBus driver?

Thanks.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> drivers/xen/xenbus/xenbus_comms.c | 2 +-
> drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
> drivers/xen/xenbus/xenbus_xs.c | 1 +
> 3 files changed, 45 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
> index 52fe7ad..c5aa55c 100644
> --- a/drivers/xen/xenbus/xenbus_comms.c
> +++ b/drivers/xen/xenbus/xenbus_comms.c
> @@ -224,7 +224,7 @@ int xb_init_comms(void)
> int err;
> err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
> 0, "xenbus", &xb_waitq);
> - if (err <= 0) {
> + if (err < 0) {
> printk(KERN_ERR "XENBUS request irq failed %i\n", err);
> return err;
> }

> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> index b793723..a67ccc0 100644
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
> return err;
> }
>
> +enum xenstore_init {
> + UNKNOWN,
> + PV,
> + HVM,
> + LOCAL,
> +};
> static int __init xenbus_init(void)
> {
> int err = 0;
> + enum xenstore_init usage = UNKNOWN;
> + uint64_t v = 0;
>
> if (!xen_domain())
> return -ENODEV;
>
> xenbus_ring_ops_init();
>
> - if (xen_hvm_domain()) {
> - uint64_t v = 0;
> - err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
> - if (err)
> - goto out_error;
> - xen_store_evtchn = (int)v;
> - err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
> - if (err)
> - goto out_error;
> - xen_store_mfn = (unsigned long)v;
> - xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
> - } else {
> - xen_store_evtchn = xen_start_info->store_evtchn;
> - xen_store_mfn = xen_start_info->store_mfn;
> - if (xen_store_evtchn)
> - xenstored_ready = 1;
> - else {
> + if (xen_pv_domain())
> + usage = PV;
> + if (xen_hvm_domain())
> + usage = HVM;
> + if (xen_hvm_domain() && xen_initial_domain())
> + usage = LOCAL;
> + if (xen_pv_domain() && !xen_start_info->store_evtchn)
> + usage = LOCAL;
> + if (xen_pv_domain() && xen_start_info->store_evtchn)
> + xenstored_ready = 1;
> +
> + switch (usage) {
> + case LOCAL:
> err = xenstored_local_init();
> if (err)
> goto out_error;
> - }
> - xen_store_interface = mfn_to_virt(xen_store_mfn);
> + xen_store_interface = mfn_to_virt(xen_store_mfn);
> + break;
> + case PV:
> + xen_store_evtchn = xen_start_info->store_evtchn;
> + xen_store_mfn = xen_start_info->store_mfn;
> + xen_store_interface = mfn_to_virt(xen_store_mfn);
> + break;
> + case HVM:
> + err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
> + if (err)
> + goto out_error;
> + xen_store_evtchn = (int)v;
> + err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
> + if (err)
> + goto out_error;
> + xen_store_mfn = (unsigned long)v;
> + xen_store_interface =
> + ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
> + break;
> + default:
> + pr_warn("Xenstore state unknown\n");
> + break;
> }
>
> /* Initialize the interface to xenstore. */
> diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
> index d1c217b..f7feb3d 100644
> --- a/drivers/xen/xenbus/xenbus_xs.c
> +++ b/drivers/xen/xenbus/xenbus_xs.c
> @@ -44,6 +44,7 @@
> #include <linux/rwsem.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <asm/xen/hypervisor.h>
> #include <xen/xenbus.h>
> #include <xen/xen.h>
> #include "xenbus_comms.h"
> --
> 1.7.2.5

2012-08-07 18:33:10

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 11/23] xen: do not compile manage, balloon, pci, acpi and cpu_hotplug on ARM

On Mon, Aug 06, 2012 at 03:27:14PM +0100, Stefano Stabellini wrote:
> Changes in v2:
>
> - make pci.o depend on CONFIG_PCI and acpi.o depend on CONFIG_ACPI.
>
> Signed-off-by: Stefano Stabellini <[email protected]>

Looks good.
> ---
> drivers/xen/Makefile | 11 ++++++++---
> 1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index fc34886..bee02b2 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -1,11 +1,17 @@
> -obj-y += grant-table.o features.o events.o manage.o balloon.o
> +ifneq ($(CONFIG_ARM),y)
> +obj-y += manage.o balloon.o
> +obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o
> +endif
> +obj-y += grant-table.o features.o events.o
> obj-y += xenbus/
>
> nostackp := $(call cc-option, -fno-stack-protector)
> CFLAGS_features.o := $(nostackp)
>
> +obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
> +dom0-$(CONFIG_PCI) := pci.o
> +dom0-$(CONFIG_ACPI) := acpi.o
> obj-$(CONFIG_BLOCK) += biomerge.o
> -obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o
> obj-$(CONFIG_XEN_XENCOMM) += xencomm.o
> obj-$(CONFIG_XEN_BALLOON) += xen-balloon.o
> obj-$(CONFIG_XEN_SELFBALLOONING) += xen-selfballoon.o
> @@ -17,7 +23,6 @@ obj-$(CONFIG_XEN_SYS_HYPERVISOR) += sys-hypervisor.o
> obj-$(CONFIG_XEN_PVHVM) += platform-pci.o
> obj-$(CONFIG_XEN_TMEM) += tmem.o
> obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o
> -obj-$(CONFIG_XEN_DOM0) += pci.o acpi.o
> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o
> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> --
> 1.7.2.5

2012-08-07 18:33:26

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] xen/arm: introduce CONFIG_XEN on ARM

On Mon, Aug 06, 2012 at 03:27:15PM +0100, Stefano Stabellini wrote:
> Changes in v2:
>
> - mark Xen guest support on ARM as EXPERIMENTAL.

OK. Looks good.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/Kconfig | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a91009c..f14664b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1855,6 +1855,16 @@ config DEPRECATED_PARAM_STRUCT
> This was deprecated in 2001 and announced to live on for 5 years.
> Some old boot loaders still use this way.
>
> +config XEN_DOM0
> + def_bool y
> +
> +config XEN
> + bool "Xen guest support on ARM (EXPERIMENTAL)"
> + depends on EXPERIMENTAL && ARM && OF
> + select XEN_DOM0
> + help
> + Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
> +
> endmenu
>
> menu "Boot options"
> --
> 1.7.2.5

2012-08-07 18:33:55

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 13/23] xen/arm: get privilege status

On Mon, Aug 06, 2012 at 03:27:16PM +0100, Stefano Stabellini wrote:
> Use Xen features to figure out if we are privileged.
>
> XENFEAT_dom0 was introduced by 23735 in xen-unstable.hg.

Looks good.

>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/xen/enlighten.c | 7 +++++++
> include/xen/interface/features.h | 3 +++
> 2 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 102d823..ac3a2d6 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -1,6 +1,7 @@
> #include <xen/xen.h>
> #include <xen/interface/xen.h>
> #include <xen/interface/memory.h>
> +#include <xen/features.h>
> #include <xen/platform_pci.h>
> #include <asm/xen/hypervisor.h>
> #include <asm/xen/hypercall.h>
> @@ -57,6 +58,12 @@ static int __init xen_guest_init(void)
> }
> xen_domain_type = XEN_HVM_DOMAIN;
>
> + xen_setup_features();
> + if (xen_feature(XENFEAT_dom0))
> + xen_start_info->flags |= SIF_INITDOMAIN|SIF_PRIVILEGED;
> + else
> + xen_start_info->flags &= ~(SIF_INITDOMAIN|SIF_PRIVILEGED);
> +
> if (!shared_info_page)
> shared_info_page = (struct shared_info *)
> get_zeroed_page(GFP_KERNEL);
> diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
> index b6ca39a..131a6cc 100644
> --- a/include/xen/interface/features.h
> +++ b/include/xen/interface/features.h
> @@ -50,6 +50,9 @@
> /* x86: pirq can be used by HVM guests */
> #define XENFEAT_hvm_pirqs 10
>
> +/* operation as Dom0 is supported */
> +#define XENFEAT_dom0 11
> +
> #define XENFEAT_NR_SUBMAPS 1
>
> #endif /* __XEN_PUBLIC_FEATURES_H__ */
> --
> 1.7.2.5

2012-08-07 18:40:37

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 15/23] xen/arm: receive Xen events on ARM

On Mon, Aug 06, 2012 at 03:27:18PM +0100, Stefano Stabellini wrote:
> Compile events.c on ARM.
> Parse, map and enable the IRQ to get event notifications from the device
> tree (node "/xen").
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/include/asm/xen/events.h | 18 ++++++++++++++++++
> arch/arm/xen/enlighten.c | 33 +++++++++++++++++++++++++++++++++
> arch/x86/xen/enlighten.c | 1 +
> arch/x86/xen/irq.c | 1 +
> arch/x86/xen/xen-ops.h | 1 -
> drivers/xen/events.c | 17 ++++++++++++++---
> include/xen/events.h | 2 ++
> 7 files changed, 69 insertions(+), 4 deletions(-)
> create mode 100644 arch/arm/include/asm/xen/events.h
>
> diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> new file mode 100644
> index 0000000..94b4e90
> --- /dev/null
> +++ b/arch/arm/include/asm/xen/events.h
> @@ -0,0 +1,18 @@
> +#ifndef _ASM_ARM_XEN_EVENTS_H
> +#define _ASM_ARM_XEN_EVENTS_H
> +
> +#include <asm/ptrace.h>
> +
> +enum ipi_vector {
> + XEN_PLACEHOLDER_VECTOR,
> +
> + /* Xen IPIs go here */
> + XEN_NR_IPIS,
> +};
> +
> +static inline int xen_irqs_disabled(struct pt_regs *regs)
> +{
> + return raw_irqs_disabled_flags(regs->ARM_cpsr);
> +}
> +
> +#endif /* _ASM_ARM_XEN_EVENTS_H */
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index e5e92d5..87b17f0 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -1,4 +1,5 @@
> #include <xen/xen.h>
> +#include <xen/events.h>
> #include <xen/grant_table.h>
> #include <xen/hvm.h>
> #include <xen/interface/xen.h>
> @@ -9,6 +10,8 @@
> #include <xen/xenbus.h>
> #include <asm/xen/hypervisor.h>
> #include <asm/xen/hypercall.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> @@ -33,6 +36,8 @@ EXPORT_SYMBOL_GPL(xen_have_vector_callback);
> int xen_platform_pci_unplug = XEN_UNPLUG_ALL;
> EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
>
> +static __read_mostly int xen_events_irq = -1;
> +

So this is global..
> int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> unsigned long addr,
> unsigned long mfn, int nr,
> @@ -66,6 +71,9 @@ static int __init xen_guest_init(void)
> if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res))
> return 0;
> xen_hvm_resume_frames = res.start >> PAGE_SHIFT;
> + xen_events_irq = irq_of_parse_and_map(node, 0);
> + pr_info("Xen support found, events_irq=%d gnttab_frame_pfn=%lx\n",
> + xen_events_irq, xen_hvm_resume_frames);
> xen_domain_type = XEN_HVM_DOMAIN;
>
> xen_setup_features();
> @@ -107,3 +115,28 @@ static int __init xen_guest_init(void)
> return 0;
> }
> core_initcall(xen_guest_init);
> +
> +static irqreturn_t xen_arm_callback(int irq, void *arg)
> +{
> + xen_hvm_evtchn_do_upcall();
> + return IRQ_HANDLED;
> +}
> +
> +static int __init xen_init_events(void)
> +{
> + if (!xen_domain() || xen_events_irq < 0)
> + return -ENODEV;
> +
> + xen_init_IRQ();
> +
> + if (request_percpu_irq(xen_events_irq, xen_arm_callback,
> + "events", xen_vcpu)) {

But here you are asking for it to be percpu? What if there are other
interrupts on the _other_ CPUs that conflict with it?
> + pr_err("Error requesting IRQ %d\n", xen_events_irq);
> + return -EINVAL;
> + }
> +
> + enable_percpu_irq(xen_events_irq, 0);

Uh, that is bold. One global to rule them all, eh? Should you make
it at least:
static DEFINE_PER_CPU(int, xen_events_irq);
?

> +
> + return 0;
> +}
> +postcore_initcall(xen_init_events);
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index ff962d4..9f8b0ef 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -33,6 +33,7 @@
> #include <linux/memblock.h>
>
> #include <xen/xen.h>
> +#include <xen/events.h>
> #include <xen/interface/xen.h>
> #include <xen/interface/version.h>
> #include <xen/interface/physdev.h>
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 1573376..01a4dc0 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -5,6 +5,7 @@
> #include <xen/interface/xen.h>
> #include <xen/interface/sched.h>
> #include <xen/interface/vcpu.h>
> +#include <xen/events.h>
>
> #include <asm/xen/hypercall.h>
> #include <asm/xen/hypervisor.h>
> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> index 202d4c1..2368295 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -35,7 +35,6 @@ void xen_set_pat(u64);
>
> char * __init xen_memory_setup(void);
> void __init xen_arch_setup(void);
> -void __init xen_init_IRQ(void);
> void xen_enable_sysenter(void);
> void xen_enable_syscall(void);
> void xen_vcpu_restore(void);
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 7595581..5ecb596 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -31,14 +31,16 @@
> #include <linux/irqnr.h>
> #include <linux/pci.h>
>
> +#ifdef CONFIG_X86
> #include <asm/desc.h>
> #include <asm/ptrace.h>
> #include <asm/irq.h>
> #include <asm/idle.h>
> #include <asm/io_apic.h>
> -#include <asm/sync_bitops.h>
> #include <asm/xen/page.h>
> #include <asm/xen/pci.h>
> +#endif
> +#include <asm/sync_bitops.h>
> #include <asm/xen/hypercall.h>
> #include <asm/xen/hypervisor.h>
>
> @@ -50,6 +52,9 @@
> #include <xen/interface/event_channel.h>
> #include <xen/interface/hvm/hvm_op.h>
> #include <xen/interface/hvm/params.h>
> +#include <xen/interface/physdev.h>
> +#include <xen/interface/sched.h>
> +#include <asm/hw_irq.h>
>
> /*
> * This lock protects updates to the following mapping and reference-count
> @@ -1374,7 +1379,9 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
> {
> struct pt_regs *old_regs = set_irq_regs(regs);
>
> +#ifdef CONFIG_X86

Please explain this with a comment.

> exit_idle();
> +#endif
> irq_enter();
>
> __xen_evtchn_do_upcall();
> @@ -1783,9 +1790,9 @@ void xen_callback_vector(void)
> void xen_callback_vector(void) {}
> #endif
>
> -void __init xen_init_IRQ(void)
> +void xen_init_IRQ(void)
> {
> - int i, rc;
> + int i;
>
> evtchn_to_irq = kcalloc(NR_EVENT_CHANNELS, sizeof(*evtchn_to_irq),
> GFP_KERNEL);
> @@ -1801,6 +1808,7 @@ void __init xen_init_IRQ(void)
>
> pirq_needs_eoi = pirq_needs_eoi_flag;
>
> +#ifdef CONFIG_X86
> if (xen_hvm_domain()) {
> xen_callback_vector();
> native_init_IRQ();
> @@ -1808,6 +1816,7 @@ void __init xen_init_IRQ(void)
> * __acpi_register_gsi can point at the right function */
> pci_xen_hvm_init();
> } else {
> + int rc;
> struct physdev_pirq_eoi_gmfn eoi_gmfn;
>
> irq_ctx_init(smp_processor_id());
> @@ -1823,4 +1832,6 @@ void __init xen_init_IRQ(void)
> } else
> pirq_needs_eoi = pirq_check_eoi_map;
> }
> +#endif
> }
> +EXPORT_SYMBOL_GPL(xen_init_IRQ);
> diff --git a/include/xen/events.h b/include/xen/events.h
> index 04399b2..c6bfe01 100644
> --- a/include/xen/events.h
> +++ b/include/xen/events.h
> @@ -109,4 +109,6 @@ int xen_irq_from_gsi(unsigned gsi);
> /* Determine whether to ignore this IRQ if it is passed to a guest. */
> int xen_test_irq_shared(int irq);
>
> +/* initialize Xen IRQ subsystem */
> +void xen_init_IRQ(void);
> #endif /* _XEN_EVENTS_H */
> --
> 1.7.2.5

2012-08-07 18:41:07

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 16/23] xen: clear IRQ_NOAUTOEN and IRQ_NOREQUEST

On Mon, Aug 06, 2012 at 03:27:19PM +0100, Stefano Stabellini wrote:
> Reset the IRQ_NOAUTOEN and IRQ_NOREQUEST flags that are enabled by
> default on ARM. If IRQ_NOAUTOEN is set, __setup_irq doesn't call
> irq_startup, that is responsible for calling irq_unmask at startup time.
> As a result event channels remain masked.

Acked by me.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> drivers/xen/events.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 5ecb596..8ffb7b7 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -836,6 +836,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)
> struct irq_info *info = info_for_irq(irq);
> WARN_ON(info == NULL || info->type != IRQT_EVTCHN);
> }
> + irq_clear_status_flags(irq, IRQ_NOREQUEST|IRQ_NOAUTOEN);
>
> out:
> mutex_unlock(&irq_mapping_update_lock);
> --
> 1.7.2.5

2012-08-07 18:41:17

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 17/23] xen/arm: implement alloc/free_xenballooned_pages with alloc_pages/kfree

On Mon, Aug 06, 2012 at 03:27:20PM +0100, Stefano Stabellini wrote:
> Only until we get the balloon driver to work.

OK. Acked-by be.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/xen/enlighten.c | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 87b17f0..c244583 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -140,3 +140,21 @@ static int __init xen_init_events(void)
> return 0;
> }
> postcore_initcall(xen_init_events);
> +
> +/* XXX: only until balloon is properly working */
> +int alloc_xenballooned_pages(int nr_pages, struct page **pages, bool highmem)
> +{
> + *pages = alloc_pages(highmem ? GFP_HIGHUSER : GFP_KERNEL,
> + get_order(nr_pages));
> + if (*pages == NULL)
> + return -ENOMEM;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(alloc_xenballooned_pages);
> +
> +void free_xenballooned_pages(int nr_pages, struct page **pages)
> +{
> + kfree(*pages);
> + *pages = NULL;
> +}
> +EXPORT_SYMBOL_GPL(free_xenballooned_pages);
> --
> 1.7.2.5

2012-08-07 18:41:37

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 18/23] xen: allow privcmd for HVM guests

On Mon, Aug 06, 2012 at 03:27:21PM +0100, Stefano Stabellini wrote:
> This patch removes the "return -ENOSYS" for auto_translated_physmap
> guests from privcmd_mmap, thus it allows ARM guests to issue privcmd
> mmap calls. However privcmd mmap calls are still going to fail for HVM
> and hybrid guests on x86 because the xen_remap_domain_mfn_range
> implementation is currently PV only.

Looks good. Ack from me.
>
> Changes in v2:
>
> - better commit message;
> - return -EINVAL from xen_remap_domain_mfn_range if
> auto_translated_physmap.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/x86/xen/mmu.c | 3 +++
> drivers/xen/privcmd.c | 4 ----
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 3a73785..885a223 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -2310,6 +2310,9 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> unsigned long range;
> int err = 0;
>
> + if (xen_feature(XENFEAT_auto_translated_physmap))
> + return -EINVAL;
> +
> prot = __pgprot(pgprot_val(prot) | _PAGE_IOMAP);
>
> BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_RESERVED | VM_IO)) ==
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index ccee0f1..85226cb 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -380,10 +380,6 @@ static struct vm_operations_struct privcmd_vm_ops = {
>
> static int privcmd_mmap(struct file *file, struct vm_area_struct *vma)
> {
> - /* Unsupported for auto-translate guests. */
> - if (xen_feature(XENFEAT_auto_translated_physmap))
> - return -ENOSYS;
> -
> /* 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;
> --
> 1.7.2.5

2012-08-07 18:41:51

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 19/23] xen/arm: compile blkfront and blkback

On Mon, Aug 06, 2012 at 03:27:22PM +0100, Stefano Stabellini wrote:

OK. Looks good.
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> drivers/block/xen-blkback/blkback.c | 1 +
> include/xen/interface/io/protocols.h | 3 +++
> 2 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index 73f196c..63dd5b9 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -42,6 +42,7 @@
>
> #include <xen/events.h>
> #include <xen/page.h>
> +#include <xen/xen.h>
> #include <asm/xen/hypervisor.h>
> #include <asm/xen/hypercall.h>
> #include "common.h"
> diff --git a/include/xen/interface/io/protocols.h b/include/xen/interface/io/protocols.h
> index 01fc8ae..0eafaf2 100644
> --- a/include/xen/interface/io/protocols.h
> +++ b/include/xen/interface/io/protocols.h
> @@ -5,6 +5,7 @@
> #define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi"
> #define XEN_IO_PROTO_ABI_IA64 "ia64-abi"
> #define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi"
> +#define XEN_IO_PROTO_ABI_ARM "arm-abi"
>
> #if defined(__i386__)
> # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
> @@ -14,6 +15,8 @@
> # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
> #elif defined(__powerpc64__)
> # define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
> +#elif defined(__arm__)
> +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_ARM
> #else
> # error arch fixup needed here
> #endif
> --
> 1.7.2.5

2012-08-07 18:42:34

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 20/23] xen/arm: compile netback

On Mon, Aug 06, 2012 at 03:27:23PM +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <[email protected]>

OK. Looks good.
> ---
> arch/arm/include/asm/xen/hypercall.h | 19 +++++++++++++++++++
> drivers/net/xen-netback/netback.c | 1 +
> drivers/net/xen-netfront.c | 1 +
> 3 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
> index 4ac0624..8a82325 100644
> --- a/arch/arm/include/asm/xen/hypercall.h
> +++ b/arch/arm/include/asm/xen/hypercall.h
> @@ -47,4 +47,23 @@ unsigned long HYPERVISOR_hvm_op(int op, void *arg);
> int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
> int HYPERVISOR_physdev_op(int cmd, void *arg);
>
> +static inline void
> +MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
> + unsigned int new_val, unsigned long flags)
> +{
> + BUG();
> +}
> +
> +static inline void
> +MULTI_mmu_update(struct multicall_entry *mcl, struct mmu_update *req,
> + int count, int *success_count, domid_t domid)
> +{
> + BUG();
> +}
> +
> +static inline int
> +HYPERVISOR_multicall(void *call_list, int nr_calls)
> +{
> + BUG();
> +}
> #endif /* _ASM_ARM_XEN_HYPERCALL_H */
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index f4a6fca..ab4f81c 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -40,6 +40,7 @@
>
> #include <net/tcp.h>
>
> +#include <xen/xen.h>
> #include <xen/events.h>
> #include <xen/interface/memory.h>
>
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 3089990..bf4ba2b 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -43,6 +43,7 @@
> #include <linux/slab.h>
> #include <net/ip.h>
>
> +#include <asm/xen/page.h>
> #include <xen/xen.h>
> #include <xen/xenbus.h>
> #include <xen/events.h>
> --
> 1.7.2.5

2012-08-07 18:43:10

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 21/23] xen: update xen_add_to_physmap interface

On Mon, Aug 06, 2012 at 03:27:24PM +0100, Stefano Stabellini wrote:
> Update struct xen_add_to_physmap to be in sync with Xen's version of the
> structure.
> The size field was introduced by:
>
> changeset: 24164:707d27fe03e7
> user: Jean Guyader <[email protected]>
> date: Fri Nov 18 13:42:08 2011 +0000
> summary: mm: New XENMEM space, XENMAPSPACE_gmfn_range
>
> According to the comment:
>
> "This new field .size is located in the 16 bits padding between .domid
> and .space in struct xen_add_to_physmap to stay compatible with older
> versions."
>
> Changes in v2:

Looks good. Let me take this as in my tree to prep it for Mukesh's patches.

>
> - remove erroneous comment in the commit message.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> include/xen/interface/memory.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> index b5c3098..b66d04c 100644
> --- a/include/xen/interface/memory.h
> +++ b/include/xen/interface/memory.h
> @@ -163,6 +163,9 @@ struct xen_add_to_physmap {
> /* Which domain to change the mapping for. */
> domid_t domid;
>
> + /* Number of pages to go through for gmfn_range */
> + uint16_t size;
> +
> /* Source mapping space. */
> #define XENMAPSPACE_shared_info 0 /* shared info page */
> #define XENMAPSPACE_grant_table 1 /* grant table page */
> --
> 1.7.2.5

2012-08-07 18:49:23

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 23/23] [HACK] xen/arm: implement xen_remap_domain_mfn_range

On Mon, Aug 06, 2012 at 03:27:26PM +0100, Stefano Stabellini wrote:
> From: Ian Campbell <[email protected]>
>
> Do not apply!

OK.
>
> This is a simple, hacky implementation of xen_remap_domain_mfn_range,
> using XENMAPSPACE_gmfn_foreign.
>
> It should use same interface as hybrid x86.

Yeah, Mukesh - can you share your patch here please?
>
> Changes in v2:
>
> - retain binary compatibility in xen_add_to_physmap: use a union.
>
> Signed-off-by: Ian Campbell <[email protected]>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/xen/enlighten.c | 79 +++++++++++++++++++++++++++++++++++++++-
> drivers/xen/privcmd.c | 16 +++++----
> drivers/xen/xenfs/super.c | 7 ++++
> include/xen/interface/memory.h | 15 ++++++--
> 4 files changed, 105 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index c244583..20ca1e4 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -16,6 +16,10 @@
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <linux/of_address.h>
> +#include <linux/mm.h>
> +#include <linux/ioport.h>
> +
> +#include <asm/pgtable.h>
>
> struct start_info _xen_start_info;
> struct start_info *xen_start_info = &_xen_start_info;
> @@ -38,12 +42,85 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
>
> static __read_mostly int xen_events_irq = -1;
>
> +#define FOREIGN_MAP_BUFFER 0x90000000UL
> +#define FOREIGN_MAP_BUFFER_SIZE 0x10000000UL
> +struct resource foreign_map_resource = {
> + .start = FOREIGN_MAP_BUFFER,
> + .end = FOREIGN_MAP_BUFFER + FOREIGN_MAP_BUFFER_SIZE,
> + .name = "Xen foreign map buffer",
> + .flags = 0,
> +};
> +
> +static unsigned long foreign_map_buffer_pfn = FOREIGN_MAP_BUFFER >> PAGE_SHIFT;
> +
> +struct remap_data {
> + struct mm_struct *mm;
> + unsigned long mfn;
> + pgprot_t prot;
> +};
> +
> +static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
> + unsigned long addr, void *data)
> +{
> + struct remap_data *rmd = data;
> + pte_t pte = pfn_pte(rmd->mfn, rmd->prot);
> +
> + if (rmd->mfn < 0x90010)
> + pr_crit("%s: ptep %p addr %#lx => %#x / %#lx\n",
> + __func__, ptep, addr, pte_val(pte), rmd->mfn);
> +
> + set_pte_at(rmd->mm, addr, ptep, pte);
> +
> + rmd->mfn++;
> + return 0;
> +}
> +
> int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> unsigned long addr,
> unsigned long mfn, int nr,
> pgprot_t prot, unsigned domid)
> {
> - return -ENOSYS;
> + int i, rc = 0;
> + struct remap_data rmd = {
> + .mm = vma->vm_mm,
> + .prot = prot,
> + };
> + struct xen_add_to_physmap xatp = {
> + .domid = DOMID_SELF,
> + .space = XENMAPSPACE_gmfn_foreign,
> +
> + .foreign_domid = domid,
> + };
> +
> + if (foreign_map_buffer_pfn + nr > ((FOREIGN_MAP_BUFFER +
> + FOREIGN_MAP_BUFFER_SIZE)>>PAGE_SHIFT)) {
> + pr_crit("RAM out of foreign map buffers...\n");
> + return -EBUSY;
> + }
> +
> + for (i = 0; i < nr; i++) {
> + xatp.idx = mfn + i;
> + xatp.gpfn = foreign_map_buffer_pfn + i;
> + rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
> + if (rc != 0) {
> + pr_crit("foreign map add_to_physmap failed, err=%d\n", rc);
> + goto out;
> + }
> + }
> +
> + rmd.mfn = foreign_map_buffer_pfn;
> + rc = apply_to_page_range(vma->vm_mm,
> + addr,
> + (unsigned long)nr << PAGE_SHIFT,
> + remap_area_mfn_pte_fn, &rmd);
> + if (rc != 0) {
> + pr_crit("apply_to_page_range failed rc=%d\n", rc);
> + goto out;
> + }
> +
> + foreign_map_buffer_pfn += nr;
> +out:
> + return rc;
> }
> EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
>
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 85226cb..3e15c22 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -20,6 +20,8 @@
> #include <linux/pagemap.h>
> #include <linux/seq_file.h>
> #include <linux/miscdevice.h>
> +#include <linux/resource.h>
> +#include <linux/ioport.h>
>
> #include <asm/pgalloc.h>
> #include <asm/pgtable.h>
> @@ -196,9 +198,6 @@ static long privcmd_ioctl_mmap(void __user *udata)
> LIST_HEAD(pagelist);
> struct mmap_mfn_state state;
>
> - if (!xen_initial_domain())
> - return -EPERM;
> -
> if (copy_from_user(&mmapcmd, udata, sizeof(mmapcmd)))
> return -EFAULT;
>
> @@ -286,9 +285,6 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
> LIST_HEAD(pagelist);
> struct mmap_batch_state state;
>
> - if (!xen_initial_domain())
> - return -EPERM;
> -
> if (copy_from_user(&m, udata, sizeof(m)))
> return -EFAULT;
>
> @@ -365,6 +361,11 @@ static long privcmd_ioctl(struct file *file,
> return ret;
> }
>
> +static void privcmd_close(struct vm_area_struct *vma)
> +{
> + /* TODO: unmap VMA */
> +}
> +
> static int privcmd_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> {
> printk(KERN_DEBUG "privcmd_fault: vma=%p %lx-%lx, pgoff=%lx, uv=%p\n",
> @@ -375,7 +376,8 @@ static int privcmd_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> }
>
> static struct vm_operations_struct privcmd_vm_ops = {
> - .fault = privcmd_fault
> + .fault = privcmd_fault,
> + .close = privcmd_close,
> };
>
> static int privcmd_mmap(struct file *file, struct vm_area_struct *vma)
> diff --git a/drivers/xen/xenfs/super.c b/drivers/xen/xenfs/super.c
> index a84b53c..edbe22f 100644
> --- a/drivers/xen/xenfs/super.c
> +++ b/drivers/xen/xenfs/super.c
> @@ -12,6 +12,7 @@
> #include <linux/module.h>
> #include <linux/fs.h>
> #include <linux/magic.h>
> +#include <linux/ioport.h>
>
> #include <xen/xen.h>
>
> @@ -80,6 +81,8 @@ static const struct file_operations capabilities_file_ops = {
> .llseek = default_llseek,
> };
>
> +extern struct resource foreign_map_resource;
> +
> static int xenfs_fill_super(struct super_block *sb, void *data, int silent)
> {
> static struct tree_descr xenfs_files[] = {
> @@ -100,6 +103,10 @@ static int xenfs_fill_super(struct super_block *sb, void *data, int silent)
> &xsd_kva_file_ops, NULL, S_IRUSR|S_IWUSR);
> xenfs_create_file(sb, sb->s_root, "xsd_port",
> &xsd_port_file_ops, NULL, S_IRUSR|S_IWUSR);
> + rc = request_resource(&iomem_resource, &foreign_map_resource);
> + if (rc < 0)
> + pr_crit("failed to register foreign map resource\n");
> + rc = 0; /* ignore */
> }
>
> return rc;
> diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> index b66d04c..dd2ffe0 100644
> --- a/include/xen/interface/memory.h
> +++ b/include/xen/interface/memory.h
> @@ -163,12 +163,19 @@ struct xen_add_to_physmap {
> /* Which domain to change the mapping for. */
> domid_t domid;
>
> - /* Number of pages to go through for gmfn_range */
> - uint16_t size;
> + union {
> + /* Number of pages to go through for gmfn_range */
> + uint16_t size;
> + /* IFF gmfn_foreign */
> + domid_t foreign_domid;
> + };
>
> /* Source mapping space. */
> -#define XENMAPSPACE_shared_info 0 /* shared info page */
> -#define XENMAPSPACE_grant_table 1 /* grant table page */
> +#define XENMAPSPACE_shared_info 0 /* shared info page */
> +#define XENMAPSPACE_grant_table 1 /* grant table page */
> +#define XENMAPSPACE_gmfn 2 /* GMFN */
> +#define XENMAPSPACE_gmfn_range 3 /* GMFN range */
> +#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
> unsigned int space;
>
> /* Index into source mapping space. */
> --
> 1.7.2.5

2012-08-07 19:00:08

by Daniel De Graaf

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

On 08/07/2012 02:21 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:27:13PM +0100, Stefano Stabellini wrote:
>> bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
>> an error.
>>
>> If Linux is running as an HVM domain and is running as Dom0, use
>> xenstored_local_init to initialize the xenstore page and event channel.
>>
>> Changes in v2:
>>
>> - refactor xenbus_init.
>
> Thank you. Lets also CC our friend at NSA who has been doing some work
> in that area. Daniel are you OK with this change - will it still make
> PV initial domain with with the MiniOS XenBus driver?
>
> Thanks.

That case will work, but what this will break is launching the initial domain
with a Xenstore stub domain already running (see below).

>>
>> Signed-off-by: Stefano Stabellini <[email protected]>
>> ---
>> drivers/xen/xenbus/xenbus_comms.c | 2 +-
>> drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
>> drivers/xen/xenbus/xenbus_xs.c | 1 +
>> 3 files changed, 45 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
>> index 52fe7ad..c5aa55c 100644
>> --- a/drivers/xen/xenbus/xenbus_comms.c
>> +++ b/drivers/xen/xenbus/xenbus_comms.c
>> @@ -224,7 +224,7 @@ int xb_init_comms(void)
>> int err;
>> err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
>> 0, "xenbus", &xb_waitq);
>> - if (err <= 0) {
>> + if (err < 0) {
>> printk(KERN_ERR "XENBUS request irq failed %i\n", err);
>> return err;
>> }
>
>> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
>> index b793723..a67ccc0 100644
>> --- a/drivers/xen/xenbus/xenbus_probe.c
>> +++ b/drivers/xen/xenbus/xenbus_probe.c
>> @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
>> return err;
>> }
>>
>> +enum xenstore_init {
>> + UNKNOWN,
>> + PV,
>> + HVM,
>> + LOCAL,
>> +};
>> static int __init xenbus_init(void)
>> {
>> int err = 0;
>> + enum xenstore_init usage = UNKNOWN;
>> + uint64_t v = 0;
>>
>> if (!xen_domain())
>> return -ENODEV;
>>
>> xenbus_ring_ops_init();
>>
>> - if (xen_hvm_domain()) {
>> - uint64_t v = 0;
>> - err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
>> - if (err)
>> - goto out_error;
>> - xen_store_evtchn = (int)v;
>> - err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
>> - if (err)
>> - goto out_error;
>> - xen_store_mfn = (unsigned long)v;
>> - xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
>> - } else {
>> - xen_store_evtchn = xen_start_info->store_evtchn;
>> - xen_store_mfn = xen_start_info->store_mfn;
>> - if (xen_store_evtchn)
>> - xenstored_ready = 1;
>> - else {
>> + if (xen_pv_domain())
>> + usage = PV;
>> + if (xen_hvm_domain())
>> + usage = HVM;

The above is correct for domUs, and is overridden for dom0s:

>> + if (xen_hvm_domain() && xen_initial_domain())
>> + usage = LOCAL;
>> + if (xen_pv_domain() && !xen_start_info->store_evtchn)
>> + usage = LOCAL;

Instead of these checks, I think it should just be:

if (!xen_start_info->store_evtchn)
usage = LOCAL;

Any domain started after xenstore will have store_evtchn set, so if you don't
have this set, you are either going to be running xenstore locally, or will
use the ioctl to change it later (and so should still set up everything as if
it will be running locally).

>> + if (xen_pv_domain() && xen_start_info->store_evtchn)
>> + xenstored_ready = 1;

This part can now just be moved unconditionally into case PV.

>> +
>> + switch (usage) {
>> + case LOCAL:
>> err = xenstored_local_init();
>> if (err)
>> goto out_error;
>> - }
>> - xen_store_interface = mfn_to_virt(xen_store_mfn);
>> + xen_store_interface = mfn_to_virt(xen_store_mfn);
>> + break;
>> + case PV:
>> + xen_store_evtchn = xen_start_info->store_evtchn;
>> + xen_store_mfn = xen_start_info->store_mfn;
>> + xen_store_interface = mfn_to_virt(xen_store_mfn);
>> + break;
>> + case HVM:
>> + err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
>> + if (err)
>> + goto out_error;
>> + xen_store_evtchn = (int)v;
>> + err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
>> + if (err)
>> + goto out_error;
>> + xen_store_mfn = (unsigned long)v;
>> + xen_store_interface =
>> + ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
>> + break;
>> + default:
>> + pr_warn("Xenstore state unknown\n");
>> + break;
>> }
>>
>> /* Initialize the interface to xenstore. */
>> diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
>> index d1c217b..f7feb3d 100644
>> --- a/drivers/xen/xenbus/xenbus_xs.c
>> +++ b/drivers/xen/xenbus/xenbus_xs.c
>> @@ -44,6 +44,7 @@
>> #include <linux/rwsem.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> +#include <asm/xen/hypervisor.h>
>> #include <xen/xenbus.h>
>> #include <xen/xen.h>
>> #include "xenbus_comms.h"
>> --
>> 1.7.2.5


--
Daniel De Graaf
National Security Agency

2012-08-08 12:41:18

by Dave Martin

[permalink] [raw]
Subject: Re: [PATCH v2 02/23] xen/arm: hypercalls

On Mon, Aug 06, 2012 at 03:27:05PM +0100, Stefano Stabellini wrote:
> Use r12 to pass the hypercall number to the hypervisor.
>
> We need a register to pass the hypercall number because we might not
> know it at compile time and HVC only takes an immediate argument.
>
> Among the available registers r12 seems to be the best choice because it
> is defined as "intra-procedure call scratch register".
>
> Use the ISS to pass an hypervisor specific tag.
>
> Changes in v2:
> - define an HYPERCALL macro for 5 arguments hypercall wrappers, even if
> at the moment is unused;
> - use ldm instead of pop;
> - fix up comments.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> ---
> arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++
> arch/arm/xen/Makefile | 2 +-
> arch/arm/xen/hypercall.S | 106 ++++++++++++++++++++++++++++++++++
> 3 files changed, 157 insertions(+), 1 deletions(-)
> create mode 100644 arch/arm/include/asm/xen/hypercall.h
> create mode 100644 arch/arm/xen/hypercall.S

[...]

> diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> new file mode 100644
> index 0000000..074f5ed
> --- /dev/null
> +++ b/arch/arm/xen/hypercall.S
> @@ -0,0 +1,106 @@
> +/******************************************************************************
> + * hypercall.S
> + *
> + * Xen hypercall wrappers
> + *
> + * Stefano Stabellini <[email protected]>, Citrix, 2012
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation; or, when distributed
> + * separately from the Linux kernel or incorporated into other
> + * software packages, subject to the following license:
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this source file (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use, copy, modify,
> + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
> + * and to permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/*
> + * The Xen hypercall calling convention is very similar to the ARM
> + * procedure calling convention: the first paramter is passed in r0, the
> + * second in r1, the third in r2 and the fourth in r3. Considering that
> + * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
> + * in r4, differently from the procedure calling convention of using the
> + * stack for that case.
> + *
> + * The hypercall number is passed in r12.
> + *
> + * The return value is in r0.
> + *
> + * The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
> + * hypercall tag.
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +#include <xen/interface/xen.h>
> +
> +
> +/* HVC 0xEA1 */
> +#ifdef CONFIG_THUMB2_KERNEL
> +#define xen_hvc .word 0xf7e08ea1
> +#else
> +#define xen_hvc .word 0xe140ea71
> +#endif

Consider using my opcode injection helpers patch for this (see
separate repost: [PATCH v2 REPOST 0/4] ARM: opcodes: Facilitate custom
opcode injection), assuming that nobody objects to it. This should mean
that the right opcodes get generated when building a kernel for a big-
endian target for example.

I believe the __HVC(imm) macro which I put in <asm/opcodes-virt.h> as an
example should do what you need in this case.

> +
> +#define HYPERCALL_SIMPLE(hypercall) \
> +ENTRY(HYPERVISOR_##hypercall) \
> + mov r12, #__HYPERVISOR_##hypercall; \
> + xen_hvc; \
> + mov pc, lr; \
> +ENDPROC(HYPERVISOR_##hypercall)
> +
> +#define HYPERCALL0 HYPERCALL_SIMPLE
> +#define HYPERCALL1 HYPERCALL_SIMPLE
> +#define HYPERCALL2 HYPERCALL_SIMPLE
> +#define HYPERCALL3 HYPERCALL_SIMPLE
> +#define HYPERCALL4 HYPERCALL_SIMPLE
> +
> +#define HYPERCALL5(hypercall) \
> +ENTRY(HYPERVISOR_##hypercall) \
> + stmdb sp!, {r4} \
> + ldr r4, [sp, #4] \
> + mov r12, #__HYPERVISOR_##hypercall; \
> + xen_hvc \
> + ldm sp!, {r4} \
> + mov pc, lr \
> +ENDPROC(HYPERVISOR_##hypercall)
> +
> + .text
> +
> +HYPERCALL2(xen_version);
> +HYPERCALL3(console_io);
> +HYPERCALL3(grant_table_op);
> +HYPERCALL2(sched_op);
> +HYPERCALL2(event_channel_op);
> +HYPERCALL2(hvm_op);
> +HYPERCALL2(memory_op);
> +HYPERCALL2(physdev_op);
> +
> +ENTRY(privcmd_call)
> + stmdb sp!, {r4}
> + mov r12, r0
> + mov r0, r1
> + mov r1, r2
> + mov r2, r3
> + ldr r3, [sp, #8]
> + ldr r4, [sp, #4]
> + xen_hvc
> + ldm sp!, {r4}
> + mov pc, lr

Note that the preferred entry/exit sequences in such cases are:

stmfd sp!, {r4,lr}
...
ldmfd sp!, {r4,pc}

...but it works either way. I would bother to change it unless you
have other changes to make too.


Cheers
---Dave

2012-08-08 16:25:38

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH v2 07/23] xen/arm: Xen detection and shared_info page mapping

On Mon, 6 Aug 2012, David Vrabel wrote:
> On 06/08/12 15:27, Stefano Stabellini wrote:
> > Check for a "/xen" node in the device tree, if it is present set
> > xen_domain_type to XEN_HVM_DOMAIN and continue initialization.
> >
> > Map the real shared info page using XENMEM_add_to_physmap with
> > XENMAPSPACE_shared_info.
> >
> > Changes in v2:
> >
> > - replace pr_info with pr_debug.
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > arch/arm/xen/enlighten.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 52 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index d27c2a6..102d823 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -5,6 +5,9 @@
> > #include <asm/xen/hypervisor.h>
> > #include <asm/xen/hypercall.h>
> > #include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_address.h>
> >
> > struct start_info _xen_start_info;
> > struct start_info *xen_start_info = &_xen_start_info;
> > @@ -33,3 +36,52 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> > return -ENOSYS;
> > }
> > EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
> > +
> > +/*
> > + * == Xen Device Tree format ==
> > + * - /xen node;
> > + * - compatible "arm,xen";
> > + * - one interrupt for Xen event notifications;
> > + * - one memory region to map the grant_table.
> > + */
>
> These needs to be documented in Documentation/devicetree/bindings/ and
> should be sent to the devicetree-discuss mailing list for review.

That's a good idea.


> The node should be called 'hypervisor' I think.
>
> The first word of the compatible string is the vendor/organization that
> defined the binding so should be "xen" here. This does give a odd
> looking "xen,xen" but we'll have to live with that.
>
> I'd suggest that the DT provided by the hypervisor or tools give the
> hypercall ABI version in the compatible string as well. e.g.,
>
> hypervisor {
> compatible = "xen,xen-4.3", "xen,xen"
> };

It makes sense, I'll do that.


> I missed the Xen patch that adds this node for dom0. Can you point me
> to it?

Nope, you didn't miss it: I don't have a patch for Xen yet.

2012-08-08 16:31:40

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 01/23] arm: initial Xen support

On Tue, 7 Aug 2012, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:27:04PM +0100, Stefano Stabellini wrote:
> > - Basic hypervisor.h and interface.h definitions.
> > - Skeleton enlighten.c, set xen_start_info to an empty struct.
> > - Make xen_initial_domain dependent on the SIF_PRIVILIGED_BIT.
> >
> > The new code only compiles when CONFIG_XEN is set, that is going to be
> > added to arch/arm/Kconfig in patch #11 "xen/arm: introduce CONFIG_XEN on
> > ARM".
>
> You can add my Ack, but do one change pls:

Thanks! I'll make the changes.


> > +/* XXX: Move pvclock definitions some place arch independent */
>
> Just use 'TODO'
>
> > +struct pvclock_vcpu_time_info {
> > + u32 version;
> > + u32 pad0;
> > + u64 tsc_timestamp;
> > + u64 system_time;
> > + u32 tsc_to_system_mul;
> > + s8 tsc_shift;
> > + u8 flags;
> > + u8 pad[2];
> > +} __attribute__((__packed__)); /* 32 bytes */
> > +
> > +struct pvclock_wall_clock {
> > + u32 version;
> > + u32 sec;
> > + u32 nsec;
> > +} __attribute__((__packed__));
>
> Mention the size and why it is OK to have it be a weird
> size while the one above is nicely padded.
>
> > +#endif
> > +
> > +#endif /* _ASM_ARM_XEN_INTERFACE_H */
> > diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
> > new file mode 100644
> > index 0000000..0bad594
> > --- /dev/null
> > +++ b/arch/arm/xen/Makefile
> > @@ -0,0 +1 @@
> > +obj-y := enlighten.o
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > new file mode 100644
> > index 0000000..d27c2a6
> > --- /dev/null
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -0,0 +1,35 @@
> > +#include <xen/xen.h>
> > +#include <xen/interface/xen.h>
> > +#include <xen/interface/memory.h>
> > +#include <xen/platform_pci.h>
> > +#include <asm/xen/hypervisor.h>
> > +#include <asm/xen/hypercall.h>
> > +#include <linux/module.h>
> > +
> > +struct start_info _xen_start_info;
> > +struct start_info *xen_start_info = &_xen_start_info;
> > +EXPORT_SYMBOL_GPL(xen_start_info);
> > +
> > +enum xen_domain_type xen_domain_type = XEN_NATIVE;
> > +EXPORT_SYMBOL_GPL(xen_domain_type);
> > +
> > +struct shared_info xen_dummy_shared_info;
> > +struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
> > +
> > +DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
> > +
> > +/* XXX: to be removed */
>
> s/XXX/TODO/ here, and mention pls why it needs to be removed.
>
> > +__read_mostly int xen_have_vector_callback;
> > +EXPORT_SYMBOL_GPL(xen_have_vector_callback);
> > +
> > +int xen_platform_pci_unplug = XEN_UNPLUG_ALL;
> > +EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
>

2012-08-08 16:33:36

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 03/23] xen/arm: page.h definitions

On Tue, 7 Aug 2012, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:27:06PM +0100, Stefano Stabellini wrote:
> > ARM Xen guests always use paging in hardware, like PV on HVM guests in
> > the X86 world.
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
>
> Ack.. with one nitpick
>
> > +/* XXX: this shouldn't be here */
>
> .. but its here b/c the frontend drivers are using it (its rolled in
> headers)- even though we won't hit the code path. So for right now
> just punt with this.

Yep, I'll do that.


> > +static inline pte_t *lookup_address(unsigned long address, unsigned int *level)
> > +{
> > + BUG();
> > + return NULL;
> > +}
> > +
> > +static inline int m2p_add_override(unsigned long mfn, struct page *page,
> > + struct gnttab_map_grant_ref *kmap_op)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline int m2p_remove_override(struct page *page, bool clear_pte)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
> > +{
> > + BUG();
> > + return false;
> > +}
> > +#endif /* _ASM_ARM_XEN_PAGE_H */
> > --
> > 1.7.2.5
>

2012-08-08 16:38:43

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 06/23] xen: missing includes

On Tue, 7 Aug 2012, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:27:09PM +0100, Stefano Stabellini wrote:
> > Changes in v2:
> > - remove pvclock hack;
> > - remove include linux/types.h from xen/interface/xen.h.
>
> I think I can take in my tree now right by itself right? Or do
> you want to keep this in your patchqueue? If so, Ack from me.

Yep, just go ahead and take the patch, thanks.


> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > arch/x86/include/asm/xen/interface.h | 2 ++
> > drivers/tty/hvc/hvc_xen.c | 2 ++
> > drivers/xen/grant-table.c | 1 +
> > drivers/xen/xenbus/xenbus_probe_frontend.c | 1 +
> > include/xen/interface/xen.h | 1 -
> > include/xen/privcmd.h | 1 +
> > 6 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> > index cbf0c9d..a93db16 100644
> > --- a/arch/x86/include/asm/xen/interface.h
> > +++ b/arch/x86/include/asm/xen/interface.h
> > @@ -121,6 +121,8 @@ struct arch_shared_info {
> > #include "interface_64.h"
> > #endif
> >
> > +#include <asm/pvclock-abi.h>
> > +
> > #ifndef __ASSEMBLY__
> > /*
> > * The following is all CPU context. Note that the fpu_ctxt block is filled
> > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> > index 944eaeb..dc07f56 100644
> > --- a/drivers/tty/hvc/hvc_xen.c
> > +++ b/drivers/tty/hvc/hvc_xen.c
> > @@ -21,6 +21,7 @@
> > #include <linux/console.h>
> > #include <linux/delay.h>
> > #include <linux/err.h>
> > +#include <linux/irq.h>
> > #include <linux/init.h>
> > #include <linux/types.h>
> > #include <linux/list.h>
> > @@ -35,6 +36,7 @@
> > #include <xen/page.h>
> > #include <xen/events.h>
> > #include <xen/interface/io/console.h>
> > +#include <xen/interface/sched.h>
> > #include <xen/hvc-console.h>
> > #include <xen/xenbus.h>
> >
> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > index 0bfc1ef..1d0d95e 100644
> > --- a/drivers/xen/grant-table.c
> > +++ b/drivers/xen/grant-table.c
> > @@ -47,6 +47,7 @@
> > #include <xen/interface/memory.h>
> > #include <xen/hvc-console.h>
> > #include <asm/xen/hypercall.h>
> > +#include <asm/xen/interface.h>
> >
> > #include <asm/pgtable.h>
> > #include <asm/sync_bitops.h>
> > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
> > index a31b54d..3159a37 100644
> > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> > @@ -21,6 +21,7 @@
> > #include <xen/xenbus.h>
> > #include <xen/events.h>
> > #include <xen/page.h>
> > +#include <xen/xen.h>
> >
> > #include <xen/platform_pci.h>
> >
> > diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> > index a890804..3871e47 100644
> > --- a/include/xen/interface/xen.h
> > +++ b/include/xen/interface/xen.h
> > @@ -10,7 +10,6 @@
> > #define __XEN_PUBLIC_XEN_H__
> >
> > #include <asm/xen/interface.h>
> > -#include <asm/pvclock-abi.h>
> >
> > /*
> > * XEN "SYSTEM CALLS" (a.k.a. HYPERCALLS).
> > diff --git a/include/xen/privcmd.h b/include/xen/privcmd.h
> > index 17857fb..4d58881 100644
> > --- a/include/xen/privcmd.h
> > +++ b/include/xen/privcmd.h
> > @@ -35,6 +35,7 @@
> >
> > #include <linux/types.h>
> > #include <linux/compiler.h>
> > +#include <xen/interface/xen.h>
> >
> > typedef unsigned long xen_pfn_t;
> >
> > --
> > 1.7.2.5
>

2012-08-08 16:43:00

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 09/23] xen/arm: Introduce xen_ulong_t for unsigned long

On Tue, 7 Aug 2012, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:27:12PM +0100, Stefano Stabellini wrote:
> > All the original Xen headers have xen_ulong_t as unsigned long type, however
> > when they have been imported in Linux, xen_ulong_t has been replaced with
> > unsigned long. That might work for x86 and ia64 but it does not for arm.
> > Bring back xen_ulong_t and let each architecture define xen_ulong_t as they
> > see fit.
> >
> > Also explicitly size pointers (__DEFINE_GUEST_HANDLE) to 64 bit.
>
> Looks ok to me.

Considering that I'll have to change it a bit in the next version
(remove the apic_physbase and multicall_entry changes), I won't add your
acked-by here just yet.


> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > arch/arm/include/asm/xen/interface.h | 8 ++++++--
> > arch/ia64/include/asm/xen/interface.h | 1 +
> > arch/x86/include/asm/xen/interface.h | 1 +
> > include/xen/interface/memory.h | 12 ++++++------
> > include/xen/interface/physdev.h | 4 ++--
> > include/xen/interface/version.h | 2 +-
> > include/xen/interface/xen.h | 6 +++---
> > 7 files changed, 20 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
> > index f904dcc..1d3030c 100644
> > --- a/arch/arm/include/asm/xen/interface.h
> > +++ b/arch/arm/include/asm/xen/interface.h
> > @@ -9,8 +9,11 @@
> >
> > #include <linux/types.h>
> >
> > +#define uint64_aligned_t uint64_t __attribute__((aligned(8)))
> > +
> > #define __DEFINE_GUEST_HANDLE(name, type) \
> > - typedef type * __guest_handle_ ## name
> > + typedef struct { union { type *p; uint64_aligned_t q; }; } \
> > + __guest_handle_ ## name
> >
> > #define DEFINE_GUEST_HANDLE_STRUCT(name) \
> > __DEFINE_GUEST_HANDLE(name, struct name)
> > @@ -21,13 +24,14 @@
> > do { \
> > if (sizeof(hnd) == 8) \
> > *(uint64_t *)&(hnd) = 0; \
> > - (hnd) = val; \
> > + (hnd).p = val; \
> > } while (0)
> >
> > #ifndef __ASSEMBLY__
> > /* Explicitly size integers that represent pfns in the interface with
> > * Xen so that we can have one ABI that works for 32 and 64 bit guests. */
> > typedef uint64_t xen_pfn_t;
> > +typedef uint64_t xen_ulong_t;
> > /* Guest handles for primitive C types. */
> > __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> > __DEFINE_GUEST_HANDLE(uint, unsigned int);
> > diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> > index 686464e..7c83445 100644
> > --- a/arch/ia64/include/asm/xen/interface.h
> > +++ b/arch/ia64/include/asm/xen/interface.h
> > @@ -71,6 +71,7 @@
> > * with Xen so that we could have one ABI that works for 32 and 64 bit
> > * guests. */
> > typedef unsigned long xen_pfn_t;
> > +typedef unsigned long xen_ulong_t;
> > /* Guest handles for primitive C types. */
> > __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> > __DEFINE_GUEST_HANDLE(uint, unsigned int);
> > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> > index 555f94d..28fc621 100644
> > --- a/arch/x86/include/asm/xen/interface.h
> > +++ b/arch/x86/include/asm/xen/interface.h
> > @@ -51,6 +51,7 @@
> > * with Xen so that on ARM we can have one ABI that works for 32 and 64
> > * bit guests. */
> > typedef unsigned long xen_pfn_t;
> > +typedef unsigned long xen_ulong_t;
> > /* Guest handles for primitive C types. */
> > __DEFINE_GUEST_HANDLE(uchar, unsigned char);
> > __DEFINE_GUEST_HANDLE(uint, unsigned int);
> > diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> > index abbbff0..b5c3098 100644
> > --- a/include/xen/interface/memory.h
> > +++ b/include/xen/interface/memory.h
> > @@ -34,7 +34,7 @@ struct xen_memory_reservation {
> > GUEST_HANDLE(xen_pfn_t) extent_start;
> >
> > /* Number of extents, and size/alignment of each (2^extent_order pages). */
> > - unsigned long nr_extents;
> > + xen_ulong_t nr_extents;
> > unsigned int extent_order;
> >
> > /*
> > @@ -92,7 +92,7 @@ struct xen_memory_exchange {
> > * command will be non-zero.
> > * 5. THIS FIELD MUST BE INITIALISED TO ZERO BY THE CALLER!
> > */
> > - unsigned long nr_exchanged;
> > + xen_ulong_t nr_exchanged;
> > };
> >
> > DEFINE_GUEST_HANDLE_STRUCT(xen_memory_exchange);
> > @@ -148,8 +148,8 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mfn_list);
> > */
> > #define XENMEM_machphys_mapping 12
> > struct xen_machphys_mapping {
> > - unsigned long v_start, v_end; /* Start and end virtual addresses. */
> > - unsigned long max_mfn; /* Maximum MFN that can be looked up. */
> > + xen_ulong_t v_start, v_end; /* Start and end virtual addresses. */
> > + xen_ulong_t max_mfn; /* Maximum MFN that can be looked up. */
> > };
> > DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mapping_t);
> >
> > @@ -169,7 +169,7 @@ struct xen_add_to_physmap {
> > unsigned int space;
> >
> > /* Index into source mapping space. */
> > - unsigned long idx;
> > + xen_ulong_t idx;
> >
> > /* GPFN where the source mapping page should appear. */
> > xen_pfn_t gpfn;
> > @@ -186,7 +186,7 @@ struct xen_translate_gpfn_list {
> > domid_t domid;
> >
> > /* Length of list. */
> > - unsigned long nr_gpfns;
> > + xen_ulong_t nr_gpfns;
> >
> > /* List of GPFNs to translate. */
> > GUEST_HANDLE(ulong) gpfn_list;
> > diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h
> > index 9ce788d..bc3ae14 100644
> > --- a/include/xen/interface/physdev.h
> > +++ b/include/xen/interface/physdev.h
> > @@ -56,7 +56,7 @@ struct physdev_eoi {
> > #define PHYSDEVOP_pirq_eoi_gmfn_v2 28
> > struct physdev_pirq_eoi_gmfn {
> > /* IN */
> > - unsigned long gmfn;
> > + xen_ulong_t gmfn;
> > };
> >
> > /*
> > @@ -108,7 +108,7 @@ struct physdev_set_iobitmap {
> > #define PHYSDEVOP_apic_write 9
> > struct physdev_apic {
> > /* IN */
> > - unsigned long apic_physbase;
> > + xen_ulong_t apic_physbase;
> > uint32_t reg;
> > /* IN or OUT */
> > uint32_t value;
> > diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
> > index e8b6519..30280c9 100644
> > --- a/include/xen/interface/version.h
> > +++ b/include/xen/interface/version.h
> > @@ -45,7 +45,7 @@ struct xen_changeset_info {
> >
> > #define XENVER_platform_parameters 5
> > struct xen_platform_parameters {
> > - unsigned long virt_start;
> > + xen_ulong_t virt_start;
> > };
> >
> > #define XENVER_get_features 6
> > diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> > index 42834a3..ec32115 100644
> > --- a/include/xen/interface/xen.h
> > +++ b/include/xen/interface/xen.h
> > @@ -274,9 +274,9 @@ DEFINE_GUEST_HANDLE_STRUCT(mmu_update);
> > * NB. The fields are natural register size for this architecture.
> > */
> > struct multicall_entry {
> > - unsigned long op;
> > - long result;
> > - unsigned long args[6];
> > + xen_ulong_t op;
> > + xen_ulong_t result;
> > + xen_ulong_t args[6];
> > };
> > DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
> >
> > --
> > 1.7.2.5
>

2012-08-08 16:52:07

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

On Tue, 7 Aug 2012, Daniel De Graaf wrote:
> On 08/07/2012 02:21 PM, Konrad Rzeszutek Wilk wrote:
> > On Mon, Aug 06, 2012 at 03:27:13PM +0100, Stefano Stabellini wrote:
> >> bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
> >> an error.
> >>
> >> If Linux is running as an HVM domain and is running as Dom0, use
> >> xenstored_local_init to initialize the xenstore page and event channel.
> >>
> >> Changes in v2:
> >>
> >> - refactor xenbus_init.
> >
> > Thank you. Lets also CC our friend at NSA who has been doing some work
> > in that area. Daniel are you OK with this change - will it still make
> > PV initial domain with with the MiniOS XenBus driver?
> >
> > Thanks.
>
> That case will work, but what this will break is launching the initial domain
> with a Xenstore stub domain already running (see below).
>
> >>
> >> Signed-off-by: Stefano Stabellini <[email protected]>
> >> ---
> >> drivers/xen/xenbus/xenbus_comms.c | 2 +-
> >> drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
> >> drivers/xen/xenbus/xenbus_xs.c | 1 +
> >> 3 files changed, 45 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
> >> index 52fe7ad..c5aa55c 100644
> >> --- a/drivers/xen/xenbus/xenbus_comms.c
> >> +++ b/drivers/xen/xenbus/xenbus_comms.c
> >> @@ -224,7 +224,7 @@ int xb_init_comms(void)
> >> int err;
> >> err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
> >> 0, "xenbus", &xb_waitq);
> >> - if (err <= 0) {
> >> + if (err < 0) {
> >> printk(KERN_ERR "XENBUS request irq failed %i\n", err);
> >> return err;
> >> }
> >
> >> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> >> index b793723..a67ccc0 100644
> >> --- a/drivers/xen/xenbus/xenbus_probe.c
> >> +++ b/drivers/xen/xenbus/xenbus_probe.c
> >> @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
> >> return err;
> >> }
> >>
> >> +enum xenstore_init {
> >> + UNKNOWN,
> >> + PV,
> >> + HVM,
> >> + LOCAL,
> >> +};
> >> static int __init xenbus_init(void)
> >> {
> >> int err = 0;
> >> + enum xenstore_init usage = UNKNOWN;
> >> + uint64_t v = 0;
> >>
> >> if (!xen_domain())
> >> return -ENODEV;
> >>
> >> xenbus_ring_ops_init();
> >>
> >> - if (xen_hvm_domain()) {
> >> - uint64_t v = 0;
> >> - err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
> >> - if (err)
> >> - goto out_error;
> >> - xen_store_evtchn = (int)v;
> >> - err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
> >> - if (err)
> >> - goto out_error;
> >> - xen_store_mfn = (unsigned long)v;
> >> - xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
> >> - } else {
> >> - xen_store_evtchn = xen_start_info->store_evtchn;
> >> - xen_store_mfn = xen_start_info->store_mfn;
> >> - if (xen_store_evtchn)
> >> - xenstored_ready = 1;
> >> - else {
> >> + if (xen_pv_domain())
> >> + usage = PV;
> >> + if (xen_hvm_domain())
> >> + usage = HVM;
>
> The above is correct for domUs, and is overridden for dom0s:
>
> >> + if (xen_hvm_domain() && xen_initial_domain())
> >> + usage = LOCAL;
> >> + if (xen_pv_domain() && !xen_start_info->store_evtchn)
> >> + usage = LOCAL;
>
> Instead of these checks, I think it should just be:
>
> if (!xen_start_info->store_evtchn)
> usage = LOCAL;
>
> Any domain started after xenstore will have store_evtchn set, so if you don't
> have this set, you are either going to be running xenstore locally, or will
> use the ioctl to change it later (and so should still set up everything as if
> it will be running locally).

That would be wrong for an HVM dom0 domain (at least on ARM), because
we don't have a start_info page at all.


> >> + if (xen_pv_domain() && xen_start_info->store_evtchn)
> >> + xenstored_ready = 1;
>
> This part can now just be moved unconditionally into case PV.

What about:

if (xen_pv_domain())
usage = PV;
if (xen_hvm_domain())
usage = HVM;
if (!xen_store_evtchn)
usage = LOCAL;

and moving xenstored_ready in case PV, like you suggested.

2012-08-08 17:01:44

by Daniel De Graaf

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

On 08/08/2012 12:51 PM, Stefano Stabellini wrote:
> On Tue, 7 Aug 2012, Daniel De Graaf wrote:
>> On 08/07/2012 02:21 PM, Konrad Rzeszutek Wilk wrote:
>>> On Mon, Aug 06, 2012 at 03:27:13PM +0100, Stefano Stabellini wrote:
>>>> bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
>>>> an error.
>>>>
>>>> If Linux is running as an HVM domain and is running as Dom0, use
>>>> xenstored_local_init to initialize the xenstore page and event channel.
>>>>
>>>> Changes in v2:
>>>>
>>>> - refactor xenbus_init.
>>>
>>> Thank you. Lets also CC our friend at NSA who has been doing some work
>>> in that area. Daniel are you OK with this change - will it still make
>>> PV initial domain with with the MiniOS XenBus driver?
>>>
>>> Thanks.
>>
>> That case will work, but what this will break is launching the initial domain
>> with a Xenstore stub domain already running (see below).
>>
>>>>
>>>> Signed-off-by: Stefano Stabellini <[email protected]>
>>>> ---
>>>> drivers/xen/xenbus/xenbus_comms.c | 2 +-
>>>> drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
>>>> drivers/xen/xenbus/xenbus_xs.c | 1 +
>>>> 3 files changed, 45 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
>>>> index 52fe7ad..c5aa55c 100644
>>>> --- a/drivers/xen/xenbus/xenbus_comms.c
>>>> +++ b/drivers/xen/xenbus/xenbus_comms.c
>>>> @@ -224,7 +224,7 @@ int xb_init_comms(void)
>>>> int err;
>>>> err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
>>>> 0, "xenbus", &xb_waitq);
>>>> - if (err <= 0) {
>>>> + if (err < 0) {
>>>> printk(KERN_ERR "XENBUS request irq failed %i\n", err);
>>>> return err;
>>>> }
>>>
>>>> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
>>>> index b793723..a67ccc0 100644
>>>> --- a/drivers/xen/xenbus/xenbus_probe.c
>>>> +++ b/drivers/xen/xenbus/xenbus_probe.c
>>>> @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
>>>> return err;
>>>> }
>>>>
>>>> +enum xenstore_init {
>>>> + UNKNOWN,
>>>> + PV,
>>>> + HVM,
>>>> + LOCAL,
>>>> +};
>>>> static int __init xenbus_init(void)
>>>> {
>>>> int err = 0;
>>>> + enum xenstore_init usage = UNKNOWN;
>>>> + uint64_t v = 0;
>>>>
>>>> if (!xen_domain())
>>>> return -ENODEV;
>>>>
>>>> xenbus_ring_ops_init();
>>>>
>>>> - if (xen_hvm_domain()) {
>>>> - uint64_t v = 0;
>>>> - err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
>>>> - if (err)
>>>> - goto out_error;
>>>> - xen_store_evtchn = (int)v;
>>>> - err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
>>>> - if (err)
>>>> - goto out_error;
>>>> - xen_store_mfn = (unsigned long)v;
>>>> - xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
>>>> - } else {
>>>> - xen_store_evtchn = xen_start_info->store_evtchn;
>>>> - xen_store_mfn = xen_start_info->store_mfn;
>>>> - if (xen_store_evtchn)
>>>> - xenstored_ready = 1;
>>>> - else {
>>>> + if (xen_pv_domain())
>>>> + usage = PV;
>>>> + if (xen_hvm_domain())
>>>> + usage = HVM;
>>
>> The above is correct for domUs, and is overridden for dom0s:
>>
>>>> + if (xen_hvm_domain() && xen_initial_domain())
>>>> + usage = LOCAL;
>>>> + if (xen_pv_domain() && !xen_start_info->store_evtchn)
>>>> + usage = LOCAL;
>>
>> Instead of these checks, I think it should just be:
>>
>> if (!xen_start_info->store_evtchn)
>> usage = LOCAL;
>>
>> Any domain started after xenstore will have store_evtchn set, so if you don't
>> have this set, you are either going to be running xenstore locally, or will
>> use the ioctl to change it later (and so should still set up everything as if
>> it will be running locally).
>
> That would be wrong for an HVM dom0 domain (at least on ARM), because
> we don't have a start_info page at all.
>
>
>>>> + if (xen_pv_domain() && xen_start_info->store_evtchn)
>>>> + xenstored_ready = 1;
>>
>> This part can now just be moved unconditionally into case PV.
>
> What about:
>
> if (xen_pv_domain())
> usage = PV;
> if (xen_hvm_domain())
> usage = HVM;
> if (!xen_store_evtchn)
> usage = LOCAL;
>
> and moving xenstored_ready in case PV, like you suggested.
>

That looks correct, but you'd need to split up the switch statement in
order to populate xen_store_evtchn before that last condition, which
ends up pretty much eliminating the usage variable.

2012-08-08 17:19:37

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

On Wed, 8 Aug 2012, Daniel De Graaf wrote:
> On 08/08/2012 12:51 PM, Stefano Stabellini wrote:
> > On Tue, 7 Aug 2012, Daniel De Graaf wrote:
> >> On 08/07/2012 02:21 PM, Konrad Rzeszutek Wilk wrote:
> >>> On Mon, Aug 06, 2012 at 03:27:13PM +0100, Stefano Stabellini wrote:
> >>>> bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
> >>>> an error.
> >>>>
> >>>> If Linux is running as an HVM domain and is running as Dom0, use
> >>>> xenstored_local_init to initialize the xenstore page and event channel.
> >>>>
> >>>> Changes in v2:
> >>>>
> >>>> - refactor xenbus_init.
> >>>
> >>> Thank you. Lets also CC our friend at NSA who has been doing some work
> >>> in that area. Daniel are you OK with this change - will it still make
> >>> PV initial domain with with the MiniOS XenBus driver?
> >>>
> >>> Thanks.
> >>
> >> That case will work, but what this will break is launching the initial domain
> >> with a Xenstore stub domain already running (see below).
> >>
> >>>>
> >>>> Signed-off-by: Stefano Stabellini <[email protected]>
> >>>> ---
> >>>> drivers/xen/xenbus/xenbus_comms.c | 2 +-
> >>>> drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
> >>>> drivers/xen/xenbus/xenbus_xs.c | 1 +
> >>>> 3 files changed, 45 insertions(+), 20 deletions(-)
> >>>>
> >>>> diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
> >>>> index 52fe7ad..c5aa55c 100644
> >>>> --- a/drivers/xen/xenbus/xenbus_comms.c
> >>>> +++ b/drivers/xen/xenbus/xenbus_comms.c
> >>>> @@ -224,7 +224,7 @@ int xb_init_comms(void)
> >>>> int err;
> >>>> err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
> >>>> 0, "xenbus", &xb_waitq);
> >>>> - if (err <= 0) {
> >>>> + if (err < 0) {
> >>>> printk(KERN_ERR "XENBUS request irq failed %i\n", err);
> >>>> return err;
> >>>> }
> >>>
> >>>> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> >>>> index b793723..a67ccc0 100644
> >>>> --- a/drivers/xen/xenbus/xenbus_probe.c
> >>>> +++ b/drivers/xen/xenbus/xenbus_probe.c
> >>>> @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
> >>>> return err;
> >>>> }
> >>>>
> >>>> +enum xenstore_init {
> >>>> + UNKNOWN,
> >>>> + PV,
> >>>> + HVM,
> >>>> + LOCAL,
> >>>> +};
> >>>> static int __init xenbus_init(void)
> >>>> {
> >>>> int err = 0;
> >>>> + enum xenstore_init usage = UNKNOWN;
> >>>> + uint64_t v = 0;
> >>>>
> >>>> if (!xen_domain())
> >>>> return -ENODEV;
> >>>>
> >>>> xenbus_ring_ops_init();
> >>>>
> >>>> - if (xen_hvm_domain()) {
> >>>> - uint64_t v = 0;
> >>>> - err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
> >>>> - if (err)
> >>>> - goto out_error;
> >>>> - xen_store_evtchn = (int)v;
> >>>> - err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
> >>>> - if (err)
> >>>> - goto out_error;
> >>>> - xen_store_mfn = (unsigned long)v;
> >>>> - xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
> >>>> - } else {
> >>>> - xen_store_evtchn = xen_start_info->store_evtchn;
> >>>> - xen_store_mfn = xen_start_info->store_mfn;
> >>>> - if (xen_store_evtchn)
> >>>> - xenstored_ready = 1;
> >>>> - else {
> >>>> + if (xen_pv_domain())
> >>>> + usage = PV;
> >>>> + if (xen_hvm_domain())
> >>>> + usage = HVM;
> >>
> >> The above is correct for domUs, and is overridden for dom0s:
> >>
> >>>> + if (xen_hvm_domain() && xen_initial_domain())
> >>>> + usage = LOCAL;
> >>>> + if (xen_pv_domain() && !xen_start_info->store_evtchn)
> >>>> + usage = LOCAL;
> >>
> >> Instead of these checks, I think it should just be:
> >>
> >> if (!xen_start_info->store_evtchn)
> >> usage = LOCAL;
> >>
> >> Any domain started after xenstore will have store_evtchn set, so if you don't
> >> have this set, you are either going to be running xenstore locally, or will
> >> use the ioctl to change it later (and so should still set up everything as if
> >> it will be running locally).
> >
> > That would be wrong for an HVM dom0 domain (at least on ARM), because
> > we don't have a start_info page at all.
> >
> >
> >>>> + if (xen_pv_domain() && xen_start_info->store_evtchn)
> >>>> + xenstored_ready = 1;
> >>
> >> This part can now just be moved unconditionally into case PV.
> >
> > What about:
> >
> > if (xen_pv_domain())
> > usage = PV;
> > if (xen_hvm_domain())
> > usage = HVM;
> > if (!xen_store_evtchn)
> > usage = LOCAL;
> >
> > and moving xenstored_ready in case PV, like you suggested.
> >
>
> That looks correct, but you'd need to split up the switch statement in
> order to populate xen_store_evtchn before that last condition, which
> ends up pretty much eliminating the usage variable.

Going back to what you wrote in the previous email, in what way this
patch breaks the case when an initial domain is started after a Xenstore
stub domain?

Assuming that we are talking about a PV initial domain on x86, the
following check

if (xen_pv_domain() && !xen_start_info->store_evtchn)
usage = LOCAL;

will return false (because store_evtchn is set), therefore usage will
remain set to PV.
And the check:

if (xen_pv_domain() && xen_start_info->store_evtchn)
xenstored_ready = 1;

will return true so xenstored_ready is going to be set to 1.

2012-08-08 17:23:18

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 21/23] xen: update xen_add_to_physmap interface

On Tue, 7 Aug 2012, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:27:24PM +0100, Stefano Stabellini wrote:
> > Update struct xen_add_to_physmap to be in sync with Xen's version of the
> > structure.
> > The size field was introduced by:
> >
> > changeset: 24164:707d27fe03e7
> > user: Jean Guyader <[email protected]>
> > date: Fri Nov 18 13:42:08 2011 +0000
> > summary: mm: New XENMEM space, XENMAPSPACE_gmfn_range
> >
> > According to the comment:
> >
> > "This new field .size is located in the 16 bits padding between .domid
> > and .space in struct xen_add_to_physmap to stay compatible with older
> > versions."
> >
> > Changes in v2:
>
> Looks good. Let me take this as in my tree to prep it for Mukesh's patches.

OK.
Beware that patch #23 is going to modify xen_add_to_physmap again to
replace .size with a union.


> > - remove erroneous comment in the commit message.
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > include/xen/interface/memory.h | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> > index b5c3098..b66d04c 100644
> > --- a/include/xen/interface/memory.h
> > +++ b/include/xen/interface/memory.h
> > @@ -163,6 +163,9 @@ struct xen_add_to_physmap {
> > /* Which domain to change the mapping for. */
> > domid_t domid;
> >
> > + /* Number of pages to go through for gmfn_range */
> > + uint16_t size;
> > +
> > /* Source mapping space. */
> > #define XENMAPSPACE_shared_info 0 /* shared info page */
> > #define XENMAPSPACE_grant_table 1 /* grant table page */
> > --
> > 1.7.2.5
>

2012-08-08 17:34:01

by Daniel De Graaf

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

On 08/08/2012 01:19 PM, Stefano Stabellini wrote:
> On Wed, 8 Aug 2012, Daniel De Graaf wrote:
>> On 08/08/2012 12:51 PM, Stefano Stabellini wrote:
>>> On Tue, 7 Aug 2012, Daniel De Graaf wrote:
>>>> On 08/07/2012 02:21 PM, Konrad Rzeszutek Wilk wrote:
>>>>> On Mon, Aug 06, 2012 at 03:27:13PM +0100, Stefano Stabellini wrote:
>>>>>> bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
>>>>>> an error.
>>>>>>
>>>>>> If Linux is running as an HVM domain and is running as Dom0, use
>>>>>> xenstored_local_init to initialize the xenstore page and event channel.
>>>>>>
>>>>>> Changes in v2:
>>>>>>
>>>>>> - refactor xenbus_init.
>>>>>
>>>>> Thank you. Lets also CC our friend at NSA who has been doing some work
>>>>> in that area. Daniel are you OK with this change - will it still make
>>>>> PV initial domain with with the MiniOS XenBus driver?
>>>>>
>>>>> Thanks.
>>>>
>>>> That case will work, but what this will break is launching the initial domain
>>>> with a Xenstore stub domain already running (see below).
>>>>
>>>>>>
>>>>>> Signed-off-by: Stefano Stabellini <[email protected]>
>>>>>> ---
>>>>>> drivers/xen/xenbus/xenbus_comms.c | 2 +-
>>>>>> drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
>>>>>> drivers/xen/xenbus/xenbus_xs.c | 1 +
>>>>>> 3 files changed, 45 insertions(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
>>>>>> index 52fe7ad..c5aa55c 100644
>>>>>> --- a/drivers/xen/xenbus/xenbus_comms.c
>>>>>> +++ b/drivers/xen/xenbus/xenbus_comms.c
>>>>>> @@ -224,7 +224,7 @@ int xb_init_comms(void)
>>>>>> int err;
>>>>>> err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
>>>>>> 0, "xenbus", &xb_waitq);
>>>>>> - if (err <= 0) {
>>>>>> + if (err < 0) {
>>>>>> printk(KERN_ERR "XENBUS request irq failed %i\n", err);
>>>>>> return err;
>>>>>> }
>>>>>
>>>>>> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
>>>>>> index b793723..a67ccc0 100644
>>>>>> --- a/drivers/xen/xenbus/xenbus_probe.c
>>>>>> +++ b/drivers/xen/xenbus/xenbus_probe.c
>>>>>> @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
>>>>>> return err;
>>>>>> }
>>>>>>
>>>>>> +enum xenstore_init {
>>>>>> + UNKNOWN,
>>>>>> + PV,
>>>>>> + HVM,
>>>>>> + LOCAL,
>>>>>> +};
>>>>>> static int __init xenbus_init(void)
>>>>>> {
>>>>>> int err = 0;
>>>>>> + enum xenstore_init usage = UNKNOWN;
>>>>>> + uint64_t v = 0;
>>>>>>
>>>>>> if (!xen_domain())
>>>>>> return -ENODEV;
>>>>>>
>>>>>> xenbus_ring_ops_init();
>>>>>>
>>>>>> - if (xen_hvm_domain()) {
>>>>>> - uint64_t v = 0;
>>>>>> - err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
>>>>>> - if (err)
>>>>>> - goto out_error;
>>>>>> - xen_store_evtchn = (int)v;
>>>>>> - err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
>>>>>> - if (err)
>>>>>> - goto out_error;
>>>>>> - xen_store_mfn = (unsigned long)v;
>>>>>> - xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
>>>>>> - } else {
>>>>>> - xen_store_evtchn = xen_start_info->store_evtchn;
>>>>>> - xen_store_mfn = xen_start_info->store_mfn;
>>>>>> - if (xen_store_evtchn)
>>>>>> - xenstored_ready = 1;
>>>>>> - else {
>>>>>> + if (xen_pv_domain())
>>>>>> + usage = PV;
>>>>>> + if (xen_hvm_domain())
>>>>>> + usage = HVM;
>>>>
>>>> The above is correct for domUs, and is overridden for dom0s:
>>>>
>>>>>> + if (xen_hvm_domain() && xen_initial_domain())
>>>>>> + usage = LOCAL;
>>>>>> + if (xen_pv_domain() && !xen_start_info->store_evtchn)
>>>>>> + usage = LOCAL;
>>>>
>>>> Instead of these checks, I think it should just be:
>>>>
>>>> if (!xen_start_info->store_evtchn)
>>>> usage = LOCAL;
>>>>
>>>> Any domain started after xenstore will have store_evtchn set, so if you don't
>>>> have this set, you are either going to be running xenstore locally, or will
>>>> use the ioctl to change it later (and so should still set up everything as if
>>>> it will be running locally).
>>>
>>> That would be wrong for an HVM dom0 domain (at least on ARM), because
>>> we don't have a start_info page at all.
>>>
>>>
>>>>>> + if (xen_pv_domain() && xen_start_info->store_evtchn)
>>>>>> + xenstored_ready = 1;
>>>>
>>>> This part can now just be moved unconditionally into case PV.
>>>
>>> What about:
>>>
>>> if (xen_pv_domain())
>>> usage = PV;
>>> if (xen_hvm_domain())
>>> usage = HVM;
>>> if (!xen_store_evtchn)
>>> usage = LOCAL;
>>>
>>> and moving xenstored_ready in case PV, like you suggested.
>>>
>>
>> That looks correct, but you'd need to split up the switch statement in
>> order to populate xen_store_evtchn before that last condition, which
>> ends up pretty much eliminating the usage variable.
>
> Going back to what you wrote in the previous email, in what way this
> patch breaks the case when an initial domain is started after a Xenstore
> stub domain?
>
> Assuming that we are talking about a PV initial domain on x86, the
> following check
>
> if (xen_pv_domain() && !xen_start_info->store_evtchn)
> usage = LOCAL;
>
> will return false (because store_evtchn is set), therefore usage will
> remain set to PV.
> And the check:
>
> if (xen_pv_domain() && xen_start_info->store_evtchn)
> xenstored_ready = 1;
>
> will return true so xenstored_ready is going to be set to 1.
>

Right, the original patch didn't break anything with PV domains. The case
it doesn't handle is an HVM initial domain with an already-running
Xenstore domain; I think this applies both to ARM and hybrid/PVH on x86.
In that case, usage would be set to LOCAL instead of HVM.

As a side note: the value of xen_initial_domain() shouldn't be connected to
determining if xenstore is running locally or not.

--
Daniel De Graaf
National Security Agency

2012-08-08 17:42:33

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

On Wed, 8 Aug 2012, Daniel De Graaf wrote:
> On 08/08/2012 01:19 PM, Stefano Stabellini wrote:
> > On Wed, 8 Aug 2012, Daniel De Graaf wrote:
> >> On 08/08/2012 12:51 PM, Stefano Stabellini wrote:
> >>> On Tue, 7 Aug 2012, Daniel De Graaf wrote:
> >>>> On 08/07/2012 02:21 PM, Konrad Rzeszutek Wilk wrote:
> >>>>> On Mon, Aug 06, 2012 at 03:27:13PM +0100, Stefano Stabellini wrote:
> >>>>>> bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not
> >>>>>> an error.
> >>>>>>
> >>>>>> If Linux is running as an HVM domain and is running as Dom0, use
> >>>>>> xenstored_local_init to initialize the xenstore page and event channel.
> >>>>>>
> >>>>>> Changes in v2:
> >>>>>>
> >>>>>> - refactor xenbus_init.
> >>>>>
> >>>>> Thank you. Lets also CC our friend at NSA who has been doing some work
> >>>>> in that area. Daniel are you OK with this change - will it still make
> >>>>> PV initial domain with with the MiniOS XenBus driver?
> >>>>>
> >>>>> Thanks.
> >>>>
> >>>> That case will work, but what this will break is launching the initial domain
> >>>> with a Xenstore stub domain already running (see below).
> >>>>
> >>>>>>
> >>>>>> Signed-off-by: Stefano Stabellini <[email protected]>
> >>>>>> ---
> >>>>>> drivers/xen/xenbus/xenbus_comms.c | 2 +-
> >>>>>> drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++-----------
> >>>>>> drivers/xen/xenbus/xenbus_xs.c | 1 +
> >>>>>> 3 files changed, 45 insertions(+), 20 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
> >>>>>> index 52fe7ad..c5aa55c 100644
> >>>>>> --- a/drivers/xen/xenbus/xenbus_comms.c
> >>>>>> +++ b/drivers/xen/xenbus/xenbus_comms.c
> >>>>>> @@ -224,7 +224,7 @@ int xb_init_comms(void)
> >>>>>> int err;
> >>>>>> err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting,
> >>>>>> 0, "xenbus", &xb_waitq);
> >>>>>> - if (err <= 0) {
> >>>>>> + if (err < 0) {
> >>>>>> printk(KERN_ERR "XENBUS request irq failed %i\n", err);
> >>>>>> return err;
> >>>>>> }
> >>>>>
> >>>>>> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> >>>>>> index b793723..a67ccc0 100644
> >>>>>> --- a/drivers/xen/xenbus/xenbus_probe.c
> >>>>>> +++ b/drivers/xen/xenbus/xenbus_probe.c
> >>>>>> @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void)
> >>>>>> return err;
> >>>>>> }
> >>>>>>
> >>>>>> +enum xenstore_init {
> >>>>>> + UNKNOWN,
> >>>>>> + PV,
> >>>>>> + HVM,
> >>>>>> + LOCAL,
> >>>>>> +};
> >>>>>> static int __init xenbus_init(void)
> >>>>>> {
> >>>>>> int err = 0;
> >>>>>> + enum xenstore_init usage = UNKNOWN;
> >>>>>> + uint64_t v = 0;
> >>>>>>
> >>>>>> if (!xen_domain())
> >>>>>> return -ENODEV;
> >>>>>>
> >>>>>> xenbus_ring_ops_init();
> >>>>>>
> >>>>>> - if (xen_hvm_domain()) {
> >>>>>> - uint64_t v = 0;
> >>>>>> - err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
> >>>>>> - if (err)
> >>>>>> - goto out_error;
> >>>>>> - xen_store_evtchn = (int)v;
> >>>>>> - err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
> >>>>>> - if (err)
> >>>>>> - goto out_error;
> >>>>>> - xen_store_mfn = (unsigned long)v;
> >>>>>> - xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
> >>>>>> - } else {
> >>>>>> - xen_store_evtchn = xen_start_info->store_evtchn;
> >>>>>> - xen_store_mfn = xen_start_info->store_mfn;
> >>>>>> - if (xen_store_evtchn)
> >>>>>> - xenstored_ready = 1;
> >>>>>> - else {
> >>>>>> + if (xen_pv_domain())
> >>>>>> + usage = PV;
> >>>>>> + if (xen_hvm_domain())
> >>>>>> + usage = HVM;
> >>>>
> >>>> The above is correct for domUs, and is overridden for dom0s:
> >>>>
> >>>>>> + if (xen_hvm_domain() && xen_initial_domain())
> >>>>>> + usage = LOCAL;
> >>>>>> + if (xen_pv_domain() && !xen_start_info->store_evtchn)
> >>>>>> + usage = LOCAL;
> >>>>
> >>>> Instead of these checks, I think it should just be:
> >>>>
> >>>> if (!xen_start_info->store_evtchn)
> >>>> usage = LOCAL;
> >>>>
> >>>> Any domain started after xenstore will have store_evtchn set, so if you don't
> >>>> have this set, you are either going to be running xenstore locally, or will
> >>>> use the ioctl to change it later (and so should still set up everything as if
> >>>> it will be running locally).
> >>>
> >>> That would be wrong for an HVM dom0 domain (at least on ARM), because
> >>> we don't have a start_info page at all.
> >>>
> >>>
> >>>>>> + if (xen_pv_domain() && xen_start_info->store_evtchn)
> >>>>>> + xenstored_ready = 1;
> >>>>
> >>>> This part can now just be moved unconditionally into case PV.
> >>>
> >>> What about:
> >>>
> >>> if (xen_pv_domain())
> >>> usage = PV;
> >>> if (xen_hvm_domain())
> >>> usage = HVM;
> >>> if (!xen_store_evtchn)
> >>> usage = LOCAL;
> >>>
> >>> and moving xenstored_ready in case PV, like you suggested.
> >>>
> >>
> >> That looks correct, but you'd need to split up the switch statement in
> >> order to populate xen_store_evtchn before that last condition, which
> >> ends up pretty much eliminating the usage variable.
> >
> > Going back to what you wrote in the previous email, in what way this
> > patch breaks the case when an initial domain is started after a Xenstore
> > stub domain?
> >
> > Assuming that we are talking about a PV initial domain on x86, the
> > following check
> >
> > if (xen_pv_domain() && !xen_start_info->store_evtchn)
> > usage = LOCAL;
> >
> > will return false (because store_evtchn is set), therefore usage will
> > remain set to PV.
> > And the check:
> >
> > if (xen_pv_domain() && xen_start_info->store_evtchn)
> > xenstored_ready = 1;
> >
> > will return true so xenstored_ready is going to be set to 1.
> >
>
> Right, the original patch didn't break anything with PV domains. The case
> it doesn't handle is an HVM initial domain with an already-running
> Xenstore domain; I think this applies both to ARM and hybrid/PVH on x86.
> In that case, usage would be set to LOCAL instead of HVM.


Right, however if I am not mistaken there is no such thing as an HVM
dom0 right now on x86 and hybrid/PVH is probably going to return true on
xen_pv_domain() and false on xen_hvm_domain().

In the ARM case, given that we don't have a start_info page, we would
need another way to figure out whether a xenstore stub domain is already
running, so I think we can just postpone the solution of that problem
for now.

2012-08-08 18:05:54

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 15/23] xen/arm: receive Xen events on ARM

On Tue, 7 Aug 2012, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:27:18PM +0100, Stefano Stabellini wrote:
> > Compile events.c on ARM.
> > Parse, map and enable the IRQ to get event notifications from the device
> > tree (node "/xen").
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > arch/arm/include/asm/xen/events.h | 18 ++++++++++++++++++
> > arch/arm/xen/enlighten.c | 33 +++++++++++++++++++++++++++++++++
> > arch/x86/xen/enlighten.c | 1 +
> > arch/x86/xen/irq.c | 1 +
> > arch/x86/xen/xen-ops.h | 1 -
> > drivers/xen/events.c | 17 ++++++++++++++---
> > include/xen/events.h | 2 ++
> > 7 files changed, 69 insertions(+), 4 deletions(-)
> > create mode 100644 arch/arm/include/asm/xen/events.h
> >
> > diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> > new file mode 100644
> > index 0000000..94b4e90
> > --- /dev/null
> > +++ b/arch/arm/include/asm/xen/events.h
> > @@ -0,0 +1,18 @@
> > +#ifndef _ASM_ARM_XEN_EVENTS_H
> > +#define _ASM_ARM_XEN_EVENTS_H
> > +
> > +#include <asm/ptrace.h>
> > +
> > +enum ipi_vector {
> > + XEN_PLACEHOLDER_VECTOR,
> > +
> > + /* Xen IPIs go here */
> > + XEN_NR_IPIS,
> > +};
> > +
> > +static inline int xen_irqs_disabled(struct pt_regs *regs)
> > +{
> > + return raw_irqs_disabled_flags(regs->ARM_cpsr);
> > +}
> > +
> > +#endif /* _ASM_ARM_XEN_EVENTS_H */
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index e5e92d5..87b17f0 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -1,4 +1,5 @@
> > #include <xen/xen.h>
> > +#include <xen/events.h>
> > #include <xen/grant_table.h>
> > #include <xen/hvm.h>
> > #include <xen/interface/xen.h>
> > @@ -9,6 +10,8 @@
> > #include <xen/xenbus.h>
> > #include <asm/xen/hypervisor.h>
> > #include <asm/xen/hypercall.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irqreturn.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/of_irq.h>
> > @@ -33,6 +36,8 @@ EXPORT_SYMBOL_GPL(xen_have_vector_callback);
> > int xen_platform_pci_unplug = XEN_UNPLUG_ALL;
> > EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
> >
> > +static __read_mostly int xen_events_irq = -1;
> > +
>
> So this is global..
> > int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> > unsigned long addr,
> > unsigned long mfn, int nr,
> > @@ -66,6 +71,9 @@ static int __init xen_guest_init(void)
> > if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res))
> > return 0;
> > xen_hvm_resume_frames = res.start >> PAGE_SHIFT;
> > + xen_events_irq = irq_of_parse_and_map(node, 0);
> > + pr_info("Xen support found, events_irq=%d gnttab_frame_pfn=%lx\n",
> > + xen_events_irq, xen_hvm_resume_frames);
> > xen_domain_type = XEN_HVM_DOMAIN;
> >
> > xen_setup_features();
> > @@ -107,3 +115,28 @@ static int __init xen_guest_init(void)
> > return 0;
> > }
> > core_initcall(xen_guest_init);
> > +
> > +static irqreturn_t xen_arm_callback(int irq, void *arg)
> > +{
> > + xen_hvm_evtchn_do_upcall();
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int __init xen_init_events(void)
> > +{
> > + if (!xen_domain() || xen_events_irq < 0)
> > + return -ENODEV;
> > +
> > + xen_init_IRQ();
> > +
> > + if (request_percpu_irq(xen_events_irq, xen_arm_callback,
> > + "events", xen_vcpu)) {
>
> But here you are asking for it to be percpu? What if there are other
> interrupts on the _other_ CPUs that conflict with it?
> > + pr_err("Error requesting IRQ %d\n", xen_events_irq);
> > + return -EINVAL;
> > + }
> > +
> > + enable_percpu_irq(xen_events_irq, 0);
>
> Uh, that is bold. One global to rule them all, eh? Should you make
> it at least:
> static DEFINE_PER_CPU(int, xen_events_irq);
> ?

That is an interesting observation.

Currently Xen is using a per-cpu interrupt (a PPI, using the GIC
terminology), and it makes sense so that we can receive event
notifications on multiple vcpus independently.
The irq range 16-31 is reserved for PPIs and I am assuming that Xen will
be able to find one spare, the same one, for all vcpus.
In fact the third field corresponding to the interrupt in the DT (0xf08
in my dts) contains the cpu mask and it is set to 0xf (the maximum)
right now.

Maybe I should just BUG_ON(xen_events_irq > 31 || xen_events_irq < 16)?

The versioning of the hypervisor node on the DT is going to help us make
any changes to the interface in the future.

2012-08-09 15:38:05

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v2 02/23] xen/arm: hypercalls

On Wed, 8 Aug 2012, Dave Martin wrote:
> On Mon, Aug 06, 2012 at 03:27:05PM +0100, Stefano Stabellini wrote:
> > Use r12 to pass the hypercall number to the hypervisor.
> >
> > We need a register to pass the hypercall number because we might not
> > know it at compile time and HVC only takes an immediate argument.
> >
> > Among the available registers r12 seems to be the best choice because it
> > is defined as "intra-procedure call scratch register".
> >
> > Use the ISS to pass an hypervisor specific tag.
> >
> > Changes in v2:
> > - define an HYPERCALL macro for 5 arguments hypercall wrappers, even if
> > at the moment is unused;
> > - use ldm instead of pop;
> > - fix up comments.
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > ---
> > arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++
> > arch/arm/xen/Makefile | 2 +-
> > arch/arm/xen/hypercall.S | 106 ++++++++++++++++++++++++++++++++++
> > 3 files changed, 157 insertions(+), 1 deletions(-)
> > create mode 100644 arch/arm/include/asm/xen/hypercall.h
> > create mode 100644 arch/arm/xen/hypercall.S
>
> [...]
>
> > diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> > new file mode 100644
> > index 0000000..074f5ed
> > --- /dev/null
> > +++ b/arch/arm/xen/hypercall.S
> > @@ -0,0 +1,106 @@
> > +/******************************************************************************
> > + * hypercall.S
> > + *
> > + * Xen hypercall wrappers
> > + *
> > + * Stefano Stabellini <[email protected]>, Citrix, 2012
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License version 2
> > + * as published by the Free Software Foundation; or, when distributed
> > + * separately from the Linux kernel or incorporated into other
> > + * software packages, subject to the following license:
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a copy
> > + * of this source file (the "Software"), to deal in the Software without
> > + * restriction, including without limitation the rights to use, copy, modify,
> > + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
> > + * and to permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +/*
> > + * The Xen hypercall calling convention is very similar to the ARM
> > + * procedure calling convention: the first paramter is passed in r0, the
> > + * second in r1, the third in r2 and the fourth in r3. Considering that
> > + * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
> > + * in r4, differently from the procedure calling convention of using the
> > + * stack for that case.
> > + *
> > + * The hypercall number is passed in r12.
> > + *
> > + * The return value is in r0.
> > + *
> > + * The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
> > + * hypercall tag.
> > + */
> > +
> > +#include <linux/linkage.h>
> > +#include <asm/assembler.h>
> > +#include <xen/interface/xen.h>
> > +
> > +
> > +/* HVC 0xEA1 */
> > +#ifdef CONFIG_THUMB2_KERNEL
> > +#define xen_hvc .word 0xf7e08ea1
> > +#else
> > +#define xen_hvc .word 0xe140ea71
> > +#endif
>
> Consider using my opcode injection helpers patch for this (see
> separate repost: [PATCH v2 REPOST 0/4] ARM: opcodes: Facilitate custom
> opcode injection), assuming that nobody objects to it. This should mean
> that the right opcodes get generated when building a kernel for a big-
> endian target for example.
>
> I believe the __HVC(imm) macro which I put in <asm/opcodes-virt.h> as an
> example should do what you need in this case.

Sure I can do that. Maybe I'll add another patch at the end of my series
to replace xen_hvc with __HVC(0xEA1), so that it remains independent
from your series.
I have learned through experience that avoiding cross patch series
dependencies help to reduce the amount of headaches during merge windows
:)


> > +
> > +#define HYPERCALL_SIMPLE(hypercall) \
> > +ENTRY(HYPERVISOR_##hypercall) \
> > + mov r12, #__HYPERVISOR_##hypercall; \
> > + xen_hvc; \
> > + mov pc, lr; \
> > +ENDPROC(HYPERVISOR_##hypercall)
> > +
> > +#define HYPERCALL0 HYPERCALL_SIMPLE
> > +#define HYPERCALL1 HYPERCALL_SIMPLE
> > +#define HYPERCALL2 HYPERCALL_SIMPLE
> > +#define HYPERCALL3 HYPERCALL_SIMPLE
> > +#define HYPERCALL4 HYPERCALL_SIMPLE
> > +
> > +#define HYPERCALL5(hypercall) \
> > +ENTRY(HYPERVISOR_##hypercall) \
> > + stmdb sp!, {r4} \
> > + ldr r4, [sp, #4] \
> > + mov r12, #__HYPERVISOR_##hypercall; \
> > + xen_hvc \
> > + ldm sp!, {r4} \
> > + mov pc, lr \
> > +ENDPROC(HYPERVISOR_##hypercall)
> > +
> > + .text
> > +
> > +HYPERCALL2(xen_version);
> > +HYPERCALL3(console_io);
> > +HYPERCALL3(grant_table_op);
> > +HYPERCALL2(sched_op);
> > +HYPERCALL2(event_channel_op);
> > +HYPERCALL2(hvm_op);
> > +HYPERCALL2(memory_op);
> > +HYPERCALL2(physdev_op);
> > +
> > +ENTRY(privcmd_call)
> > + stmdb sp!, {r4}
> > + mov r12, r0
> > + mov r0, r1
> > + mov r1, r2
> > + mov r2, r3
> > + ldr r3, [sp, #8]
> > + ldr r4, [sp, #4]
> > + xen_hvc
> > + ldm sp!, {r4}
> > + mov pc, lr
>
> Note that the preferred entry/exit sequences in such cases are:
>
> stmfd sp!, {r4,lr}
> ...
> ldmfd sp!, {r4,pc}
>
> ...but it works either way. I would bother to change it unless you
> have other changes to make too.

Wouldn't this needlessly save and restore one more register (lr) to the
stack?
I would try to keep the hypercall wrappers as small as possible...

2012-08-09 16:50:31

by Dave Martin

[permalink] [raw]
Subject: Re: [PATCH v2 02/23] xen/arm: hypercalls

On Thu, Aug 09, 2012 at 04:37:24PM +0100, Stefano Stabellini wrote:
> On Wed, 8 Aug 2012, Dave Martin wrote:
> > On Mon, Aug 06, 2012 at 03:27:05PM +0100, Stefano Stabellini wrote:
> > > Use r12 to pass the hypercall number to the hypervisor.
> > >
> > > We need a register to pass the hypercall number because we might not
> > > know it at compile time and HVC only takes an immediate argument.
> > >
> > > Among the available registers r12 seems to be the best choice because it
> > > is defined as "intra-procedure call scratch register".
> > >
> > > Use the ISS to pass an hypervisor specific tag.
> > >
> > > Changes in v2:
> > > - define an HYPERCALL macro for 5 arguments hypercall wrappers, even if
> > > at the moment is unused;
> > > - use ldm instead of pop;
> > > - fix up comments.
> > >
> > > Signed-off-by: Stefano Stabellini <[email protected]>
> > > ---
> > > arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++
> > > arch/arm/xen/Makefile | 2 +-
> > > arch/arm/xen/hypercall.S | 106 ++++++++++++++++++++++++++++++++++
> > > 3 files changed, 157 insertions(+), 1 deletions(-)
> > > create mode 100644 arch/arm/include/asm/xen/hypercall.h
> > > create mode 100644 arch/arm/xen/hypercall.S
> >
> > [...]
> >
> > > diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> > > new file mode 100644
> > > index 0000000..074f5ed
> > > --- /dev/null
> > > +++ b/arch/arm/xen/hypercall.S
> > > @@ -0,0 +1,106 @@
> > > +/******************************************************************************
> > > + * hypercall.S
> > > + *
> > > + * Xen hypercall wrappers
> > > + *
> > > + * Stefano Stabellini <[email protected]>, Citrix, 2012
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public License version 2
> > > + * as published by the Free Software Foundation; or, when distributed
> > > + * separately from the Linux kernel or incorporated into other
> > > + * software packages, subject to the following license:
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining a copy
> > > + * of this source file (the "Software"), to deal in the Software without
> > > + * restriction, including without limitation the rights to use, copy, modify,
> > > + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
> > > + * and to permit persons to whom the Software is furnished to do so, subject to
> > > + * the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice shall be included in
> > > + * all copies or substantial portions of the Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> > > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + */
> > > +
> > > +/*
> > > + * The Xen hypercall calling convention is very similar to the ARM
> > > + * procedure calling convention: the first paramter is passed in r0, the
> > > + * second in r1, the third in r2 and the fourth in r3. Considering that
> > > + * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
> > > + * in r4, differently from the procedure calling convention of using the
> > > + * stack for that case.
> > > + *
> > > + * The hypercall number is passed in r12.
> > > + *
> > > + * The return value is in r0.
> > > + *
> > > + * The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
> > > + * hypercall tag.
> > > + */
> > > +
> > > +#include <linux/linkage.h>
> > > +#include <asm/assembler.h>
> > > +#include <xen/interface/xen.h>
> > > +
> > > +
> > > +/* HVC 0xEA1 */
> > > +#ifdef CONFIG_THUMB2_KERNEL
> > > +#define xen_hvc .word 0xf7e08ea1
> > > +#else
> > > +#define xen_hvc .word 0xe140ea71
> > > +#endif
> >
> > Consider using my opcode injection helpers patch for this (see
> > separate repost: [PATCH v2 REPOST 0/4] ARM: opcodes: Facilitate custom
> > opcode injection), assuming that nobody objects to it. This should mean
> > that the right opcodes get generated when building a kernel for a big-
> > endian target for example.
> >
> > I believe the __HVC(imm) macro which I put in <asm/opcodes-virt.h> as an
> > example should do what you need in this case.
>
> Sure I can do that. Maybe I'll add another patch at the end of my series
> to replace xen_hvc with __HVC(0xEA1), so that it remains independent
> from your series.
> I have learned through experience that avoiding cross patch series
> dependencies help to reduce the amount of headaches during merge windows
> :)

I agree. I'll let you know when my patch gets merged -- in the meantime,
it makes sense for you to keep your existing code.

>
>
> > > +
> > > +#define HYPERCALL_SIMPLE(hypercall) \
> > > +ENTRY(HYPERVISOR_##hypercall) \
> > > + mov r12, #__HYPERVISOR_##hypercall; \
> > > + xen_hvc; \
> > > + mov pc, lr; \
> > > +ENDPROC(HYPERVISOR_##hypercall)
> > > +
> > > +#define HYPERCALL0 HYPERCALL_SIMPLE
> > > +#define HYPERCALL1 HYPERCALL_SIMPLE
> > > +#define HYPERCALL2 HYPERCALL_SIMPLE
> > > +#define HYPERCALL3 HYPERCALL_SIMPLE
> > > +#define HYPERCALL4 HYPERCALL_SIMPLE
> > > +
> > > +#define HYPERCALL5(hypercall) \
> > > +ENTRY(HYPERVISOR_##hypercall) \
> > > + stmdb sp!, {r4} \
> > > + ldr r4, [sp, #4] \
> > > + mov r12, #__HYPERVISOR_##hypercall; \
> > > + xen_hvc \
> > > + ldm sp!, {r4} \
> > > + mov pc, lr \
> > > +ENDPROC(HYPERVISOR_##hypercall)
> > > +
> > > + .text
> > > +
> > > +HYPERCALL2(xen_version);
> > > +HYPERCALL3(console_io);
> > > +HYPERCALL3(grant_table_op);
> > > +HYPERCALL2(sched_op);
> > > +HYPERCALL2(event_channel_op);
> > > +HYPERCALL2(hvm_op);
> > > +HYPERCALL2(memory_op);
> > > +HYPERCALL2(physdev_op);
> > > +
> > > +ENTRY(privcmd_call)
> > > + stmdb sp!, {r4}
> > > + mov r12, r0
> > > + mov r0, r1
> > > + mov r1, r2
> > > + mov r2, r3
> > > + ldr r3, [sp, #8]
> > > + ldr r4, [sp, #4]
> > > + xen_hvc
> > > + ldm sp!, {r4}
> > > + mov pc, lr
> >
> > Note that the preferred entry/exit sequences in such cases are:
> >
> > stmfd sp!, {r4,lr}
> > ...
> > ldmfd sp!, {r4,pc}
> >
> > ...but it works either way. I would bother to change it unless you
> > have other changes to make too.
>
> Wouldn't this needlessly save and restore one more register (lr) to the
> stack?
> I would try to keep the hypercall wrappers as small as possible...

Argh, ignore me -- I was hallucinating for some reason that we actually
needed to save lr, but we don't.

Using the stmfd/ldmfd mnemonics might still be nicer than stmdb/ldm, since
the fd suffix makes the stack semantics more obvious, and the code
looks more symmetrical. This was the conventional way to write these
mnemonics before the "push" and "pop" mnemonics existed.

That's purely cosmetic, though.

Cheers
---Dave

2012-08-09 17:04:34

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH v2 10/23] xen/arm: compile and run xenbus

> > Right, the original patch didn't break anything with PV domains. The case
> > it doesn't handle is an HVM initial domain with an already-running
> > Xenstore domain; I think this applies both to ARM and hybrid/PVH on x86.
> > In that case, usage would be set to LOCAL instead of HVM.
>
>
> Right, however if I am not mistaken there is no such thing as an HVM
> dom0 right now on x86 and hybrid/PVH is probably going to return true on
> xen_pv_domain() and false on xen_hvm_domain().

The other way around. HVM = true, PV = false.

Mukesh, correct me if I am wrong pls.
>
> In the ARM case, given that we don't have a start_info page, we would
> need another way to figure out whether a xenstore stub domain is already
> running, so I think we can just postpone the solution of that problem
> for now.