| Message ID | 1365167495-18508-4-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 5 Apr 2013, Stefano Stabellini wrote: > Split xen_guest_init in two functions, one of them (xen_early_init) is > going to be called very early from setup_arch. > > Change machine_desc->smp_init to xen_smp_init if Xen is present on the > platform. xen_smp_init just sets smp_ops to psci_smp_ops. I still think the above is redundant, especially since psci_smp_ops has priority over default mdesc->smp_ops. And doing so does break MCPM. But we're apparently going in circle over this issue. Nicolas
On Fri, 5 Apr 2013, Nicolas Pitre wrote: > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > > > Split xen_guest_init in two functions, one of them (xen_early_init) is > > going to be called very early from setup_arch. > > > > Change machine_desc->smp_init to xen_smp_init if Xen is present on the > > platform. xen_smp_init just sets smp_ops to psci_smp_ops. > > I still think the above is redundant, especially since psci_smp_ops has > priority over default mdesc->smp_ops. And doing so does break MCPM. But > we're apparently going in circle over this issue. Did you read my last email on this issue? http://marc.info/?l=linux-arm-kernel&m=136500098703317&w=2 Let's examine the use cases here, assuming that your MCPM patch series has been applied. This is what happens: - No Xen Xen is not running on the platform and a Xen hypervisor node is not available on device tree. Everything keeps working seamlessly, this patch doesn't change anything. - we are running on Xen Xen is running on the platform, we are running as a guest on Xen and an hypervisor node is available on device tree. Let's also assume that there aren't any "arm,cci" compatible nodes on device tree because Xen wouldn't export this kind of information to any guests right now. Therefore PSCI should be used to boot secondary cpus. Because the versatile express machine sets smp_init to vexpress_smp_init_ops, vexpress_smp_init_ops will be called. vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* Xen. With this patch, xen_smp_init will be called instead of vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, therefore *unbreaking* Xen. In fact what makes this patch really necessary is smp_init together with the MCPM series. Do you agree with me?
On Fri, 5 Apr 2013, Stefano Stabellini wrote: > On Fri, 5 Apr 2013, Nicolas Pitre wrote: > > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > > > > > Split xen_guest_init in two functions, one of them (xen_early_init) is > > > going to be called very early from setup_arch. > > > > > > Change machine_desc->smp_init to xen_smp_init if Xen is present on the > > > platform. xen_smp_init just sets smp_ops to psci_smp_ops. > > > > I still think the above is redundant, especially since psci_smp_ops has > > priority over default mdesc->smp_ops. And doing so does break MCPM. But > > we're apparently going in circle over this issue. > > Did you read my last email on this issue? > > http://marc.info/?l=linux-arm-kernel&m=136500098703317&w=2 > > > Let's examine the use cases here, assuming that your MCPM patch series > has been applied. > > > This is what happens: > > - No Xen > Xen is not running on the platform and a Xen hypervisor node is not > available on device tree. > Everything keeps working seamlessly, this patch doesn't change anything. > > - we are running on Xen > Xen is running on the platform, we are running as a guest on Xen and an > hypervisor node is available on device tree. > Let's also assume that there aren't any "arm,cci" compatible nodes on > device tree because Xen wouldn't export this kind of information to any > guests right now. Therefore PSCI should be used to boot secondary cpus. Just in case this isn't clear enough: we don't have big.LITTLE support in Xen right now, not in the hypervisor and certainly not in guests. I'm keen on having big.LITTLE support in the hypervisor (thus some code similar to your MCPM patch series will probably end up in Xen at some point) but I doubt we'll expose big.LITTLE to guests any time soon. It's going to be years, so I am not particularly worried about it right now. > Because the versatile express machine sets smp_init to > vexpress_smp_init_ops, vexpress_smp_init_ops will be called. > vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* > Xen. > > With this patch, xen_smp_init will be called instead of > vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, > therefore *unbreaking* Xen. > > In fact what makes this patch really necessary is smp_init together with > the MCPM series. > > > Do you agree with me? >
On Fri, 5 Apr 2013, Stefano Stabellini wrote: > This is what happens: > > - No Xen > Xen is not running on the platform and a Xen hypervisor node is not > available on device tree. > Everything keeps working seamlessly, this patch doesn't change anything. > > - we are running on Xen > Xen is running on the platform, we are running as a guest on Xen and an > hypervisor node is available on device tree. > Let's also assume that there aren't any "arm,cci" compatible nodes on > device tree because Xen wouldn't export this kind of information to any > guests right now. Therefore PSCI should be used to boot secondary cpus. > Because the versatile express machine sets smp_init to > vexpress_smp_init_ops, vexpress_smp_init_ops will be called. > vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* > Xen. OK I see. > With this patch, xen_smp_init will be called instead of > vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, > therefore *unbreaking* Xen. However that breaks MCPM. > In fact what makes this patch really necessary is smp_init together with > the MCPM series. Yes. > Do you agree with me? On the Xen issue, yes. Nicolas
On Fri, 5 Apr 2013, Stefano Stabellini wrote: > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > > - we are running on Xen > > Xen is running on the platform, we are running as a guest on Xen and an > > hypervisor node is available on device tree. > > Let's also assume that there aren't any "arm,cci" compatible nodes on > > device tree because Xen wouldn't export this kind of information to any > > guests right now. Therefore PSCI should be used to boot secondary cpus. > > Just in case this isn't clear enough: we don't have big.LITTLE support > in Xen right now, not in the hypervisor and certainly not in guests. > I'm keen on having big.LITTLE support in the hypervisor (thus some > code similar to your MCPM patch series will probably end up in Xen at > some point) but I doubt we'll expose big.LITTLE to guests any time > soon. It's going to be years, so I am not particularly worried about > it right now. I fully understand that it is unlikely that a Xen guest will "see" a big.LITTLE environment in the near future. My concern is about a kernel that is _configured_ to run either on a native VExpress machine that might or might not be b.L, or as a Xen guest, in the same zImage binary. Nicolas
On 04/05/2013 02:36 PM, Nicolas Pitre wrote: > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > >> This is what happens: >> >> - No Xen >> Xen is not running on the platform and a Xen hypervisor node is not >> available on device tree. >> Everything keeps working seamlessly, this patch doesn't change anything. >> >> - we are running on Xen >> Xen is running on the platform, we are running as a guest on Xen and an >> hypervisor node is available on device tree. >> Let's also assume that there aren't any "arm,cci" compatible nodes on >> device tree because Xen wouldn't export this kind of information to any >> guests right now. Therefore PSCI should be used to boot secondary cpus. >> Because the versatile express machine sets smp_init to >> vexpress_smp_init_ops, vexpress_smp_init_ops will be called. >> vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* >> Xen. > > OK I see. > >> With this patch, xen_smp_init will be called instead of >> vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, >> therefore *unbreaking* Xen. > > However that breaks MCPM. You mean on bare metal, right? For the bare metal, "xen,xen" property would not be present and xen_smp_init is not used. So the vexpress MCPM ops will be used. Aren't Dom0 cpu's basically virtual cpus? If Xen ever needs the MCPM support, the Xen hook itself can figure out whether to use MCPM support. Rob
On Fri, 5 Apr 2013, Rob Herring wrote: > On 04/05/2013 02:36 PM, Nicolas Pitre wrote: > > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > > > >> This is what happens: > >> > >> - No Xen > >> Xen is not running on the platform and a Xen hypervisor node is not > >> available on device tree. > >> Everything keeps working seamlessly, this patch doesn't change anything. > >> > >> - we are running on Xen > >> Xen is running on the platform, we are running as a guest on Xen and an > >> hypervisor node is available on device tree. > >> Let's also assume that there aren't any "arm,cci" compatible nodes on > >> device tree because Xen wouldn't export this kind of information to any > >> guests right now. Therefore PSCI should be used to boot secondary cpus. > >> Because the versatile express machine sets smp_init to > >> vexpress_smp_init_ops, vexpress_smp_init_ops will be called. > >> vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* > >> Xen. > > > > OK I see. > > > >> With this patch, xen_smp_init will be called instead of > >> vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, > >> therefore *unbreaking* Xen. > > > > However that breaks MCPM. > > You mean on bare metal, right? For the bare metal, "xen,xen" property > would not be present and xen_smp_init is not used. So the vexpress MCPM > ops will be used. Aren't Dom0 cpu's basically virtual cpus? If Xen ever > needs the MCPM support, the Xen hook itself can figure out whether to > use MCPM support. Well, if Xen has its own mdesc distinct from the VExpress one then things are indeed fine. Nicolas
On Fri, 5 Apr 2013, Rob Herring wrote: > On 04/05/2013 02:36 PM, Nicolas Pitre wrote: > > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > > > >> This is what happens: > >> > >> - No Xen > >> Xen is not running on the platform and a Xen hypervisor node is not > >> available on device tree. > >> Everything keeps working seamlessly, this patch doesn't change anything. > >> > >> - we are running on Xen > >> Xen is running on the platform, we are running as a guest on Xen and an > >> hypervisor node is available on device tree. > >> Let's also assume that there aren't any "arm,cci" compatible nodes on > >> device tree because Xen wouldn't export this kind of information to any > >> guests right now. Therefore PSCI should be used to boot secondary cpus. > >> Because the versatile express machine sets smp_init to > >> vexpress_smp_init_ops, vexpress_smp_init_ops will be called. > >> vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* > >> Xen. > > > > OK I see. > > > >> With this patch, xen_smp_init will be called instead of > >> vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, > >> therefore *unbreaking* Xen. > > > > However that breaks MCPM. > > You mean on bare metal, right? For the bare metal, "xen,xen" property > would not be present and xen_smp_init is not used. So the vexpress MCPM > ops will be used. That is correct. > Aren't Dom0 cpu's basically virtual cpus? If Xen ever > needs the MCPM support, the Xen hook itself can figure out whether to > use MCPM support. Right.
On Fri, 5 Apr 2013, Nicolas Pitre wrote: > On Fri, 5 Apr 2013, Rob Herring wrote: > > > On 04/05/2013 02:36 PM, Nicolas Pitre wrote: > > > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > > > > > >> This is what happens: > > >> > > >> - No Xen > > >> Xen is not running on the platform and a Xen hypervisor node is not > > >> available on device tree. > > >> Everything keeps working seamlessly, this patch doesn't change anything. > > >> > > >> - we are running on Xen > > >> Xen is running on the platform, we are running as a guest on Xen and an > > >> hypervisor node is available on device tree. > > >> Let's also assume that there aren't any "arm,cci" compatible nodes on > > >> device tree because Xen wouldn't export this kind of information to any > > >> guests right now. Therefore PSCI should be used to boot secondary cpus. > > >> Because the versatile express machine sets smp_init to > > >> vexpress_smp_init_ops, vexpress_smp_init_ops will be called. > > >> vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* > > >> Xen. > > > > > > OK I see. > > > > > >> With this patch, xen_smp_init will be called instead of > > >> vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, > > >> therefore *unbreaking* Xen. > > > > > > However that breaks MCPM. > > > > You mean on bare metal, right? For the bare metal, "xen,xen" property > > would not be present and xen_smp_init is not used. So the vexpress MCPM > > ops will be used. Aren't Dom0 cpu's basically virtual cpus? If Xen ever > > needs the MCPM support, the Xen hook itself can figure out whether to > > use MCPM support. > > Well, if Xen has its own mdesc distinct from the VExpress one then > things > are indeed fine. It's not about the mdesc: Xen has its own hypervisor node on device tree if and only if Xen is running on the platform, therefore the Xen early hook is never going to do anything at all on native. In other words, this patch should NOT change the behaviour of Linux on native, and if it did, it would be a bug.
On Sat, 6 Apr 2013, Stefano Stabellini wrote: > On Fri, 5 Apr 2013, Nicolas Pitre wrote: > > On Fri, 5 Apr 2013, Rob Herring wrote: > > > > > On 04/05/2013 02:36 PM, Nicolas Pitre wrote: > > > > On Fri, 5 Apr 2013, Stefano Stabellini wrote: > > > > > > > >> This is what happens: > > > >> > > > >> - No Xen > > > >> Xen is not running on the platform and a Xen hypervisor node is not > > > >> available on device tree. > > > >> Everything keeps working seamlessly, this patch doesn't change anything. > > > >> > > > >> - we are running on Xen > > > >> Xen is running on the platform, we are running as a guest on Xen and an > > > >> hypervisor node is available on device tree. > > > >> Let's also assume that there aren't any "arm,cci" compatible nodes on > > > >> device tree because Xen wouldn't export this kind of information to any > > > >> guests right now. Therefore PSCI should be used to boot secondary cpus. > > > >> Because the versatile express machine sets smp_init to > > > >> vexpress_smp_init_ops, vexpress_smp_init_ops will be called. > > > >> vexpress_smp_init_ops sets smp_ops to vexpress_smp_ops, that *break* > > > >> Xen. > > > > > > > > OK I see. > > > > > > > >> With this patch, xen_smp_init will be called instead of > > > >> vexpress_smp_init_ops, and smp_ops will be set to psci_smp_ops, > > > >> therefore *unbreaking* Xen. > > > > > > > > However that breaks MCPM. > > > > > > You mean on bare metal, right? For the bare metal, "xen,xen" property > > > would not be present and xen_smp_init is not used. So the vexpress MCPM > > > ops will be used. Aren't Dom0 cpu's basically virtual cpus? If Xen ever > > > needs the MCPM support, the Xen hook itself can figure out whether to > > > use MCPM support. > > > > Well, if Xen has its own mdesc distinct from the VExpress one then > > things > > are indeed fine. > > It's not about the mdesc: Xen has its own hypervisor node on device tree > if and only if Xen is running on the platform, therefore the Xen early > hook is never going to do anything at all on native. > > In other words, this patch should NOT change the behaviour of Linux on > native, and if it did, it would be a bug. Perfect. Nicolas
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2c3bdce..344e299 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1892,6 +1892,7 @@ config XEN depends on ARM && AEABI && OF depends on CPU_V7 && !CPU_V6 depends on !GENERIC_ATOMIC64 + select ARM_PSCI help Say Y if you want to run Linux in a Virtual Machine on Xen on ARM. diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h index d7ab99a..17b3ea2 100644 --- a/arch/arm/include/asm/xen/hypervisor.h +++ b/arch/arm/include/asm/xen/hypervisor.h @@ -16,4 +16,10 @@ static inline enum paravirt_lazy_mode paravirt_get_lazy_mode(void) return PARAVIRT_LAZY_NONE; } +#ifdef CONFIG_XEN +void 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 2f738cb..336305e 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -55,6 +55,8 @@ #include <asm/unwind.h> #include <asm/memblock.h> #include <asm/virt.h> +#include <asm/xen/hypervisor.h> +#include <xen/xen.h> #include "atags.h" #include "tcm.h" @@ -768,6 +770,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) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index b002822..fc1bc2a 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -11,6 +11,8 @@ #include <xen/xenbus.h> #include <xen/page.h> #include <xen/xen-ops.h> +#include <asm/mach/arch.h> +#include <asm/psci.h> #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include <linux/interrupt.h> @@ -176,11 +178,33 @@ static int __init xen_secondary_init(unsigned int cpu) return 0; } +static void __init xen_smp_init(void) +{ +#ifdef CONFIG_SMP + if (psci_smp_available()) + smp_set_ops(&psci_smp_ops); +#endif +} + /* * see Documentation/devicetree/bindings/arm/xen.txt for the * documentation of the Xen Device Tree format. */ #define GRANT_TABLE_PHYSADDR 0 +void __init xen_early_init(void) +{ + struct device_node *node; + + node = of_find_compatible_node(NULL, NULL, "xen,xen"); + if (!node) { + pr_debug("No Xen support\n"); + return; + } + + xen_domain_type = XEN_HVM_DOMAIN; + machine_desc->smp_init = xen_smp_init; +} + static int __init xen_guest_init(void) { struct xen_add_to_physmap xatp; @@ -194,25 +218,21 @@ static int __init xen_guest_init(void) int i; node = of_find_compatible_node(NULL, NULL, "xen,xen"); - if (!node) { - pr_debug("No Xen support\n"); - return 0; - } + s = of_get_property(node, "compatible", &len); if (strlen(xen_prefix) + 3 < len && !strncmp(xen_prefix, s, strlen(xen_prefix))) version = s + strlen(xen_prefix); if (version == NULL) { - pr_debug("Xen version not found\n"); - return 0; + pr_warn("Xen version not found\n"); + return -EINVAL; } if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res)) - return 0; + return -EINVAL; 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", version, xen_events_irq, xen_hvm_resume_frames); - xen_domain_type = XEN_HVM_DOMAIN; xen_setup_features(); if (xen_feature(XENFEAT_dom0))
Split xen_guest_init in two functions, one of them (xen_early_init) is going to be called very early from setup_arch. Change machine_desc->smp_init to xen_smp_init if Xen is present on the platform. xen_smp_init just sets smp_ops to psci_smp_ops. XEN selects ARM_PSCI. Changes in v6: - xen_smp_init: call smp_set_ops only ifdef CONFIG_SMP; - move more of the initialization to xen_guest_init; - select ARM_PSCI if XEN. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- arch/arm/Kconfig | 1 + arch/arm/include/asm/xen/hypervisor.h | 6 +++++ arch/arm/kernel/setup.c | 3 ++ arch/arm/xen/enlighten.c | 36 +++++++++++++++++++++++++------- 4 files changed, 38 insertions(+), 8 deletions(-)