Re: [Xen-devel] [PATCH] xen/arm: copy cpu clock-frequency to CPU DT node.

["Jon Fraser" <jfraser@xxxxxxxxxxxx>]

On Tue, 2013-11-05 at 10:04 +0000, Ian Campbell wrote:
> On Tue, 2013-11-05 at 09:56 +0000, Ian Campbell wrote:
> > On Mon, 2013-11-04 at 13:35 -0500, Jon Fraser wrote:
> > > On Mon, 2013-11-04 at 17:12 +0000, Ian Campbell wrote:
> > > > On Fri, 2013-11-01 at 16:58 -0400, Jon Fraser wrote:
> > > > > When creating the CPU DT node, copy the clock-frequency if present.
> > > > > 
> > > ...
> > > > Julien's the expert but I think you need to use dt_property_read_u32
> > > > here, to get the correct endianness conversion (as well as for pure
> > > > forms sake of using the correct API for the job).
> > > > 
> > > I'll fix that.
> > > 
> > > > >              break;
> > > > >          }
> > > > >      }
> > > > > @@ -457,6 +459,12 @@ static int make_cpus_node(const struct domain 
> > > > > *d, void *fdt,
> > > > >          if ( res )
> > > > >              return res;
> > > > >  
> > > > > +        if (clock_frequency) {
> > > > > +            res = fdt_property_cell(fdt, "clock-frequency", *(u32 
> > > > > *)clock_frequency);
> > > > 
> > > > I suppose there ought to be some API for this side of things too, but I
> > > > can't see it right now...
> > > > 
> > > > Note that fdt_property_cell contains a cpu_to_fdt32 so it is converting
> > > > while the read of the property not, so I think the code is broken as is?
> > > 
> > > Yesss, it is broken.  When I checked the property in /proc/device-tree,
> > > I failed to realize it was endian swapped.
> > 
> > I guess nothing much critical is relying on this value. What is it
> > supposed to be used for?
> > linux/Documentation/devicetree/booting-without-of.txt seems to imply it
> > is mostly optional for non-PPC.
> 
> BTW, I'm asking because I'm not sure if exposing the underlying clock
> speed to the guest VCPU is the right thing to do. It can vary across
> pCPUs (e.g. big.LITTLE) and with power control etc.
> 
> Ian.

As far as I can tell, it is intended to be used for load balancing
for the case of differing cpu capabilities.   Looking down the road,
if I was doing Power Management in Dom0, I would want to know
the cpu topology and cpu speeds, but that info is probably available by
other means.

I just wanted to squelch the annoying linux kernel error messages.

/cpus/cpu@0 missing clock-frequency property
/cpus/cpu@1 missing clock-frequency property
/cpus/cpu@2 missing clock-frequency property
/cpus/cpu@3 missing clock-frequency property

Do you foresee, at this point, any problem with giving all the guest
VCPUs the same clock frequency setting?

Jon



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel