[PATCH 1/7] dt: update PSCI binding documentation for v0.2

[Mark Rutland]

On Tue, Jul 30, 2013 at 03:33:34PM +0100, Stefano Stabellini wrote:
> On Tue, 30 Jul 2013, Mark Rutland wrote:
> > On Tue, Jul 30, 2013 at 01:56:50PM +0100, Mark Rutland wrote:
> > > On Tue, Jul 30, 2013 at 01:42:49PM +0100, Rob Herring wrote:
> > > > On 07/30/2013 04:49 AM, Mark Rutland wrote:
> > > > > On Mon, Jul 29, 2013 at 09:18:43PM +0100, Rob Herring wrote:
> > > > >> On 07/29/2013 05:13 AM, Mark Rutland wrote:
> > > > >>> Hi Rob,
> > > > >>>
> > > > >>> On Sun, Jul 28, 2013 at 10:56:32PM +0100, Rob Herring wrote:
> > > > >>>> From: Rob Herring <rob.herring at calxeda.com>
> > > > 
> > > > [snip]
> > > > 
> > > > >>> One of the things changed in PSCI 0.2 was the SMC calling convention,
> > > > >>> though this isn't clear in the PSCI document. The function IDs for 32bit
> > > > >>> and 64bit callers may differ, and we need to support describing an
> > > > >>> arbitrary configuration of the two (same ID for both, different across
> > > > >>> 32-bit/64-bit, only supported for 64-bit, only supported for 32-bit).
> > > > >>>
> > > > >>> I'd like to ensure the binding can deal with that from the start. We
> > > > >>> could do this by having -32 and -64 variants of each function id (e.g.
> > > > >>> cpu_off-64) , if the IDs actually differ, and use the regular combined
> > > > >>> ID if they don't.
> > > > >>
> > > > >> Uggg. I guess I should have read the SMC calling convention doc... I was
> > > > >> simply documenting what is already in the PSCI doc, but obviously that
> > > > >> is not fully flushed out.
> > > > >>
> > > > >> How about something like this (for the complicated case of both 32 and
> > > > >> 64 bit):
> > > > >>
> > > > >> 	method		= "smc", "smc64";
> > > > >> 	psci_version	= <0x84000000 0xc4000000>;
> > > > >> 	cpu_suspend	= <0x84000001 0xc4000001>;
> > > > >> 	cpu_off		= <0x84000002 0xc4000002>;
> > > > >> 	cpu_on		= <0x84000003 0xc4000003>;
> > > > >>
> > > > >> "smc" is a synonym for smc32 for compatibility. The number and order of
> > > > >> methods determines the number and order of function IDs.
> > > > > 
> > > > > While this may be compatible with the arm implementation, it won't be
> > > > > compatible with the arm64 implementation, which assumes smc64 by
> > > > > default.
> > > > > 
> > > > > As far as I am aware, the implementations currently in use (KVM and Xen)
> > > > > use the same ID for both, so I think "smc" should cover an ID valid for
> > > > > a native register width calling convention, and "smc64" and "smc32"
> > > > > describing values only valid for 64-bit wide and 32-bit wide calling
> > > > > conventions respectively.
> > > > 
> > > > The problem is that does not work for a 32-bit kernel on 64-bit h/w as
> > > > native from the dts perspective is smc64. Just like the cpu bindings,
> > > > the binding cannot change based on 32 or 64 bit OS. I don't think we
> > > > really have to deal with that here. We can simply say "smc" is only for
> > > > "arm,psci" and deprecated for "arm,psci-0.2".
> > > 
> > > Agreed. I'd be happy with only having "smc32" and "smc64" for
> > > "arm,psci-0.2".
> 
> I would be happy with that too (Xen only uses HVC as "method").
> 
> 
> > Actually, from some quick discussion with Marc and Dave, I think we can
> > handle this better, in a way that leaves this backwards compatible.
> > 
> > Rather than relying on the method string to tell us the calling
> > convention, I think we should rely on the function ID, as I proposed
> > earlier. The existing function ids provided in the "arm,psci" binding
> > are implicitly relying on the PSCI implementation to detect the register
> > width and act accordingly. This is trivially true on 32bit hardware, KVM
> > (where the same ID is used for 32-bit and 64-bit guests), and while I'm
> > not entirely sure about Xen I believe it's true there. We can make this
> > explicit as we extend the binding.
> > 
> > Having a -64 and -32 variant of each ID (while not pretty) allows us to
> > add additional IDs for functions that might only have a 32-bit or 64-bit
> > interface implemented, in addition to functions with common IDs:
> > 
> > psci {
> > 	compatible = "$VENDOR,psci-0.2", "arm,psci-0.2", "arm,psci";
> > 	cpu_off = <12345678>;
> > 	cpu_on = <01234567>;
> > 	system_reset-32 = <02222222>;
> > 	system_reset-64 = <12222222>;
> > 	affinity_info-64 = <15555555>;
> > };
> > 
> > This means that hypervisors could update their PSCI implementation while
> > keeping their DTS compatible with existing kernels.
> 
> Are you proposing of getting rid of "method" completely and therefore
> have 4 possible function IDs for each function:
> 
> system_reset-32-HVC
> system_reset-64-SMC
> system_reset-32-HVC
> system_reset-64-SMC
> 
> or are you proposing of choosing just the register width via function
> IDs?

Just the register width via the function ID's property name.

> 
> Honestly I think it would be cleaner to introduce a new field called
> "width" can that be 32 or 64 and represent the register width, rather
> than having an explosion of function IDs.
> 

Possibly. I'm worried people may create a mixture of shared, 64-bit
only, and 32-bit only IDs.

Thanks,
Mark.