diff mbox

[RFC,V4,4/6] hw/arm/virt: Use PSCI v0.2 compatible when kernel supports it

Message ID 1399280234-25036-5-git-send-email-pranavkumar@linaro.org
State New
Headers show

Commit Message

Pranavkumar Sawargaonkar May 5, 2014, 8:57 a.m. UTC 
If we have in-kernel emulation of PSCI v0.2 for KVM ARM/ARM64 then
we enable PSCI v0.2 for each VCPU at the time of VCPU init hence we
need to provide PSCI v0.2 compatible string via generated DTB.

This patch updates generated DTB to have PSCI v0.2 compatible string
when we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64.

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Signed-off-by: Anup Patel <anup.patel@linaro.org>
---
 hw/arm/virt.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Rob Herring May 5, 2014, 2:06 p.m. UTC | #1 
On Mon, May 5, 2014 at 3:57 AM, Pranavkumar Sawargaonkar
<pranavkumar@linaro.org> wrote:
> If we have in-kernel emulation of PSCI v0.2 for KVM ARM/ARM64 then
> we enable PSCI v0.2 for each VCPU at the time of VCPU init hence we
> need to provide PSCI v0.2 compatible string via generated DTB.
>
> This patch updates generated DTB to have PSCI v0.2 compatible string
> when we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64.
>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> ---
>  hw/arm/virt.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

This does not compile for me:

  CC    aarch64-softmmu/hw/arm/virt.o
hw/arm/virt.c: In function ‘create_fdt’:
hw/arm/virt.c:186:44: error: ‘KVM_CAP_ARM_PSCI_0_2’ undeclared (first
use in this function)
         if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
                                            ^

>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2bbc931..e4ae8ba 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -185,13 +185,17 @@ static void create_fdt(VirtBoardInfo *vbi)
>      /* No PSCI for TCG yet */
>      if (kvm_enabled()) {
>          qemu_fdt_add_subnode(fdt, "/psci");
> -        qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
> -        qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc");
> -        qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend",
> +        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");

As I mentioned in the last version, this is wrong. You may have an old
guest kernel that only supports PSCI 0.1. You need to support either
PSCI 0.1 only OR both PSCI 0.2 and 0.1.

Rob

> +            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_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");
>      }
>  }
>
> --
> 1.7.9.5
>
Rob Herring May 5, 2014, 2:09 p.m. UTC | #2 
On Mon, May 5, 2014 at 9:06 AM, Rob Herring <robherring2@gmail.com> wrote:
> On Mon, May 5, 2014 at 3:57 AM, Pranavkumar Sawargaonkar
> <pranavkumar@linaro.org> wrote:
>> If we have in-kernel emulation of PSCI v0.2 for KVM ARM/ARM64 then
>> we enable PSCI v0.2 for each VCPU at the time of VCPU init hence we
>> need to provide PSCI v0.2 compatible string via generated DTB.
>>
>> This patch updates generated DTB to have PSCI v0.2 compatible string
>> when we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64.
>>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> ---
>>  hw/arm/virt.c |   16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> This does not compile for me:
>
>   CC    aarch64-softmmu/hw/arm/virt.o
> hw/arm/virt.c: In function ‘create_fdt’:
> hw/arm/virt.c:186:44: error: ‘KVM_CAP_ARM_PSCI_0_2’ undeclared (first
> use in this function)
>          if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
>                                             ^

Note that I am building for system emulation, not KVM which may
explain the difference (I assume it actually builds for you).

Rob
Peter Maydell May 5, 2014, 2:43 p.m. UTC | #3 
On 5 May 2014 15:09, Rob Herring <robherring2@gmail.com> wrote:
> On Mon, May 5, 2014 at 9:06 AM, Rob Herring <robherring2@gmail.com> wrote:
>> This does not compile for me:
>>
>>   CC    aarch64-softmmu/hw/arm/virt.o
>> hw/arm/virt.c: In function ‘create_fdt’:
>> hw/arm/virt.c:186:44: error: ‘KVM_CAP_ARM_PSCI_0_2’ undeclared (first
>> use in this function)
>>          if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
>>                                             ^
>
> Note that I am building for system emulation, not KVM which may
> explain the difference (I assume it actually builds for you).

