| Message ID | 1377801154-29215-5-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 29, 2013 at 07:32:26PM +0100, Stefano Stabellini wrote: > Move Xen initialization earlier, before any DMA requests can be made. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> I guess you should cc the corresponding maintainers here. > arch/arm/include/asm/xen/hypervisor.h | 8 ++++++++ > arch/arm/kernel/setup.c | 2 ++ > arch/arm/xen/enlighten.c | 21 ++++++++++++++------- > arch/arm64/kernel/setup.c | 2 ++ > 4 files changed, 26 insertions(+), 7 deletions(-) [...] > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -53,6 +53,7 @@ > #include <asm/traps.h> > #include <asm/memblock.h> > #include <asm/psci.h> > +#include <asm/xen/hypervisor.h> > > unsigned int processor_id; > EXPORT_SYMBOL(processor_id); > @@ -267,6 +268,7 @@ void __init setup_arch(char **cmdline_p) > unflatten_device_tree(); > > psci_init(); > + xen_early_init(); So Xen guests don't have any hope for single Image? Basically you set dma_ops unconditionally in xen_early_init(), even if the kernel is not intended to run under Xen.
On Thu, 5 Sep 2013, Catalin Marinas wrote: > On Thu, Aug 29, 2013 at 07:32:26PM +0100, Stefano Stabellini wrote: > > Move Xen initialization earlier, before any DMA requests can be made. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > I guess you should cc the corresponding maintainers here. Thanks for the reminder, I'll do that. > > arch/arm/include/asm/xen/hypervisor.h | 8 ++++++++ > > arch/arm/kernel/setup.c | 2 ++ > > arch/arm/xen/enlighten.c | 21 ++++++++++++++------- > > arch/arm64/kernel/setup.c | 2 ++ > > 4 files changed, 26 insertions(+), 7 deletions(-) > > [...] > > > --- a/arch/arm64/kernel/setup.c > > +++ b/arch/arm64/kernel/setup.c > > @@ -53,6 +53,7 @@ > > #include <asm/traps.h> > > #include <asm/memblock.h> > > #include <asm/psci.h> > > +#include <asm/xen/hypervisor.h> > > > > unsigned int processor_id; > > EXPORT_SYMBOL(processor_id); > > @@ -267,6 +268,7 @@ void __init setup_arch(char **cmdline_p) > > unflatten_device_tree(); > > > > psci_init(); > > + xen_early_init(); > > So Xen guests don't have any hope for single Image? Basically you set > dma_ops unconditionally in xen_early_init(), even if the kernel is not > intended to run under Xen. That should not happen: if we are not running on Xen xen_early_init returns early, before calling xen_mm_init.
On Thu, 2013-09-05 at 17:59 +0100, Stefano Stabellini wrote: > On Thu, 5 Sep 2013, Catalin Marinas wrote: > > On Thu, Aug 29, 2013 at 07:32:26PM +0100, Stefano Stabellini wrote: > > > Move Xen initialization earlier, before any DMA requests can be made. > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > I guess you should cc the corresponding maintainers here. > > Thanks for the reminder, I'll do that. > > > > > arch/arm/include/asm/xen/hypervisor.h | 8 ++++++++ > > > arch/arm/kernel/setup.c | 2 ++ > > > arch/arm/xen/enlighten.c | 21 ++++++++++++++------- > > > arch/arm64/kernel/setup.c | 2 ++ > > > 4 files changed, 26 insertions(+), 7 deletions(-) > > > > [...] > > > > > --- a/arch/arm64/kernel/setup.c > > > +++ b/arch/arm64/kernel/setup.c > > > @@ -53,6 +53,7 @@ > > > #include <asm/traps.h> > > > #include <asm/memblock.h> > > > #include <asm/psci.h> > > > +#include <asm/xen/hypervisor.h> > > > > > > unsigned int processor_id; > > > EXPORT_SYMBOL(processor_id); > > > @@ -267,6 +268,7 @@ void __init setup_arch(char **cmdline_p) > > > unflatten_device_tree(); > > > > > > psci_init(); > > > + xen_early_init(); > > > > So Xen guests don't have any hope for single Image? Basically you set > > dma_ops unconditionally in xen_early_init(), even if the kernel is not > > intended to run under Xen. > > That should not happen: if we are not running on Xen xen_early_init > returns early, before calling xen_mm_init. x96 has a call to init_hypervisor_platform() at approximately this location, which detects and calls the init function for any of Xen, KVM, hyperv and vmware. I guess only Xen and KVM are currently relevant on Linux ARM(64), so perhaps adding similar infrastructure on ARM would be overkill at this point. I don't know if KVM needs such an early C-land hook, I suppose it needs it even earlier so it can set up the hyp mode trampoline from head.S? Ian.
On Fri, Sep 06, 2013 at 09:58:59AM +0100, Ian Campbell wrote: > On Thu, 2013-09-05 at 17:59 +0100, Stefano Stabellini wrote: > > On Thu, 5 Sep 2013, Catalin Marinas wrote: > > > On Thu, Aug 29, 2013 at 07:32:26PM +0100, Stefano Stabellini wrote: > > > > Move Xen initialization earlier, before any DMA requests can be made. > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > I guess you should cc the corresponding maintainers here. > > > > Thanks for the reminder, I'll do that. > > > > > > > > arch/arm/include/asm/xen/hypervisor.h | 8 ++++++++ > > > > arch/arm/kernel/setup.c | 2 ++ > > > > arch/arm/xen/enlighten.c | 21 ++++++++++++++------- > > > > arch/arm64/kernel/setup.c | 2 ++ > > > > 4 files changed, 26 insertions(+), 7 deletions(-) > > > > > > [...] > > > > > > > --- a/arch/arm64/kernel/setup.c > > > > +++ b/arch/arm64/kernel/setup.c > > > > @@ -53,6 +53,7 @@ > > > > #include <asm/traps.h> > > > > #include <asm/memblock.h> > > > > #include <asm/psci.h> > > > > +#include <asm/xen/hypervisor.h> > > > > > > > > unsigned int processor_id; > > > > EXPORT_SYMBOL(processor_id); > > > > @@ -267,6 +268,7 @@ void __init setup_arch(char **cmdline_p) > > > > unflatten_device_tree(); > > > > > > > > psci_init(); > > > > + xen_early_init(); > > > > > > So Xen guests don't have any hope for single Image? Basically you set > > > dma_ops unconditionally in xen_early_init(), even if the kernel is not > > > intended to run under Xen. > > > > That should not happen: if we are not running on Xen xen_early_init > > returns early, before calling xen_mm_init. > > x96 has a call to init_hypervisor_platform() at approximately this > location, which detects and calls the init function for any of Xen, KVM, > hyperv and vmware. I would rather have a core_initcall(xen_early_init()) if possible, rather than hard-coded calls in setup_arch(). This early stuff is DT-driven, so in theory you don't need a specific xen call. The only thing is that you end up with swiotlb_init() and 64MB wasted if the Xen guest does not plan to use them. > I guess only Xen and KVM are currently relevant on Linux ARM(64), so > perhaps adding similar infrastructure on ARM would be overkill at this > point. I don't know if KVM needs such an early C-land hook, I suppose > it needs it even earlier so it can set up the hyp mode trampoline from > head.S? head.S installs a Hyp stub if it starts in that mode and then forget about. Later when KVM is initialised it installs its own code by doing an HVC call.
On Fri, Sep 06, 2013 at 03:09:21PM +0100, Catalin Marinas wrote: > On Fri, Sep 06, 2013 at 09:58:59AM +0100, Ian Campbell wrote: > > On Thu, 2013-09-05 at 17:59 +0100, Stefano Stabellini wrote: > > > On Thu, 5 Sep 2013, Catalin Marinas wrote: > > > > On Thu, Aug 29, 2013 at 07:32:26PM +0100, Stefano Stabellini wrote: > > > > > Move Xen initialization earlier, before any DMA requests can be made. > > > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > > > I guess you should cc the corresponding maintainers here. > > > > > > Thanks for the reminder, I'll do that. > > > > > > > > > > > arch/arm/include/asm/xen/hypervisor.h | 8 ++++++++ > > > > > arch/arm/kernel/setup.c | 2 ++ > > > > > arch/arm/xen/enlighten.c | 21 ++++++++++++++------- > > > > > arch/arm64/kernel/setup.c | 2 ++ > > > > > 4 files changed, 26 insertions(+), 7 deletions(-) > > > > > > > > [...] > > > > > > > > > --- a/arch/arm64/kernel/setup.c > > > > > +++ b/arch/arm64/kernel/setup.c > > > > > @@ -53,6 +53,7 @@ > > > > > #include <asm/traps.h> > > > > > #include <asm/memblock.h> > > > > > #include <asm/psci.h> > > > > > +#include <asm/xen/hypervisor.h> > > > > > > > > > > unsigned int processor_id; > > > > > EXPORT_SYMBOL(processor_id); > > > > > @@ -267,6 +268,7 @@ void __init setup_arch(char **cmdline_p) > > > > > unflatten_device_tree(); > > > > > > > > > > psci_init(); > > > > > + xen_early_init(); > > > > > > > > So Xen guests don't have any hope for single Image? Basically you set > > > > dma_ops unconditionally in xen_early_init(), even if the kernel is not > > > > intended to run under Xen. > > > > > > That should not happen: if we are not running on Xen xen_early_init > > > returns early, before calling xen_mm_init. > > > > x96 has a call to init_hypervisor_platform() at approximately this > > location, which detects and calls the init function for any of Xen, KVM, > > hyperv and vmware. > > I would rather have a core_initcall(xen_early_init()) if possible, > rather than hard-coded calls in setup_arch(). This early stuff is > DT-driven, so in theory you don't need a specific xen call. The only > thing is that you end up with swiotlb_init() and 64MB wasted if the Xen > guest does not plan to use them. There is a swiotlb_free mechanism in case the allocation was not neccessary. > > > I guess only Xen and KVM are currently relevant on Linux ARM(64), so > > perhaps adding similar infrastructure on ARM would be overkill at this > > point. I don't know if KVM needs such an early C-land hook, I suppose > > it needs it even earlier so it can set up the hyp mode trampoline from > > head.S? > > head.S installs a Hyp stub if it starts in that mode and then forget > about. Later when KVM is initialised it installs its own code by doing > an HVC call. > > -- > Catalin
On Fri, 6 Sep 2013, Konrad Rzeszutek Wilk wrote: > On Fri, Sep 06, 2013 at 03:09:21PM +0100, Catalin Marinas wrote: > > On Fri, Sep 06, 2013 at 09:58:59AM +0100, Ian Campbell wrote: > > > On Thu, 2013-09-05 at 17:59 +0100, Stefano Stabellini wrote: > > > > On Thu, 5 Sep 2013, Catalin Marinas wrote: > > > > > On Thu, Aug 29, 2013 at 07:32:26PM +0100, Stefano Stabellini wrote: > > > > > > Move Xen initialization earlier, before any DMA requests can be made. > > > > > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > > > > > I guess you should cc the corresponding maintainers here. > > > > > > > > Thanks for the reminder, I'll do that. > > > > > > > > > > > > > > arch/arm/include/asm/xen/hypervisor.h | 8 ++++++++ > > > > > > arch/arm/kernel/setup.c | 2 ++ > > > > > > arch/arm/xen/enlighten.c | 21 ++++++++++++++------- > > > > > > arch/arm64/kernel/setup.c | 2 ++ > > > > > > 4 files changed, 26 insertions(+), 7 deletions(-) > > > > > > > > > > [...] > > > > > > > > > > > --- a/arch/arm64/kernel/setup.c > > > > > > +++ b/arch/arm64/kernel/setup.c > > > > > > @@ -53,6 +53,7 @@ > > > > > > #include <asm/traps.h> > > > > > > #include <asm/memblock.h> > > > > > > #include <asm/psci.h> > > > > > > +#include <asm/xen/hypervisor.h> > > > > > > > > > > > > unsigned int processor_id; > > > > > > EXPORT_SYMBOL(processor_id); > > > > > > @@ -267,6 +268,7 @@ void __init setup_arch(char **cmdline_p) > > > > > > unflatten_device_tree(); > > > > > > > > > > > > psci_init(); > > > > > > + xen_early_init(); > > > > > > > > > > So Xen guests don't have any hope for single Image? Basically you set > > > > > dma_ops unconditionally in xen_early_init(), even if the kernel is not > > > > > intended to run under Xen. > > > > > > > > That should not happen: if we are not running on Xen xen_early_init > > > > returns early, before calling xen_mm_init. > > > > > > x96 has a call to init_hypervisor_platform() at approximately this > > > location, which detects and calls the init function for any of Xen, KVM, > > > hyperv and vmware. > > > > I would rather have a core_initcall(xen_early_init()) if possible, > > rather than hard-coded calls in setup_arch(). This early stuff is > > DT-driven, so in theory you don't need a specific xen call. The only > > thing is that you end up with swiotlb_init() and 64MB wasted if the Xen > > guest does not plan to use them. > > There is a swiotlb_free mechanism in case the allocation was not > neccessary. I think I'll just use the late swiotlb initialization.
diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h index d7ab99a..2782591 100644 --- a/arch/arm/include/asm/xen/hypervisor.h +++ b/arch/arm/include/asm/xen/hypervisor.h @@ -1,6 +1,8 @@ #ifndef _ASM_ARM_XEN_HYPERVISOR_H #define _ASM_ARM_XEN_HYPERVISOR_H +#include <linux/init.h> + extern struct shared_info *HYPERVISOR_shared_info; extern struct start_info *xen_start_info; @@ -16,4 +18,10 @@ static inline enum paravirt_lazy_mode paravirt_get_lazy_mode(void) return PARAVIRT_LAZY_NONE; } +#ifdef CONFIG_XEN +void __init xen_early_init(void); +#else +static inline void xen_early_init(void) { return; } +#endif + #endif /* _ASM_ARM_XEN_HYPERVISOR_H */ diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 63af9a7..cb7b8e2 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -45,6 +45,7 @@ #include <asm/cacheflush.h> #include <asm/cachetype.h> #include <asm/tlbflush.h> +#include <asm/xen/hypervisor.h> #include <asm/prom.h> #include <asm/mach/arch.h> @@ -889,6 +890,7 @@ void __init setup_arch(char **cmdline_p) arm_dt_init_cpu_maps(); psci_init(); + xen_early_init(); #ifdef CONFIG_SMP if (is_smp()) { if (!mdesc->smp_init || !mdesc->smp_init()) { diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index c9770ba..14d17ab 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -195,21 +195,19 @@ static void xen_power_off(void) * documentation of the Xen Device Tree format. */ #define GRANT_TABLE_PHYSADDR 0 -static int __init xen_guest_init(void) +void __init xen_early_init(void) { - struct xen_add_to_physmap xatp; - static struct shared_info *shared_info_page = 0; + struct resource res; struct device_node *node; int len; const char *s = NULL; const char *version = NULL; const char *xen_prefix = "xen,xen-"; - struct resource res; node = of_find_compatible_node(NULL, NULL, "xen,xen"); if (!node) { pr_debug("No Xen support\n"); - return 0; + return; } s = of_get_property(node, "compatible", &len); if (strlen(xen_prefix) + 3 < len && @@ -217,10 +215,10 @@ static int __init xen_guest_init(void) version = s + strlen(xen_prefix); if (version == NULL) { pr_debug("Xen version not found\n"); - return 0; + return; } if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res)) - return 0; + return; xen_hvm_resume_frames = res.start >> PAGE_SHIFT; xen_events_irq = irq_of_parse_and_map(node, 0); pr_info("Xen %s support found, events_irq=%d gnttab_frame_pfn=%lx\n", @@ -232,6 +230,15 @@ static int __init xen_guest_init(void) xen_start_info->flags |= SIF_INITDOMAIN|SIF_PRIVILEGED; else xen_start_info->flags &= ~(SIF_INITDOMAIN|SIF_PRIVILEGED); +} + +static int __init xen_guest_init(void) +{ + struct xen_add_to_physmap xatp; + static struct shared_info *shared_info_page = 0; + + if (!xen_domain()) + return 0; if (!shared_info_page) shared_info_page = (struct shared_info *) diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index add6ea6..e0d438a 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -53,6 +53,7 @@ #include <asm/traps.h> #include <asm/memblock.h> #include <asm/psci.h> +#include <asm/xen/hypervisor.h> unsigned int processor_id; EXPORT_SYMBOL(processor_id); @@ -267,6 +268,7 @@ void __init setup_arch(char **cmdline_p) unflatten_device_tree(); psci_init(); + xen_early_init(); cpu_logical_map(0) = read_cpuid_mpidr() & MPIDR_HWID_BITMASK; #ifdef CONFIG_SMP
Move Xen initialization earlier, before any DMA requests can be made. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Changes in v3: - add missing __init in xen_early_init declaration. --- arch/arm/include/asm/xen/hypervisor.h | 8 ++++++++ arch/arm/kernel/setup.c | 2 ++ arch/arm/xen/enlighten.c | 21 ++++++++++++++------- arch/arm64/kernel/setup.c | 2 ++ 4 files changed, 26 insertions(+), 7 deletions(-)