| Message ID | 1399305623-22016-7-git-send-email-robherring2@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 5 May 2014 17:00, Rob Herring <robherring2@gmail.com> wrote: > From: Rob Herring <rob.herring@linaro.org> > > Now that we have PSCI emulation, enable it for the virt platform. > This simplifies the virt machine a bit now that PSCI and SMP no longer > need to be KVM only features. > > Signed-off-by: Rob Herring <rob.herring@linaro.org> > --- > > Note: This will need to be rebased as comments on KVM PSCI 0.2 support > are addressed. This patch effectively includes my comments. > > hw/arm/virt.c | 42 ++++++++++++++++++------------------------ > 1 file changed, 18 insertions(+), 24 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 9f28c12..d8ab88d 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -181,21 +181,22 @@ static void create_fdt(VirtBoardInfo *vbi) > "clk24mhz"); > qemu_fdt_setprop_cell(fdt, "/apb-pclk", "phandle", vbi->clock_phandle); > > - /* No PSCI for TCG yet */ > - if (kvm_enabled()) { > - qemu_fdt_add_subnode(fdt, "/psci"); > - if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) { > - qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci-0.2"); > - } else { > - qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); > - qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", > - PSCI_FN_CPU_SUSPEND); > - qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", PSCI_FN_CPU_OFF); > - qemu_fdt_setprop_cell(fdt, "/psci", "cpu_on", PSCI_FN_CPU_ON); > - qemu_fdt_setprop_cell(fdt, "/psci", "migrate", PSCI_FN_MIGRATE); > - } > - qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc"); > + qemu_fdt_add_subnode(fdt, "/psci"); > + qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc"); > + if (kvm_enabled() && !kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) { > + qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); > + } else { > + const char compat[] = "arm,psci-0.2\0arm,psci"; > + qemu_fdt_setprop(fdt, "/psci", "compatible", compat, sizeof(compat)); > } My suggestion to Pranav was that we abstract away the "which PSCI version?" decision into a field in ARMCPU, in which case we can just have TCG always set it to 0.2. So some of this logic will get a little simpler on rebase. thanks -- PMM
On Wed, May 14, 2014 at 12:51 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 5 May 2014 17:00, Rob Herring <robherring2@gmail.com> wrote: >> From: Rob Herring <rob.herring@linaro.org> >> >> Now that we have PSCI emulation, enable it for the virt platform. >> This simplifies the virt machine a bit now that PSCI and SMP no longer >> need to be KVM only features. [...] >> + qemu_fdt_add_subnode(fdt, "/psci"); >> + qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc"); >> + if (kvm_enabled() && !kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) { >> + qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); >> + } else { >> + const char compat[] = "arm,psci-0.2\0arm,psci"; >> + qemu_fdt_setprop(fdt, "/psci", "compatible", compat, sizeof(compat)); >> } > > My suggestion to Pranav was that we abstract away the "which PSCI > version?" decision into a field in ARMCPU, in which case we can > just have TCG always set it to 0.2. So some of this logic > will get a little simpler on rebase. You can't. You have to support both because you don't know what the kernel supports. An old kernel will only support arm,psci. Rob
On 14 May 2014 20:15, Rob Herring <rob.herring@linaro.org> wrote: > On Wed, May 14, 2014 at 12:51 PM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> My suggestion to Pranav was that we abstract away the "which PSCI >> version?" decision into a field in ARMCPU, in which case we can >> just have TCG always set it to 0.2. So some of this logic >> will get a little simpler on rebase. > > You can't. You have to support both because you don't know what the > kernel supports. An old kernel will only support arm,psci. An old host kernel, or an old guest kernel? The former is fine, because the KVM CPU init code will just ask for the KVM capability and fill in the ARMCPU field appropriately. For the latter, how are you supposed to determine what the guest kernel can support? thanks -- PMM
On Wed, May 14, 2014 at 4:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 14 May 2014 20:15, Rob Herring <rob.herring@linaro.org> wrote: >> On Wed, May 14, 2014 at 12:51 PM, Peter Maydell >> <peter.maydell@linaro.org> wrote: >>> My suggestion to Pranav was that we abstract away the "which PSCI >>> version?" decision into a field in ARMCPU, in which case we can >>> just have TCG always set it to 0.2. So some of this logic >>> will get a little simpler on rebase. >> >> You can't. You have to support both because you don't know what the >> kernel supports. An old kernel will only support arm,psci. > > An old host kernel, or an old guest kernel? The former is fine, > because the KVM CPU init code will just ask for the KVM > capability and fill in the ARMCPU field appropriately. > For the latter, how are you supposed to determine what the > guest kernel can support? Guest kernels and this was exactly my point that you can't determine it. The virt dtb is for the guest kernel and must be either 0.1 PSCI only or both 0.1 and 0.2. I think I misread what you meant. Reading the other thread, as long as you just mean changing the if statement like this, then we are in agreement: if (psci version is 0.1) { qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); } else { const char compat[] = "arm,psci-0.2\0arm,psci"; qemu_fdt_setprop(fdt, "/psci", "compatible", compat, sizeof(compat)); } Rob
On 14 May 2014 23:58, Rob Herring <rob.herring@linaro.org> wrote: > On Wed, May 14, 2014 at 4:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> An old host kernel, or an old guest kernel? The former is fine, >> because the KVM CPU init code will just ask for the KVM >> capability and fill in the ARMCPU field appropriately. >> For the latter, how are you supposed to determine what the >> guest kernel can support? > > Guest kernels and this was exactly my point that you can't determine > it. The virt dtb is for the guest kernel and must be either 0.1 PSCI > only or both 0.1 and 0.2. I think I misread what you meant. Reading > the other thread, as long as you just mean changing the if statement > like this, then we are in agreement: > > if (psci version is 0.1) { > qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); > } else { > const char compat[] = "arm,psci-0.2\0arm,psci"; > qemu_fdt_setprop(fdt, "/psci", "compatible", compat, sizeof(compat)); > } Yes, that's all I meant; sorry for the confusion. I hadn't noticed that we announce and support both the 0.1 and 0.2 interfaces in the else {} branch, which is probably why my phrasing was confusing. thanks -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 9f28c12..d8ab88d 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -181,21 +181,22 @@ static void create_fdt(VirtBoardInfo *vbi) "clk24mhz"); qemu_fdt_setprop_cell(fdt, "/apb-pclk", "phandle", vbi->clock_phandle); - /* No PSCI for TCG yet */ - if (kvm_enabled()) { - qemu_fdt_add_subnode(fdt, "/psci"); - if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) { - qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci-0.2"); - } else { - qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); - qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", - PSCI_FN_CPU_SUSPEND); - qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", PSCI_FN_CPU_OFF); - qemu_fdt_setprop_cell(fdt, "/psci", "cpu_on", PSCI_FN_CPU_ON); - qemu_fdt_setprop_cell(fdt, "/psci", "migrate", PSCI_FN_MIGRATE); - } - qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc"); + qemu_fdt_add_subnode(fdt, "/psci"); + qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc"); + if (kvm_enabled() && !kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) { + qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); + } else { + const char compat[] = "arm,psci-0.2\0arm,psci"; + qemu_fdt_setprop(fdt, "/psci", "compatible", compat, sizeof(compat)); } + qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", + QEMU_PSCI_FN_CPU_SUSPEND); + qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", + QEMU_PSCI_FN_CPU_OFF); + qemu_fdt_setprop_cell(fdt, "/psci", "cpu_on", + QEMU_PSCI_FN_CPU_ON); + qemu_fdt_setprop_cell(fdt, "/psci", "migrate", + QEMU_PSCI_FN_MIGRATE); } static void fdt_add_timer_nodes(const VirtBoardInfo *vbi) @@ -410,16 +411,6 @@ static void machvirt_init(QEMUMachineInitArgs *args) vbi->smp_cpus = smp_cpus; - /* - * Only supported method of starting secondary CPUs is PSCI and - * PSCI is not yet supported with TCG, so limit smp_cpus to 1 - * if we're not using KVM. - */ - if (!kvm_enabled() && smp_cpus > 1) { - error_report("mach-virt: must enable KVM to use multiple CPUs"); - exit(1); - } - if (args->ram_size > vbi->memmap[VIRT_MEM].size) { error_report("mach-virt: cannot model more than 30GB RAM"); exit(1); @@ -438,6 +429,9 @@ static void machvirt_init(QEMUMachineInitArgs *args) } cpuobj = object_new(object_class_get_name(oc)); + object_property_set_int(cpuobj, QEMU_PSCI_METHOD_HVC, "psci-method", + NULL); + /* Secondary CPUs start in PSCI powered-down state */ if (n > 0) { object_property_set_bool(cpuobj, true, "start-powered-off", NULL);