Yes, you can't use the kernel header constants here, you need
to use the QEMU_ variants that kvm-consts.h provides.

thanks
-- PMM
Pranavkumar Sawargaonkar May 6, 2014, 5:24 a.m. UTC | #4 
Hi Rob, Peter,

On 5 May 2014 20:13, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 May 2014 15:09, Rob Herring <robherring2@gmail.com> wrote:
>> On Mon, May 5, 2014 at 9:06 AM, Rob Herring <robherring2@gmail.com> wrote:
>>> This does not compile for me:
>>>
>>>   CC    aarch64-softmmu/hw/arm/virt.o
>>> hw/arm/virt.c: In function ‘create_fdt’:
>>> hw/arm/virt.c:186:44: error: ‘KVM_CAP_ARM_PSCI_0_2’ undeclared (first
>>> use in this function)
>>>          if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
>>>                                             ^
>>
>> Note that I am building for system emulation, not KVM which may
>> explain the difference (I assume it actually builds for you).
>
Yes actually it builds for me since i am building KVM.

> Yes, you can't use the kernel header constants here, you need
> to use the QEMU_ variants that kvm-consts.h provides.

Sure I will do use QEMU_ variants in next patch so that it builds for
non KVM case also.

>
> thanks
> -- PMM

Thanks,
Pranav
Rob Herring May 6, 2014, 2:58 p.m. UTC | #5 
On Tue, May 6, 2014 at 12:24 AM, Pranavkumar Sawargaonkar
<pranavkumar@linaro.org> wrote:
> Hi Rob, Peter,
>
> On 5 May 2014 20:13, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 5 May 2014 15:09, Rob Herring <robherring2@gmail.com> wrote:
>>> On Mon, May 5, 2014 at 9:06 AM, Rob Herring <robherring2@gmail.com> wrote:
>>>> This does not compile for me:
>>>>
>>>>   CC    aarch64-softmmu/hw/arm/virt.o
>>>> hw/arm/virt.c: In function ‘create_fdt’:
>>>> hw/arm/virt.c:186:44: error: ‘KVM_CAP_ARM_PSCI_0_2’ undeclared (first
>>>> use in this function)
>>>>          if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
>>>>                                             ^
>>>
>>> Note that I am building for system emulation, not KVM which may
>>> explain the difference (I assume it actually builds for you).
>>
> Yes actually it builds for me since i am building KVM.
>
>> Yes, you can't use the kernel header constants here, you need
>> to use the QEMU_ variants that kvm-consts.h provides.
>
> Sure I will do use QEMU_ variants in next patch so that it builds for
> non KVM case also.

We created the psci.h header to be a common header to define the ABI.
It has no linux dependency other than that is the master copy ATM. Can
we just copy the header to a non-Linux location rather than creating
duplicate QEMU_ prefixed defines?

Rob
Peter Maydell May 6, 2014, 2:59 p.m. UTC | #6 
On 6 May 2014 15:58, Rob Herring <robherring2@gmail.com> wrote:
> We created the psci.h header to be a common header to define the ABI.
> It has no linux dependency other than that is the master copy ATM. Can
> we just copy the header to a non-Linux location rather than creating
> duplicate QEMU_ prefixed defines?

Can we guarantee that the kernel headers won't fail
to compile if we include both QEMU's copy of the psci.h header
and the one the kernel has?

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2bbc931..e4ae8ba 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -185,13 +185,17 @@  static void create_fdt(VirtBoardInfo *vbi)
     /* No PSCI for TCG yet */
     if (kvm_enabled()) {
         qemu_fdt_add_subnode(fdt, "/psci");
-        qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
-        qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc");
-        qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend",
+        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_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");
     }
 }