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

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(-)

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
>

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

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

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

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

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