sync_bitops functions are equivalent to the SMP implementation of the
original functions, independently from CONFIG_SMP being defined.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/include/asm/sync_bitops.h | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/include/asm/sync_bitops.h
@@ -246,6 +246,7 @@ endifcore-$(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/
@@ -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>++structstart_info_xen_start_info;+structstart_info*xen_start_info=&_xen_start_info;+EXPORT_SYMBOL_GPL(xen_start_info);++enumxen_domain_typexen_domain_type=XEN_NATIVE;+EXPORT_SYMBOL_GPL(xen_domain_type);++structshared_infoxen_dummy_shared_info;+structshared_info*HYPERVISOR_shared_info=(void*)&xen_dummy_shared_info;++DEFINE_PER_CPU(structvcpu_info*,xen_vcpu);++/* XXX: to be removed */+__read_mostlyintxen_have_vector_callback;+EXPORT_SYMBOL_GPL(xen_have_vector_callback);++intxen_platform_pci_unplug=XEN_UNPLUG_ALL;+EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);++intxen_remap_domain_mfn_range(structvm_area_struct*vma,+unsignedlongaddr,+unsignedlongmfn,intnr,+pgprot_tprot,unsigneddomid)+{+return-ENOSYS;+}+EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
@@ -33,3 +36,56 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,return-ENOSYS;}EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);++/*+*==XenDeviceTreeformat==+*-/xennode;+*-compatible"arm,xen";+*-oneinterruptforXeneventnotifications;+*-onememoryregiontomapthegrant_table.+*/+staticint__initxen_guest_init(void)+{+intcpu;+structxen_add_to_physmapxatp;+staticstructshared_info*shared_info_page=0;+structdevice_node*node;++node=of_find_compatible_node(NULL,NULL,"arm,xen");+if(!node){+pr_info("No Xen support\n");+return0;+}+xen_domain_type=XEN_HVM_DOMAIN;++if(!shared_info_page)+shared_info_page=(structshared_info*)+get_zeroed_page(GFP_KERNEL);+if(!shared_info_page){+pr_err("not enough memory");+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=(structshared_info*)shared_info_page;++/* xen_vcpu is a pointer to the vcpu_info struct in the shared_info+*page,weuseitintheeventchannelupcallandinsomepvclock+*relatedfunctions.Wedon'tneedthevcpu_infoplacement+*optimizationsbecausewedon'tuseanypv_mmuorpv_irqopon+*HVM.+*Whenxen_hvm_init_shared_infoisrunatboottimeonlyvcpu0is+*onlinebutxen_hvm_init_shared_infoisrunatresumetimetooand+*inthatcasemultiplevcpusmightbeonline.*/+for_each_online_cpu(cpu){+per_cpu(xen_vcpu,cpu)=+&HYPERVISOR_shared_info->vcpu_info[cpu];+}+return0;+}+core_initcall(xen_guest_init);
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 <redacted>
---
arch/arm/include/asm/xen/interface.h | 2 ++
arch/ia64/include/asm/xen/interface.h | 2 +-
arch/x86/include/asm/xen/interface.h | 2 ++
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, 15 insertions(+), 13 deletions(-)
@@ -25,6 +25,7 @@}while(0)#ifndef __ASSEMBLY__+typedefuint64_txen_pfn_t;/* Guest handles for primitive C types. */__DEFINE_GUEST_HANDLE(uchar,unsignedchar);__DEFINE_GUEST_HANDLE(uint,unsignedint);
@@ -35,6 +36,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
@@ -31,7 +31,7 @@ struct xen_memory_reservation {*OUT:GMFNbasesofextentsthatwereallocated*(NB.Thiscommandalsoupdatesthemach_to_phystranslationtable)*/-GUEST_HANDLE(ulong)extent_start;+GUEST_HANDLE(xen_pfn_t)extent_start;/* Number of extents, and size/alignment of each (2^extent_order pages). */unsignedlongnr_extents;
@@ -432,11 +432,11 @@ struct start_info {unsignedlongnr_pages;/* Total pages allocated to this domain. */unsignedlongshared_info;/* MACHINE address of shared info struct. */uint32_tflags;/* SIF_xxx flags. */-unsignedlongstore_mfn;/* MACHINE page number of shared page. */+xen_pfn_tstore_mfn;/* MACHINE page number of shared page. */uint32_tstore_evtchn;/* Event channel for store communication. */union{struct{-unsignedlongmfn;/* MACHINE page number of console page. */+xen_pfn_tmfn;/* MACHINE page number of console page. */uint32_tevtchn;/* Event channel for console page. */}domU;struct{
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.
Signed-off-by: Stefano Stabellini <redacted>
---
drivers/xen/xenbus/xenbus_comms.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 27 +++++++++++++++++----------
drivers/xen/xenbus/xenbus_xs.c | 1 +
3 files changed, 19 insertions(+), 11 deletions(-)
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++++++++++++
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/hypercall.S | 65 ++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/include/asm/xen/hypercall.h
create mode 100644 arch/arm/xen/hypercall.S
ARM Xen guests always use paging in hardware, like PV on HVM guests in
the X86 world.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/include/asm/xen/page.h | 77 +++++++++++++++++++++++++++++++++++++++
1 files changed, 77 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/include/asm/xen/page.h
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 [off-list ref]
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."
This is not true on ARM where there is not padding, but it is valid on
X86, so introducing size is safe on X86 and it is going to fix the
interace for ARM.
Signed-off-by: Stefano Stabellini <redacted>
---
include/xen/interface/memory.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
@@ -163,6 +163,9 @@ struct xen_add_to_physmap {/* Which domain to change the mapping for. */domid_tdomid;+/* Number of pages to go through for gmfn_range */+uint16_tsize;+/* Source mapping space. */#define XENMAPSPACE_shared_info 0 /* shared info page */#define XENMAPSPACE_grant_table 1 /* grant table page */
On ARM all guests are HVM guests, including Dom0.
Allow dom0_write_console to be called by an HVM domain.
Signed-off-by: Stefano Stabellini <redacted>
---
drivers/tty/hvc/hvc_xen.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
From: Ian Campbell <redacted>
Currently ARM setup_early_printk does not support alternative early
consoles and it always registers early_console only.
This patch adds support for xenboot_console.
Signed-off-by: Ian Campbell <redacted>
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/kernel/early_printk.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
In order for privcmd mmap to work correctly, xen_remap_domain_mfn_range
needs to be implemented for HVM guests.
If it is not, mmap is going to fail later on.
Signed-off-by: Stefano Stabellini <redacted>
---
drivers/xen/privcmd.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
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 <redacted>
---
arch/arm/xen/enlighten.c | 7 +++++++
include/xen/interface/features.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)
@@ -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__ */
Compile events.c on ARM.
Parse, map and enable the IRQ to get event notifications from the device
tree (node "/xen").
On ARM Linux irqs are not enabled by default:
- call enable_percpu_irq for xen_events_irq (drivers are supposed
to call enable_irq after request_irq);
- 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 <redacted>
---
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 | 18 +++++++++++++++---
include/xen/events.h | 2 ++
6 files changed, 52 insertions(+), 4 deletions(-)
@@ -834,6 +839,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)structirq_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);
@@ -109,4 +109,6 @@ int xen_irq_from_gsi(unsigned gsi);/* Determine whether to ignore this IRQ if it is passed to a guest. */intxen_test_irq_shared(intirq);+/* initialize Xen IRQ subsystem */+voidxen_init_IRQ(void);#endif /* _XEN_EVENTS_H */
Only until we get the balloon driver to work.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
@@ -121,6 +121,24 @@ int __init xen_guest_init(void)return0;}EXPORT_SYMBOL_GPL(xen_guest_init);++/* XXX: only until balloon is properly working */+intalloc_xenballooned_pages(intnr_pages,structpage**pages,boolhighmem)+{+*pages=alloc_pages(highmem?GFP_HIGHUSER:GFP_KERNEL,+get_order(nr_pages));+if(*pages==NULL)+return-ENOMEM;+return0;+}+EXPORT_SYMBOL_GPL(alloc_xenballooned_pages);++voidfree_xenballooned_pages(intnr_pages,structpage**pages)+{+kfree(*pages);+*pages=NULL;+}+EXPORT_SYMBOL_GPL(free_xenballooned_pages);core_initcall(xen_guest_init);staticirqreturn_txen_arm_callback(intirq,void*arg)
From: Ian Campbell <redacted>
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.
Signed-off-by: Ian Campbell <redacted>
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 79 +++++++++++++++++++++++++++++++++++++++-
drivers/xen/privcmd.c | 16 +++++----
drivers/xen/xenfs/super.c | 7 ++++
include/xen/interface/memory.h | 10 ++++--
4 files changed, 101 insertions(+), 11 deletions(-)
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).
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 13 +++++++++++++
drivers/xen/grant-table.c | 2 +-
2 files changed, 14 insertions(+), 1 deletions(-)
We used to rely on a core_initcall to initialize Xen on ARM, however
core_initcalls are actually called after early consoles are initialized.
That means that hvc_xen.c is going to be initialized before Xen.
Given the lack of a better alternative, just call a new Xen
initialization function (xen_guest_init) from xen_cons_init.
xen_guest_init has to be arch independent, so write both an ARM and an
x86 implementation. The x86 implementation is currently empty because we
can be sure that xen_hvm_guest_init is called early enough.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 7 ++++++-
arch/x86/xen/enlighten.c | 8 ++++++++
drivers/tty/hvc/hvc_xen.c | 7 ++++++-
include/xen/xen.h | 2 ++
4 files changed, 22 insertions(+), 2 deletions(-)
@@ -1567,4 +1567,12 @@ const struct hypervisor_x86 x86_hyper_xen_hvm __refconst = {.init_platform=xen_hvm_guest_init,};EXPORT_SYMBOL(x86_hyper_xen_hvm);++int__initxen_guest_init(void)+{+/* do nothing: rely on x86_hyper_xen_hvm for the initialization */+return0;++}+EXPORT_SYMBOL_GPL(xen_guest_init);#endif
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-07-26 16:39:44
On Thu, Jul 26, 2012 at 04:33:43PM +0100, Stefano Stabellini wrote:
- Basic hypervisor.h and interface.h definitions.
- Skelethon enlighten.c, set xen_start_info to an empty struct.
Skeleton
- Do not limit xen_initial_domain to PV guests.
Better wording: Make xen_initial_domain dependent on the SIF_PRIVILIGED_BIT.
Which reminds me - what about PV guests that do PCI passthrough. Aren't
they "more" priviligied than normal PV guests? Or not really?
The new code only compiles when CONFIG_XEN is set, that is going to be
added to arch/arm/Kconfig in a later patch.
@@ -246,6 +246,7 @@ endifcore-$(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/
@@ -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>++structstart_info_xen_start_info;+structstart_info*xen_start_info=&_xen_start_info;+EXPORT_SYMBOL_GPL(xen_start_info);++enumxen_domain_typexen_domain_type=XEN_NATIVE;+EXPORT_SYMBOL_GPL(xen_domain_type);++structshared_infoxen_dummy_shared_info;+structshared_info*HYPERVISOR_shared_info=(void*)&xen_dummy_shared_info;++DEFINE_PER_CPU(structvcpu_info*,xen_vcpu);++/* XXX: to be removed */
In this patch series later on? Or just when you try to collapse the x86 and arm variant
together?
quoted hunk
+__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);
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-07-26 16:43:30
On Thu, Jul 26, 2012 at 04:33:44PM +0100, Stefano Stabellini wrote:
quoted hunk
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++++++++++++
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/hypercall.S | 65 ++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/include/asm/xen/hypercall.h
create mode 100644 arch/arm/xen/hypercall.S
I think you meant 'and the fourth in r3'.
So where does the similarity end? Just in that we use r12?
+ * 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.
+ *
+ * Stefano Stabellini [off-list ref], Citrix, 2012
+ */
+
+#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
+
+/* We need to save and restore r4, because Xen clobbers it. */
Hmm, the comment says r4, but right below I see r12?
Should this comment be by 'privcmd_call'?
I don't if it makes such a difference, but putting the headers in sorted
order is sometimes nicer than just randomly.. But that might be just
me liking an orderly world nowadays :-)
From: Christopher Covington <hidden> Date: 2012-07-26 19:19:55
Hi Stefano,
On 07/26/2012 11:33 AM, 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.
You're not going to JIT assemble the appropriate HVC instruction? Darn.
How many call numbers are there, though? 8? It seems like it'd be
reasonable to take the approach that seems to be favored for MRC/MCR
instructions, using a function containing switch statement that chooses
between several inline assembly instructions based off an enum passed to
the function. See for example arch_timer_reg_read in
arch/arm/kernel/arch_timer.c.
Regards,
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Rather than hacking around this, why not clean it up:
asm/pvclock-abi.h clearly isn't intended to be included here
(from the perspective of the origin of xen/interface/xen.h, at
least), nor is linux/types.h.
Or if it is really needed to deviate from the original header in
this respect, then clearly the inclusion ought to not be arch
specific or be moved to an arch specific header.
Jan
While I assume that this part (and the cpu_hotplug one below) is
temporary, ...
+obj-$(CONFIG_XEN_DOM0) += pci.o acpi.o
... at least this one should imo be solved with a proper long term
mechanism, i.e. the usual var-$(CONFIG_...) approach, i.e.
dom0-$(CONFIG_PCI) := pci.o
dom0-$(CONFIG_ACPI) := acpi.o
obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
Jan
From: Jan Beulich <hidden> Date: 2012-07-27 07:04:19
quoted
quoted
On 26.07.12 at 17:33, Stefano Stabellini [off-list ref] wrote:
In order for privcmd mmap to work correctly, xen_remap_domain_mfn_range
needs to be implemented for HVM guests.
If it is not, mmap is going to fail later on.
Somehow, for me at least, this description doesn't connect to the
actual change.
/* 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;
From: Ian Campbell <hidden> Date: 2012-07-27 09:00:59
On Thu, 2012-07-26 at 17:56 +0100, David Vrabel wrote:
On 26/07/12 16:33, Stefano Stabellini wrote:
quoted
+ * The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
+ * hypercall tag.
Is this number, 0xea1, assigned to Xen by some external body?
The value and semantics of the hvc instructions immediate operand is
entirely up to the hypervisor authors. We could have chosen 0 or some
random number, we went for the latter because it increases the chances,
by some tiny amount, that we won't clash with some other hypervisors ABI
which makes supporting "foreign" guests that bit easier should it even
come to it.
IOW it's arbitrary in the same way that a Linux system calls used to use
int 0x80.
Ian.
From: Ian Campbell <hidden> Date: 2012-07-27 09:17:58
On Thu, 2012-07-26 at 17:33 +0100, Konrad Rzeszutek Wilk wrote:
On Thu, Jul 26, 2012 at 04:33:44PM +0100, Stefano Stabellini wrote:
quoted
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++++++++++++
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/hypercall.S | 65 ++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/include/asm/xen/hypercall.h
create mode 100644 arch/arm/xen/hypercall.S
I think you meant 'and the fourth in r3'.
So where does the similarity end? Just in that we use r12?
The standard ARM function calling convention is arguments 1-4 on r0-r3
and arguments 5+ on the stack. r12 is a scratch register which can be
clobbered by the *linker* on subroutine call (r12 is also called "ip"
the intra-procedure call scratch register).
The hypervisor doesn't want to be accessing hypercall arguments off the
guest stack, for obvious reasons, so we use r4 for the fifth argument
(and if we even implemented 6 argument hypercalls we'd use r5, etc).
There is no equivalent to the hypercall number in the procedure calling
convention so we picked r12 because it is up and out of the way and is
otherwise a scratch register. Obviously that you must not make a
procedure call between setting the hypercall number in r12 and calling
the hvc instruction.
quoted
+ * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
+ * in r4, differently from the procedure calling convention of using the
quoted
+ * 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.
+ *
+ * Stefano Stabellini [off-list ref], Citrix, 2012
+ */
+
+#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
+
+/* We need to save and restore r4, because Xen clobbers it. */
Hmm, the comment says r4, but right below I see r12?
The ARM procedure calling convention allows a subroutine to clobber
r1..r3 (r0 is the return value) but not r4 which must be preserved. But
the hypervisor ABI clobbers all argument registers so the caller has to
specially preserve r4 in this context whenever there is a 5 argument
hypercall.
I presume that none of the hypercalls defined below have 5 arguments and
therefore we don't need to preserve r4 except in the generic
privcmd_call function.
To be honest I prefer the style which we use on x86 which is to define
hypercall{0,1,2,3,4,5} macros and to wrap those with the specific names
using inline functions.
I find the x86 way more self documenting, and being in C prevents errors
around the number of arguments. It also allows for better in-lining and
exposes to gcc the actual clobbers, which might allow it to avoid saving
r4 on the stack at all etc.
Should this comment be by 'privcmd_call'?
When we add a 5 argument hypercall I suppose we'll see the required
push/pop of r4 added to this macro too.
+ mov pc, lr
+ENDPROC(privcmd_call);
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
http://lists.xen.org/xen-devel
From: Ian Campbell <hidden> Date: 2012-07-27 09:19:59
On Thu, 2012-07-26 at 20:19 +0100, Christopher Covington wrote:
Hi Stefano,
On 07/26/2012 11:33 AM, Stefano Stabellini wrote:
quoted
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.
You're not going to JIT assemble the appropriate HVC instruction? Darn.
;-)
How many call numbers are there, though? 8?
The maximum currently defined hypercall number is 55, although there are
some small gaps so there's actually more like 45 in total.
It seems like it'd be
reasonable to take the approach that seems to be favored for MRC/MCR
instructions, using a function containing switch statement that chooses
between several inline assembly instructions based off an enum passed to
the function. See for example arch_timer_reg_read in
arch/arm/kernel/arch_timer.c.
I don't think it is feasible with this number of hypercalls, even
accepting that in many cases the number will be a constant so gcc can
likely optimise almost all of it away.
Is there something wrong with the r12 based approach?
Ian.
From: Ian Campbell <hidden> Date: 2012-07-27 09:28:29
On Thu, 2012-07-26 at 17:37 +0100, Konrad Rzeszutek Wilk wrote:
On Thu, Jul 26, 2012 at 04:33:46PM +0100, Stefano Stabellini wrote:
quoted
sync_bitops functions are equivalent to the SMP implementation of the
original functions, independently from CONFIG_SMP being defined.
So why can't the code be changed to use that? Is it that
the _set_bit, _clear_bit, etc are not available with !CONFIG_SMP?
_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.
The users are common code and the sync_foo vs foo distinction matters on
some platforms (e.g. x86 where a UP kernel would omit the LOCK prefix
for the normal ones).
From: Ian Campbell <hidden> Date: 2012-07-27 09:37:10
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted hunk
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
@@ -33,3 +36,56 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,return-ENOSYS;}EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);++/*+*==XenDeviceTreeformat==+*-/xennode;+*-compatible"arm,xen";+*-oneinterruptforXeneventnotifications;+*-onememoryregiontomapthegrant_table.+*/+staticint__initxen_guest_init(void)+{+intcpu;+structxen_add_to_physmapxatp;+staticstructshared_info*shared_info_page=0;+structdevice_node*node;++node=of_find_compatible_node(NULL,NULL,"arm,xen");+if(!node){+pr_info("No Xen support\n");+return0;+}
This should either only print in the success case (to avoid spamming
everyone) or we need a little bit of infrastructure like on x86 so that
we print exactly one of:
"Booting natively on bearmetal"
"Booting paravirtualised on %s", hypervisor->name
+ 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");
+ 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.
+ * When xen_hvm_init_shared_info is run at boot time only vcpu 0 is
+ * online but xen_hvm_init_shared_info is run at resume time too and
+ * in that case multiple vcpus might be online. */
+ for_each_online_cpu(cpu) {
+ per_cpu(xen_vcpu, cpu) =
+ &HYPERVISOR_shared_info->vcpu_info[cpu];
On ARM 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.
Ian.
From: Ian Campbell <hidden> Date: 2012-07-27 09:42:35
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
We used to rely on a core_initcall to initialize Xen on ARM, however
core_initcalls are actually called after early consoles are initialized.
That means that hvc_xen.c is going to be initialized before Xen.
Given the lack of a better alternative, just call a new Xen
initialization function (xen_guest_init) from xen_cons_init.
Can't we just arrange for this to be called super early on from
setup_arch? That's got to be better than calling it from some random
function which happens to get called early enough.
I presume that KVM is going to want some similarly early init hooks etc
and therefore ARM could benefit from the same sort of infrastructure as
is in arch/x86/include/asm/hypervisor.h?
quoted hunk
xen_guest_init has to be arch independent, so write both an ARM and an
x86 implementation. The x86 implementation is currently empty because we
can be sure that xen_hvm_guest_init is called early enough.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 7 ++++++-
arch/x86/xen/enlighten.c | 8 ++++++++
drivers/tty/hvc/hvc_xen.c | 7 ++++++-
include/xen/xen.h | 2 ++
4 files changed, 22 insertions(+), 2 deletions(-)
@@ -1567,4 +1567,12 @@ const struct hypervisor_x86 x86_hyper_xen_hvm __refconst = {.init_platform=xen_hvm_guest_init,};EXPORT_SYMBOL(x86_hyper_xen_hvm);++int__initxen_guest_init(void)+{+/* do nothing: rely on x86_hyper_xen_hvm for the initialization */+return0;++}+EXPORT_SYMBOL_GPL(xen_guest_init);#endif
From: Ian Campbell <hidden> Date: 2012-07-27 09:45:32
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted hunk
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 <redacted>
---
arch/arm/xen/enlighten.c | 7 +++++++
include/xen/interface/features.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)
@@ -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__ */
From: Ian Campbell <hidden> Date: 2012-07-27 09:48:18
On Thu, 2012-07-26 at 16:34 +0100, Stefano Stabellini wrote:
+#define XEN_IO_PROTO_ABI_ARM "arm-abi"
I wonder if we ought to call this arm-aarch32-abi or something?
I wonder if we can also take the opportunity to fix the ABI cockup for
disks on ARM and make the structs the same for both 32 and 64 bit?
Ian.
On Thu, Jul 26, 2012 at 04:33:43PM +0100, Stefano Stabellini wrote:
quoted
- Basic hypervisor.h and interface.h definitions.
- Skelethon enlighten.c, set xen_start_info to an empty struct.
Skeleton
quoted
- Do not limit xen_initial_domain to PV guests.
Better wording: Make xen_initial_domain dependent on the SIF_PRIVILIGED_BIT.
Which reminds me - what about PV guests that do PCI passthrough. Aren't
they "more" priviligied than normal PV guests? Or not really?
Not really, from the Xen POV.
quoted
The new code only compiles when CONFIG_XEN is set, that is going to be
added to arch/arm/Kconfig in a later patch.
@@ -246,6 +246,7 @@ endifcore-$(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/
That is weird. It is 4+4+4 = 12 bytes? Don't you want it to be 16 bytes?
I agree that 16 bytes would be a better choice, but it needs to match
the struct in Xen that is defined as follow:
uint32_t wc_version; /* Version counter: see vcpu_time_info_t. */
uint32_t wc_sec; /* Secs 00:00:00 UTC, Jan 1, 1970. */
uint32_t wc_nsec; /* Nsecs 00:00:00 UTC, Jan 1, 1970. */
in xen/include/public/xen.h.
@@ -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>++structstart_info_xen_start_info;+structstart_info*xen_start_info=&_xen_start_info;+EXPORT_SYMBOL_GPL(xen_start_info);++enumxen_domain_typexen_domain_type=XEN_NATIVE;+EXPORT_SYMBOL_GPL(xen_domain_type);++structshared_infoxen_dummy_shared_info;+structshared_info*HYPERVISOR_shared_info=(void*)&xen_dummy_shared_info;++DEFINE_PER_CPU(structvcpu_info*,xen_vcpu);++/* XXX: to be removed */
In this patch series later on? Or just when you try to collapse the x86 and arm variant
together?
The latter.
It is not harmful but we don't need xen_have_vector_callback on ARM, so it
would be nice if we didn't have to define it here.
quoted
+__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);
From: Christopher Covington <hidden> Date: 2012-07-27 12:56:19
On 07/27/2012 05:19 AM, Ian Campbell wrote:
On Thu, 2012-07-26 at 20:19 +0100, Christopher Covington wrote:
quoted
Hi Stefano,
On 07/26/2012 11:33 AM, Stefano Stabellini wrote:
quoted
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.
You're not going to JIT assemble the appropriate HVC instruction? Darn.
;-)
quoted
How many call numbers are there, though? 8?
The maximum currently defined hypercall number is 55, although there are
some small gaps so there's actually more like 45 in total.
quoted
It seems like it'd be
reasonable to take the approach that seems to be favored for MRC/MCR
instructions, using a function containing switch statement that chooses
between several inline assembly instructions based off an enum passed to
the function. See for example arch_timer_reg_read in
arch/arm/kernel/arch_timer.c.
I don't think it is feasible with this number of hypercalls, even
accepting that in many cases the number will be a constant so gcc can
likely optimise almost all of it away.
Is there something wrong with the r12 based approach?
Only that you're defining a custom interface for something that there is
a potentially more standard interface for. I just wanted to double check
that all the ways of using the potentially more standard interface had
been explored and found to be unreasonable.
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
On Thu, 2012-07-26 at 17:33 +0100, Konrad Rzeszutek Wilk wrote:
quoted
On Thu, Jul 26, 2012 at 04:33:44PM +0100, Stefano Stabellini wrote:
quoted
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++++++++++++
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/hypercall.S | 65 ++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/include/asm/xen/hypercall.h
create mode 100644 arch/arm/xen/hypercall.S
I think you meant 'and the fourth in r3'.
So where does the similarity end? Just in that we use r12?
The standard ARM function calling convention is arguments 1-4 on r0-r3
and arguments 5+ on the stack. r12 is a scratch register which can be
clobbered by the *linker* on subroutine call (r12 is also called "ip"
the intra-procedure call scratch register).
The hypervisor doesn't want to be accessing hypercall arguments off the
guest stack, for obvious reasons, so we use r4 for the fifth argument
(and if we even implemented 6 argument hypercalls we'd use r5, etc).
There is no equivalent to the hypercall number in the procedure calling
convention so we picked r12 because it is up and out of the way and is
otherwise a scratch register. Obviously that you must not make a
procedure call between setting the hypercall number in r12 and calling
the hvc instruction.
quoted
quoted
+ * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
+ * in r4, differently from the procedure calling convention of using the
quoted
+ * 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.
+ *
+ * Stefano Stabellini [off-list ref], Citrix, 2012
+ */
+
+#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
+
+/* We need to save and restore r4, because Xen clobbers it. */
Hmm, the comment says r4, but right below I see r12?
The ARM procedure calling convention allows a subroutine to clobber
r1..r3 (r0 is the return value) but not r4 which must be preserved. But
the hypervisor ABI clobbers all argument registers so the caller has to
specially preserve r4 in this context whenever there is a 5 argument
hypercall.
I presume that none of the hypercalls defined below have 5 arguments and
therefore we don't need to preserve r4 except in the generic
privcmd_call function.
To be honest I prefer the style which we use on x86 which is to define
hypercall{0,1,2,3,4,5} macros and to wrap those with the specific names
using inline functions.
I find the x86 way more self documenting, and being in C prevents errors
around the number of arguments. It also allows for better in-lining and
exposes to gcc the actual clobbers, which might allow it to avoid saving
r4 on the stack at all etc.
Considering that we cannot do the same thing that we do on x86 (see this
thread http://marc.info/?l=linux-kernel&m=133052035426427&w=2), I
decided to go for the assembly implementation because it is much shorter
and easier to understand (for me at least, being just 3 lines of code in
the generic case and just one macro) and this way we can exploit the
code generated by gcc to put the arguments in the right registers.
Also I like the fact that it is the same strategy used by libc to issue
syscalls.
As you can see it results in 3 lines of code for all the hypercalls
except the ones that might take more than 4 arguments, that right now is
just privcmd.
quoted
Should this comment be by 'privcmd_call'?
When we add a 5 argument hypercall I suppose we'll see the required
push/pop of r4 added to this macro too.
For performance and simplicity I would add a second macro that push/pop
r4, only required for hypercalls with more than 4 arguments.
From: Ian Campbell <hidden> Date: 2012-07-27 13:18:59
On Fri, 2012-07-27 at 14:02 +0100, Stefano Stabellini wrote:
On Fri, 27 Jul 2012, Ian Campbell wrote:
quoted
On Thu, 2012-07-26 at 17:33 +0100, Konrad Rzeszutek Wilk wrote:
quoted
On Thu, Jul 26, 2012 at 04:33:44PM +0100, Stefano Stabellini wrote:
quoted
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++++++++++++
arch/arm/xen/Makefile | 2 +-
arch/arm/xen/hypercall.S | 65 ++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/include/asm/xen/hypercall.h
create mode 100644 arch/arm/xen/hypercall.S
I think you meant 'and the fourth in r3'.
So where does the similarity end? Just in that we use r12?
The standard ARM function calling convention is arguments 1-4 on r0-r3
and arguments 5+ on the stack. r12 is a scratch register which can be
clobbered by the *linker* on subroutine call (r12 is also called "ip"
the intra-procedure call scratch register).
The hypervisor doesn't want to be accessing hypercall arguments off the
guest stack, for obvious reasons, so we use r4 for the fifth argument
(and if we even implemented 6 argument hypercalls we'd use r5, etc).
There is no equivalent to the hypercall number in the procedure calling
convention so we picked r12 because it is up and out of the way and is
otherwise a scratch register. Obviously that you must not make a
procedure call between setting the hypercall number in r12 and calling
the hvc instruction.
quoted
quoted
+ * Xen hypercalls have 5 arguments at most, the fifth paramter is passed
+ * in r4, differently from the procedure calling convention of using the
quoted
+ * 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.
+ *
+ * Stefano Stabellini [off-list ref], Citrix, 2012
+ */
+
+#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
+
+/* We need to save and restore r4, because Xen clobbers it. */
Hmm, the comment says r4, but right below I see r12?
The ARM procedure calling convention allows a subroutine to clobber
r1..r3 (r0 is the return value) but not r4 which must be preserved. But
the hypervisor ABI clobbers all argument registers so the caller has to
specially preserve r4 in this context whenever there is a 5 argument
hypercall.
I presume that none of the hypercalls defined below have 5 arguments and
therefore we don't need to preserve r4 except in the generic
privcmd_call function.
To be honest I prefer the style which we use on x86 which is to define
hypercall{0,1,2,3,4,5} macros and to wrap those with the specific names
using inline functions.
I find the x86 way more self documenting, and being in C prevents errors
around the number of arguments. It also allows for better in-lining and
exposes to gcc the actual clobbers, which might allow it to avoid saving
r4 on the stack at all etc.
I
decided to go for the assembly implementation because it is much shorter
and easier to understand (for me at least, being just 3 lines of code in
the generic case and just one macro) and this way we can exploit the
code generated by gcc to put the arguments in the right registers.
Also I like the fact that it is the same strategy used by libc to issue
syscalls.
Fair enough.
As you can see it results in 3 lines of code for all the hypercalls
except the ones that might take more than 4 arguments, that right now is
just privcmd.
quoted
quoted
Should this comment be by 'privcmd_call'?
When we add a 5 argument hypercall I suppose we'll see the required
push/pop of r4 added to this macro too.
For performance and simplicity I would add a second macro that push/pop
r4, only required for hypercalls with more than 4 arguments.
For clarity / documentation purposes it might actually be worthwhile to
define all of HYPERCALL{0,1,2,3,4} even if the {0,1,2,3} cases are all
just:
#define HYPERCALL0(x) HYPERCALL_SIMPLE(x)
When we add a 5 argument hypercall I suppose we'll see the required
push/pop of r4 added to this macro too.
For performance and simplicity I would add a second macro that push/pop
r4, only required for hypercalls with more than 4 arguments.
For clarity / documentation purposes it might actually be worthwhile to
define all of HYPERCALL{0,1,2,3,4} even if the {0,1,2,3} cases are all
just:
#define HYPERCALL0(x) HYPERCALL_SIMPLE(x)
I don't if it makes such a difference, but putting the headers in sorted
order is sometimes nicer than just randomly.. But that might be just
me liking an orderly world nowadays :-)
lookup_address shouldn't be here because it is an x86-only interface.
However both gntdev.c and xenbus_client.c call it (in PV only code paths
that are never taken in a PV on HVM guest), so in order to compile them
I have to define lookup_address.
quoted
+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 */
On Thu, 2012-07-26 at 20:19 +0100, Christopher Covington wrote:
quoted
Hi Stefano,
On 07/26/2012 11:33 AM, Stefano Stabellini wrote:
quoted
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.
You're not going to JIT assemble the appropriate HVC instruction? Darn.
;-)
I admit having spent few hours thinking about how to implement a
self-modifying function able to change the ISS at run time. Fortunately
few hours later I was struck by common sense and I decided to follow a
different direction ;-)
quoted
The maximum currently defined hypercall number is 55, although there are
some small gaps so there's actually more like 45 in total.
quoted
It seems like it'd be
reasonable to take the approach that seems to be favored for MRC/MCR
instructions, using a function containing switch statement that chooses
between several inline assembly instructions based off an enum passed to
the function. See for example arch_timer_reg_read in
arch/arm/kernel/arch_timer.c.
I don't think it is feasible with this number of hypercalls, even
accepting that in many cases the number will be a constant so gcc can
likely optimise almost all of it away.
Is there something wrong with the r12 based approach?
Only that you're defining a custom interface for something that there is
a potentially more standard interface for. I just wanted to double check
that all the ways of using the potentially more standard interface had
been explored and found to be unreasonable.
On 26.07.12 at 17:33, Stefano Stabellini [off-list ref] wrote:
In order for privcmd mmap to work correctly, xen_remap_domain_mfn_range
needs to be implemented for HVM guests.
If it is not, mmap is going to fail later on.
Somehow, for me at least, this description doesn't connect to the
actual change.
We can remove the "return -ENOSYS" from privcmd_mmap but the actual mmap
is still not going to work unless xen_remap_domain_mfn_range is
implemented correctly.
The x86 implementation of xen_remap_domain_mfn_range is PV only so it is
not going to work for HVM or auto_translated_physmap guests.
As a result mmap_batch_fn is going to fail.
It is safe in the sense that is not going to crash dom0 or the
hypervisor, but it is not going to work.
Actually in order for it to be safe we need this additional change:
From: Russell King - ARM Linux <hidden> Date: 2012-07-27 14:21:44
On Fri, Jul 27, 2012 at 02:02:18PM +0100, Stefano Stabellini wrote:
quoted
quoted
quoted
+/******************************************************************************
+ * hypercall.h
+ *
+ * Linux-specific hypervisor handling.
+ *
+ * Stefano Stabellini [off-list ref], 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
Erm, is that an additional restriction on the GPL which prevents me from
shipping this code on a CD and charging for the act of creating the CD
and shipping it? That would technically make the above statement
incompatible with the GPL.
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted
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 <redacted>
---
arch/arm/xen/enlighten.c | 7 +++++++
include/xen/interface/features.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)
@@ -58,6 +59,12 @@ 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);
What happens here on platforms prior to hypervisor changeset 23735?
It wouldn't work.
Considering that we are certainly not going to backport ARM support to
Xen 4.1, and that both ARM and XENFEAT_dom0 will be present in Xen 4.2,
do we really need to support the Xen unstable changesets between ARM was
introduced and XENFEAT_dom0 appeared?
From: Ian Campbell <hidden> Date: 2012-07-27 14:33:54
On Fri, 2012-07-27 at 15:25 +0100, Stefano Stabellini wrote:
On Fri, 27 Jul 2012, Ian Campbell wrote:
quoted
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted
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 <redacted>
---
arch/arm/xen/enlighten.c | 7 +++++++
include/xen/interface/features.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)
@@ -58,6 +59,12 @@ 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);
What happens here on platforms prior to hypervisor changeset 23735?
It wouldn't work.
Considering that we are certainly not going to backport ARM support to
Xen 4.1, and that both ARM and XENFEAT_dom0 will be present in Xen 4.2,
do we really need to support the Xen unstable changesets between ARM was
introduced and XENFEAT_dom0 appeared?
On Fri, 27 Jul 2012, Russell King - ARM Linux wrote:
On Fri, Jul 27, 2012 at 02:02:18PM +0100, Stefano Stabellini wrote:
quoted
quoted
quoted
quoted
+/******************************************************************************
+ * hypercall.h
+ *
+ * Linux-specific hypervisor handling.
+ *
+ * Stefano Stabellini [off-list ref], 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
Erm, is that an additional restriction on the GPL which prevents me from
shipping this code on a CD and charging for the act of creating the CD
and shipping it? That would technically make the above statement
incompatible with the GPL.
IMNAL but this is just an alternative, less strict, MIT license for this
file, same as the x86 counterpart
(arch/x86/include/asm/xen/hypercall.h). The intent is to allow other
operating systems, the BSDs for example, to be able to use it if they
want to.
Actually, given that the ARM implementation is not inline, I should
remember to add this copyright header to the assembly source file too.
From: Ian Campbell <hidden> Date: 2012-07-27 14:39:35
On Fri, 2012-07-27 at 15:21 +0100, Russell King - ARM Linux wrote:
On Fri, Jul 27, 2012 at 02:02:18PM +0100, Stefano Stabellini wrote:
quoted
quoted
quoted
quoted
+/******************************************************************************
+ * hypercall.h
+ *
+ * Linux-specific hypervisor handling.
+ *
+ * Stefano Stabellini [off-list ref], 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
Erm, is that an additional restriction on the GPL which prevents me from
shipping this code on a CD and charging for the act of creating the CD
and shipping it? That would technically make the above statement
incompatible with the GPL.
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
@@ -33,3 +36,56 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,return-ENOSYS;}EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);++/*+*==XenDeviceTreeformat==+*-/xennode;+*-compatible"arm,xen";+*-oneinterruptforXeneventnotifications;+*-onememoryregiontomapthegrant_table.+*/+staticint__initxen_guest_init(void)+{+intcpu;+structxen_add_to_physmapxatp;+staticstructshared_info*shared_info_page=0;+structdevice_node*node;++node=of_find_compatible_node(NULL,NULL,"arm,xen");+if(!node){+pr_info("No Xen support\n");+return0;+}
This should either only print in the success case (to avoid spamming
everyone) or we need a little bit of infrastructure like on x86 so that
we print exactly one of:
"Booting natively on bearmetal"
"Booting paravirtualised on %s", hypervisor->name
This function is only going to be called once (actually it might be
called twice with the change introduced by "xen/arm: Introduce
xen_guest_init").
I thought that it would be an acceptible level of verbosity for pr_info.
Maybe I should just turn the pr_info into pr_debug?
quoted
+ 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");
+ 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.
+ * When xen_hvm_init_shared_info is run at boot time only vcpu 0 is
+ * online but xen_hvm_init_shared_info is run at resume time too and
+ * in that case multiple vcpus might be online. */
+ for_each_online_cpu(cpu) {
+ per_cpu(xen_vcpu, cpu) =
+ &HYPERVISOR_shared_info->vcpu_info[cpu];
On ARM 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.
From: Ian Campbell <hidden> Date: 2012-07-27 14:51:28
On Fri, 2012-07-27 at 15:48 +0100, Stefano Stabellini wrote:
On Fri, 27 Jul 2012, Ian Campbell wrote:
quoted
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
@@ -33,3 +36,56 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma,return-ENOSYS;}EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);++/*+*==XenDeviceTreeformat==+*-/xennode;+*-compatible"arm,xen";+*-oneinterruptforXeneventnotifications;+*-onememoryregiontomapthegrant_table.+*/+staticint__initxen_guest_init(void)+{+intcpu;+structxen_add_to_physmapxatp;+staticstructshared_info*shared_info_page=0;+structdevice_node*node;++node=of_find_compatible_node(NULL,NULL,"arm,xen");+if(!node){+pr_info("No Xen support\n");+return0;+}
This should either only print in the success case (to avoid spamming
everyone) or we need a little bit of infrastructure like on x86 so that
we print exactly one of:
"Booting natively on bearmetal"
"Booting paravirtualised on %s", hypervisor->name
This function is only going to be called once (actually it might be
called twice with the change introduced by "xen/arm: Introduce
xen_guest_init").
Once (or twice), per boot, per ARM system running Linux in the world...
Ian.
From: Russell King - ARM Linux <hidden> Date: 2012-07-27 14:59:24
On Fri, Jul 27, 2012 at 03:39:31PM +0100, Ian Campbell wrote:
On Fri, 2012-07-27 at 15:21 +0100, Russell King - ARM Linux wrote:
quoted
On Fri, Jul 27, 2012 at 02:02:18PM +0100, Stefano Stabellini wrote:
quoted
quoted
quoted
quoted
+/******************************************************************************
+ * hypercall.h
+ *
+ * Linux-specific hypervisor handling.
+ *
+ * Stefano Stabellini [off-list ref], 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
Erm, is that an additional restriction on the GPL which prevents me from
shipping this code on a CD and charging for the act of creating the CD
and shipping it? That would technically make the above statement
incompatible with the GPL.
Ok, but be aware that you won't be able to take code from the Linux
kernel and place it in a file marked with that license header (because
the code authors haven't given permission for it to be placed under any
other license other than GPLv2.)
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted
We used to rely on a core_initcall to initialize Xen on ARM, however
core_initcalls are actually called after early consoles are initialized.
That means that hvc_xen.c is going to be initialized before Xen.
Given the lack of a better alternative, just call a new Xen
initialization function (xen_guest_init) from xen_cons_init.
Can't we just arrange for this to be called super early on from
setup_arch? That's got to be better than calling it from some random
function which happens to get called early enough.
While I agree with you that an explicit call to xen_guest_init from
generic code might be better, xen_cons_init is not just a random
function: it is a console_initcall and therefore we know for sure that
it is going be the first one to be called.
In fact if we didn't want the PV console to work so early we could just
rely on a core_initcall to initialize everything and we wouldn't have
any issues.
In any case if the ARM maintainers agree I could add a generic
hypervisor initialization call the end of setup_arch.
I presume that KVM is going to want some similarly early init hooks etc
and therefore ARM could benefit from the same sort of infrastructure as
is in arch/x86/include/asm/hypervisor.h?
From: Ian Campbell <hidden> Date: 2012-07-27 16:12:05
On Fri, 2012-07-27 at 16:54 +0100, Stefano Stabellini wrote:
On Fri, 27 Jul 2012, Ian Campbell wrote:
quoted
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted
We used to rely on a core_initcall to initialize Xen on ARM, however
core_initcalls are actually called after early consoles are initialized.
That means that hvc_xen.c is going to be initialized before Xen.
Given the lack of a better alternative, just call a new Xen
initialization function (xen_guest_init) from xen_cons_init.
Can't we just arrange for this to be called super early on from
setup_arch? That's got to be better than calling it from some random
function which happens to get called early enough.
While I agree with you that an explicit call to xen_guest_init from
generic code might be better, xen_cons_init is not just a random
function: it is a console_initcall and therefore we know for sure that
it is going be the first one to be called.
Initialising something != console in a console_initcall just because it
happens to be called early enough meets my definition of calling it from
a random place.
In fact if we didn't want the PV console to work so early we could just
rely on a core_initcall to initialize everything and we wouldn't have
any issues.
In any case if the ARM maintainers agree I could add a generic
hypervisor initialization call the end of setup_arch.
quoted
I presume that KVM is going to want some similarly early init hooks etc
and therefore ARM could benefit from the same sort of infrastructure as
is in arch/x86/include/asm/hypervisor.h?
Rather than hacking around this, why not clean it up:
asm/pvclock-abi.h clearly isn't intended to be included here
(from the perspective of the origin of xen/interface/xen.h, at
least), nor is linux/types.h.
Or if it is really needed to deviate from the original header in
this respect, then clearly the inclusion ought to not be arch
specific or be moved to an arch specific header.
I think you are right: linux/types.h is not actually needed and
pvclock-abi.h should probably be included by asm/xen/interface.h.
While I assume that this part (and the cpu_hotplug one below) is
temporary, ...
quoted
+obj-$(CONFIG_XEN_DOM0) += pci.o acpi.o
... at least this one should imo be solved with a proper long term
mechanism, i.e. the usual var-$(CONFIG_...) approach, i.e.
dom0-$(CONFIG_PCI) := pci.o
dom0-$(CONFIG_ACPI) := acpi.o
obj-$(CONFIG_XEN_DOM0) += $(dom0-y)
That is weird. It is 4+4+4 = 12 bytes? Don't you want it to be 16 bytes?
I agree that 16 bytes would be a better choice, but it needs to match
the struct in Xen that is defined as follow:
uint32_t wc_version; /* Version counter: see vcpu_time_info_t. */
uint32_t wc_sec; /* Secs 00:00:00 UTC, Jan 1, 1970. */
uint32_t wc_nsec; /* Nsecs 00:00:00 UTC, Jan 1, 1970. */
Would it make sense to add some paddigin then at least? In both
cases? Or is it too late for this?
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-08-01 14:29:23
On Thu, Jul 26, 2012 at 04:33:49PM +0100, Stefano Stabellini wrote:
quoted hunk
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.
Signed-off-by: Stefano Stabellini <redacted>
---
arch/arm/xen/enlighten.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
+ 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.
+ * When xen_hvm_init_shared_info is run at boot time only vcpu 0 is
+ * online but xen_hvm_init_shared_info is run at resume time too and
+ * in that case multiple vcpus might be online. */
+ for_each_online_cpu(cpu) {
+ per_cpu(xen_vcpu, cpu) =
+ &HYPERVISOR_shared_info->vcpu_info[cpu];
+ }
+ return 0;
This above looks stringly similar to the x86 one. Could it be
abstracted away to share the same code? Or is that something that
ought to be done later on when there is more meat on the bone?
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-08-01 14:32:20
On Thu, Jul 26, 2012 at 04:33:50PM +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.
How come?
Bring back xen_pfn_t and let each architecture define xen_pfn_t as they
see fit.
I am OK with this as long as your include a comment in both of the
interface.h saying why this is needed. I am curious why 'unsinged long'
won't work? Is it b/c on ARM you always want a 64-bit type?
@@ -25,6 +25,7 @@}while(0)#ifndef __ASSEMBLY__+typedefuint64_txen_pfn_t;/* Guest handles for primitive C types. */__DEFINE_GUEST_HANDLE(uchar,unsignedchar);__DEFINE_GUEST_HANDLE(uint,unsignedint);
@@ -35,6 +36,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
@@ -31,7 +31,7 @@ struct xen_memory_reservation {*OUT:GMFNbasesofextentsthatwereallocated*(NB.Thiscommandalsoupdatesthemach_to_phystranslationtable)*/-GUEST_HANDLE(ulong)extent_start;+GUEST_HANDLE(xen_pfn_t)extent_start;/* Number of extents, and size/alignment of each (2^extent_order pages). */unsignedlongnr_extents;
@@ -432,11 +432,11 @@ struct start_info {unsignedlongnr_pages;/* Total pages allocated to this domain. */unsignedlongshared_info;/* MACHINE address of shared info struct. */uint32_tflags;/* SIF_xxx flags. */-unsignedlongstore_mfn;/* MACHINE page number of shared page. */+xen_pfn_tstore_mfn;/* MACHINE page number of shared page. */uint32_tstore_evtchn;/* Event channel for store communication. */union{struct{-unsignedlongmfn;/* MACHINE page number of console page. */+xen_pfn_tmfn;/* MACHINE page number of console page. */uint32_tevtchn;/* Event channel for console page. */}domU;struct{
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-08-01 14:38:00
On Thu, Jul 26, 2012 at 04:33:51PM +0100, Stefano Stabellini wrote:
quoted hunk
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.
Signed-off-by: Stefano Stabellini <redacted>
---
drivers/xen/xenbus/xenbus_comms.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 27 +++++++++++++++++----------
drivers/xen/xenbus/xenbus_xs.c | 1 +
3 files changed, 19 insertions(+), 11 deletions(-)
@@ -729,16 +729,23 @@ static int __init xenbus_init(void)xenbus_ring_ops_init();if(xen_hvm_domain()){-uint64_tv=0;-err=hvm_get_parameter(HVM_PARAM_STORE_EVTCHN,&v);-if(err)-gotoout_error;-xen_store_evtchn=(int)v;-err=hvm_get_parameter(HVM_PARAM_STORE_PFN,&v);-if(err)-gotoout_error;-xen_store_mfn=(unsignedlong)v;-xen_store_interface=ioremap(xen_store_mfn<<PAGE_SHIFT,PAGE_SIZE);+if(xen_initial_domain()){+err=xenstored_local_init();+xen_store_interface=+phys_to_virt(xen_store_mfn<<PAGE_SHIFT);+}else{+uint64_tv=0;+err=hvm_get_parameter(HVM_PARAM_STORE_EVTCHN,&v);+if(err)+gotoout_error;+xen_store_evtchn=(int)v;+err=hvm_get_parameter(HVM_PARAM_STORE_PFN,&v);+if(err)+gotoout_error;+xen_store_mfn=(unsignedlong)v;+xen_store_interface=+ioremap(xen_store_mfn<<PAGE_SHIFT,PAGE_SIZE);+}
This, and along with the Hybrid PV dom0 (not yet posted, but it was doing
similar manipulation here) is getting more and more like a rat-mess.
Any chance we can just abstract the three different XenStore access
ways and just have something like this:
enum {
USE_UNKNOWN
USE_HVM,
USE_PV,
USE_LOCAL
USE_ALREADY_INIT
};
int usage = USE_UNKNOWN;
if (xen_pv_domain())
usage = USE_PV;
if (xen_hvm_domain())
usage = USE_HVM;
if (xen_initial_domain())
usage = USE_LOCAL;
if (xen_start_info->store_evtchn)
usage = USE_ALREADY_INIT;
.. other overwrites..
switch (usage) {
.. blah blah.
}
What is the benefit of this ? I was hoping at some point to rip out all of those
XEN_DOM0 and just have, mostly,
CONFIG_XEN_BACKEND_SUPPORT
(which would compile whatever is needed for HVM or PV guests to run
blkback/netback/grant/grantalloc/etc)
CONFIG_XEN_FRONTEND_SUPPORT
(the vice-versa)
CONFIG_XEN_PCI
which would have the PCI support, the ACPI routing (which is
predomaintaly most of the dom0 support), VGA text support, and
whatever else is in there.
In that fashion you could compile a kernel with CONFIG_XEN_BACKEND_SUPPORT
without any CONFIG_XEN_PCI and drop it in as an HVM device driver domain.
Thought maybe that wouldn't really work as if you do PCI passthrough to such
domain, you are going to need the PCI support and ACPI routing. The VGA text
maybe not...
OK, never mind - we should brainstorm it and figure out how to make this
nicely work. In the meantime this is OK.
+
+config XEN
+ bool "Xen guest support on ARM"
+ depends on ARM && OF
+ select XEN_DOM0
+ help
+ Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
+
endmenu
menu "Userspace binary formats"
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
http://lists.xen.org/xen-devel
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-08-01 14:45:14
On Fri, Jul 27, 2012 at 10:28:25AM +0100, Ian Campbell wrote:
On Thu, 2012-07-26 at 17:37 +0100, Konrad Rzeszutek Wilk wrote:
quoted
On Thu, Jul 26, 2012 at 04:33:46PM +0100, Stefano Stabellini wrote:
quoted
sync_bitops functions are equivalent to the SMP implementation of the
original functions, independently from CONFIG_SMP being defined.
So why can't the code be changed to use that? Is it that
the _set_bit, _clear_bit, etc are not available with !CONFIG_SMP?
_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.
The users are common code and the sync_foo vs foo distinction matters on
some platforms (e.g. x86 where a UP kernel would omit the LOCK prefix
for the normal ones).
OK, that makes sense. Stefano can you include that comment in the git
commit description and in the sync_bitops.h file please?
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-08-01 14:47:39
On Thu, Jul 26, 2012 at 04:33:54PM +0100, Stefano Stabellini wrote:
We used to rely on a core_initcall to initialize Xen on ARM, however
core_initcalls are actually called after early consoles are initialized.
That means that hvc_xen.c is going to be initialized before Xen.
Given the lack of a better alternative, just call a new Xen
initialization function (xen_guest_init) from xen_cons_init.
xen_guest_init has to be arch independent, so write both an ARM and an
x86 implementation. The x86 implementation is currently empty because we
can be sure that xen_hvm_guest_init is called early enough.
Should the arm version then not be anymore on the core_initcall then?
@@ -1567,4 +1567,12 @@ const struct hypervisor_x86 x86_hyper_xen_hvm __refconst = {.init_platform=xen_hvm_guest_init,};EXPORT_SYMBOL(x86_hyper_xen_hvm);++int__initxen_guest_init(void)+{+/* do nothing: rely on x86_hyper_xen_hvm for the initialization */+return0;++}+EXPORT_SYMBOL_GPL(xen_guest_init);#endif
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-08-01 14:49:09
On Fri, Jul 27, 2012 at 03:33:50PM +0100, Ian Campbell wrote:
On Fri, 2012-07-27 at 15:25 +0100, Stefano Stabellini wrote:
quoted
On Fri, 27 Jul 2012, Ian Campbell wrote:
quoted
On Thu, 2012-07-26 at 16:33 +0100, Stefano Stabellini wrote:
quoted
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 <redacted>
---
arch/arm/xen/enlighten.c | 7 +++++++
include/xen/interface/features.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)
@@ -58,6 +59,12 @@ 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);
What happens here on platforms prior to hypervisor changeset 23735?
It wouldn't work.
Considering that we are certainly not going to backport ARM support to
Xen 4.1, and that both ARM and XENFEAT_dom0 will be present in Xen 4.2,
do we really need to support the Xen unstable changesets between ARM was
introduced and XENFEAT_dom0 appeared?
From: Konrad Rzeszutek Wilk <hidden> Date: 2012-08-01 14:53:39
On Thu, Jul 26, 2012 at 04:33:57PM +0100, Stefano Stabellini wrote:
quoted hunk
Compile events.c on ARM.
Parse, map and enable the IRQ to get event notifications from the device
tree (node "/xen").
On ARM Linux irqs are not enabled by default:
- call enable_percpu_irq for xen_events_irq (drivers are supposed
to call enable_irq after request_irq);
- 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 <redacted>
---
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 | 18 +++++++++++++++---
include/xen/events.h | 2 ++
6 files changed, 52 insertions(+), 4 deletions(-)
@@ -834,6 +839,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)structirq_info*info=info_for_irq(irq);WARN_ON(info==NULL||info->type!=IRQT_EVTCHN);}+irq_clear_status_flags(irq,IRQ_NOREQUEST|IRQ_NOAUTOEN);
I feel that this should be its own commit by itself. I am not certain
of the implication of this on x86 and I think it deserves some explanation.
@@ -1811,6 +1820,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());
@@ -109,4 +109,6 @@ int xen_irq_from_gsi(unsigned gsi);/* Determine whether to ignore this IRQ if it is passed to a guest. */intxen_test_irq_shared(intirq);+/* initialize Xen IRQ subsystem */+voidxen_init_IRQ(void);#endif /* _XEN_EVENTS_H */