Re: [Qemu-devel] [RFC PATCH 6/7] hw/arm/virt: Use PSCI v0.2 function IDs when kernel supports its
On Fri, Mar 14, 2014 at 05:10:34PM +, Rob Herring wrote: > On Fri, Mar 14, 2014 at 5:23 AM, Mark Rutland wrote: > > On Fri, Mar 14, 2014 at 06:53:53AM +, Pranavkumar Sawargaonkar wrote: > >> Hi Christoffer, > >> > >> On 14 March 2014 09:19, Christoffer Dall > >> wrote: > >> > On Thu, Feb 27, 2014 at 12:21:07PM +0530, Pranavkumar Sawargaonkar 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 function IDs via generated DTB. > >> >> > >> >> This patch updates generated DTB to have PSCI v0.2 function IDs when > >> >> we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64. > >> >> > >> >> Signed-off-by: Pranavkumar Sawargaonkar > >> >> Signed-off-by: Anup Patel > >> >> --- > >> >> hw/arm/virt.c | 25 - > >> >> 1 file changed, 20 insertions(+), 5 deletions(-) > >> >> > >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c > >> >> index 517f2fe..a818a80 100644 > >> >> --- a/hw/arm/virt.c > >> >> +++ b/hw/arm/virt.c > >> >> @@ -187,11 +187,26 @@ static void create_fdt(VirtBoardInfo *vbi) > >> >> qemu_fdt_add_subnode(fdt, "/psci"); > >> >> qemu_fdt_setprop_string(fdt, "/psci", "compatible", > >> >> "arm,psci"); > >> > > >> > was there a decision on the format of the psci 0.2 bindings? > >> > > >> > I seem to recall that we should add arm,psci-0.2 or something like that. > >> > >> Yes there was a discussion related to that by Mark Rutland and Rob Herring > >> : > >> http://www.spinics.net/lists/arm-kernel/msg298509.html > >> > >> But there is no dt binding added related to psci 0.2 in kernel (I am > >> not sure about final conclusion of the dt binding to be added). If the > >> dt binding gets finalized I will definitely revise this patch. > >> For now I have added old binding only to test the rfc patch (may now > >> be the right way to do but do not have any option also). > > > > I believe Rob and I were happy with PSCI 0.2 IDs being implicit, though > > for compatibility with existing kernels the IDs in the "arm,psci" > > binding might also be listed. > > > > The only issue was Calxeda highbank/midway systems using pre-release > > PSCI 0.2 IDs for functions not in the "arm,psci" binding. For those we > > could allocate a compatilbe string like "calxeda,highbank-psci-0.2" and > > allow them to be implicit, or just leave them in their current > > (unsupported) state. > > > > Rob, thoughts? > > We need to add "arm,psci-0.2" compatible since "arm,psci" defines all > the function IDs as required and with 0.2 they are optional. > > For highbank, since firmware and DTB is essentially frozen now, a > compatible change is not going to happen. It probably means just > leaving things as is. Ok. > > For reference, this is what highbank/midway's binding looks like: > > psci { > compatible = "arm,psci"; > method = "smc"; > cpu_suspend = <0x8402>; > cpu_off = <0x8404>; > cpu_on = <0x8406>; > system_off = <0x84000100>; > system_reset= <0x84000101>; > }; > > I suppose I could just ignore the binding and make highbank reset/off > functions do custom some PSCI calls. This would mean exposing > invoke_psci_fn or creating some custom call wrapper. Is there any good way to tell if a machine is highbank or midway (e.g. the top-level compatible string)? We could check for that and if so read the additional values from the dt. That way highbank and midway get supported but new machines would still have to follow the IDs from the final release. This doesn't necessarily have to be documented as an official binding, it's essentially a workaround for a quirk (though one with a good reason for existing). Does that sound reasonable? Cheers, Mark.
Re: [Qemu-devel] [RFC PATCH 6/7] hw/arm/virt: Use PSCI v0.2 function IDs when kernel supports its
On Fri, Mar 14, 2014 at 5:23 AM, Mark Rutland wrote: > On Fri, Mar 14, 2014 at 06:53:53AM +, Pranavkumar Sawargaonkar wrote: >> Hi Christoffer, >> >> On 14 March 2014 09:19, Christoffer Dall wrote: >> > On Thu, Feb 27, 2014 at 12:21:07PM +0530, Pranavkumar Sawargaonkar 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 function IDs via generated DTB. >> >> >> >> This patch updates generated DTB to have PSCI v0.2 function IDs when >> >> we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64. >> >> >> >> Signed-off-by: Pranavkumar Sawargaonkar >> >> Signed-off-by: Anup Patel >> >> --- >> >> hw/arm/virt.c | 25 - >> >> 1 file changed, 20 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> >> index 517f2fe..a818a80 100644 >> >> --- a/hw/arm/virt.c >> >> +++ b/hw/arm/virt.c >> >> @@ -187,11 +187,26 @@ static void create_fdt(VirtBoardInfo *vbi) >> >> qemu_fdt_add_subnode(fdt, "/psci"); >> >> qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); >> > >> > was there a decision on the format of the psci 0.2 bindings? >> > >> > I seem to recall that we should add arm,psci-0.2 or something like that. >> >> Yes there was a discussion related to that by Mark Rutland and Rob Herring : >> http://www.spinics.net/lists/arm-kernel/msg298509.html >> >> But there is no dt binding added related to psci 0.2 in kernel (I am >> not sure about final conclusion of the dt binding to be added). If the >> dt binding gets finalized I will definitely revise this patch. >> For now I have added old binding only to test the rfc patch (may now >> be the right way to do but do not have any option also). > > I believe Rob and I were happy with PSCI 0.2 IDs being implicit, though > for compatibility with existing kernels the IDs in the "arm,psci" > binding might also be listed. > > The only issue was Calxeda highbank/midway systems using pre-release > PSCI 0.2 IDs for functions not in the "arm,psci" binding. For those we > could allocate a compatilbe string like "calxeda,highbank-psci-0.2" and > allow them to be implicit, or just leave them in their current > (unsupported) state. > > Rob, thoughts? We need to add "arm,psci-0.2" compatible since "arm,psci" defines all the function IDs as required and with 0.2 they are optional. For highbank, since firmware and DTB is essentially frozen now, a compatible change is not going to happen. It probably means just leaving things as is. For reference, this is what highbank/midway's binding looks like: psci { compatible = "arm,psci"; method = "smc"; cpu_suspend = <0x8402>; cpu_off = <0x8404>; cpu_on = <0x8406>; system_off = <0x84000100>; system_reset= <0x84000101>; }; I suppose I could just ignore the binding and make highbank reset/off functions do custom some PSCI calls. This would mean exposing invoke_psci_fn or creating some custom call wrapper. Rob
Re: [Qemu-devel] [RFC PATCH 6/7] hw/arm/virt: Use PSCI v0.2 function IDs when kernel supports its
On Fri, Mar 14, 2014 at 06:53:53AM +, Pranavkumar Sawargaonkar wrote: > Hi Christoffer, > > On 14 March 2014 09:19, Christoffer Dall wrote: > > On Thu, Feb 27, 2014 at 12:21:07PM +0530, Pranavkumar Sawargaonkar 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 function IDs via generated DTB. > >> > >> This patch updates generated DTB to have PSCI v0.2 function IDs when > >> we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64. > >> > >> Signed-off-by: Pranavkumar Sawargaonkar > >> Signed-off-by: Anup Patel > >> --- > >> hw/arm/virt.c | 25 - > >> 1 file changed, 20 insertions(+), 5 deletions(-) > >> > >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c > >> index 517f2fe..a818a80 100644 > >> --- a/hw/arm/virt.c > >> +++ b/hw/arm/virt.c > >> @@ -187,11 +187,26 @@ static void create_fdt(VirtBoardInfo *vbi) > >> qemu_fdt_add_subnode(fdt, "/psci"); > >> qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); > > > > was there a decision on the format of the psci 0.2 bindings? > > > > I seem to recall that we should add arm,psci-0.2 or something like that. > > Yes there was a discussion related to that by Mark Rutland and Rob Herring : > http://www.spinics.net/lists/arm-kernel/msg298509.html > > But there is no dt binding added related to psci 0.2 in kernel (I am > not sure about final conclusion of the dt binding to be added). If the > dt binding gets finalized I will definitely revise this patch. > For now I have added old binding only to test the rfc patch (may now > be the right way to do but do not have any option also). I believe Rob and I were happy with PSCI 0.2 IDs being implicit, though for compatibility with existing kernels the IDs in the "arm,psci" binding might also be listed. The only issue was Calxeda highbank/midway systems using pre-release PSCI 0.2 IDs for functions not in the "arm,psci" binding. For those we could allocate a compatilbe string like "calxeda,highbank-psci-0.2" and allow them to be implicit, or just leave them in their current (unsupported) state. Rob, thoughts? Thanks, Mark.
Re: [Qemu-devel] [RFC PATCH 6/7] hw/arm/virt: Use PSCI v0.2 function IDs when kernel supports its
Hi Christoffer, On 14 March 2014 09:19, Christoffer Dall wrote: > On Thu, Feb 27, 2014 at 12:21:07PM +0530, Pranavkumar Sawargaonkar 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 function IDs via generated DTB. >> >> This patch updates generated DTB to have PSCI v0.2 function IDs when >> we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64. >> >> Signed-off-by: Pranavkumar Sawargaonkar >> Signed-off-by: Anup Patel >> --- >> hw/arm/virt.c | 25 - >> 1 file changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 517f2fe..a818a80 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -187,11 +187,26 @@ static void create_fdt(VirtBoardInfo *vbi) >> qemu_fdt_add_subnode(fdt, "/psci"); >> qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); > > was there a decision on the format of the psci 0.2 bindings? > > I seem to recall that we should add arm,psci-0.2 or something like that. Yes there was a discussion related to that by Mark Rutland and Rob Herring : http://www.spinics.net/lists/arm-kernel/msg298509.html But there is no dt binding added related to psci 0.2 in kernel (I am not sure about final conclusion of the dt binding to be added). If the dt binding gets finalized I will definitely revise this patch. For now I have added old binding only to test the rfc patch (may now be the right way to do but do not have any option also). > > -Christoffer Thanks, Pranav
Re: [Qemu-devel] [RFC PATCH 6/7] hw/arm/virt: Use PSCI v0.2 function IDs when kernel supports its
On 27 February 2014 12:21, Pranavkumar Sawargaonkar 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 function IDs via generated DTB. > > This patch updates generated DTB to have PSCI v0.2 function IDs when > we have in-kernel emulation PSCI v0.2 for KVM ARM/ARM64. > > Signed-off-by: Pranavkumar Sawargaonkar > Signed-off-by: Anup Patel > --- > hw/arm/virt.c | 25 - > 1 file changed, 20 insertions(+), 5 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 517f2fe..a818a80 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -187,11 +187,26 @@ static void create_fdt(VirtBoardInfo *vbi) > 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", > - 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); > +if (kvm_check_extension(kvm_state, KVM_CAP_ARM_PSCI_0_2)) { > +qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", > + PSCI_0_2_FN_CPU_SUSPEND); > +qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", > + PSCI_0_2_FN_CPU_OFF); > +qemu_fdt_setprop_cell(fdt, "/psci", "cpu_on", > + PSCI_0_2_FN_CPU_ON); > +qemu_fdt_setprop_cell(fdt, "/psci", "migrate", > + PSCI_0_2_FN_MIGRATE); > +qemu_fdt_setprop_cell(fdt, "/psci", "system_off", > + PSCI_0_2_FN_SYSTEM_OFF); > +qemu_fdt_setprop_cell(fdt, "/psci", "system_reset", > + PSCI_0_2_FN_SYSTEM_RESET); > +} else { > +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); > +} > } > } > > -- > 1.7.9.5 > Sorry for mentioning wrong qemu devel mail id in my previous mail. Correcting correct qemu-devel id. Thanks, Pranav