Message-ID: <CAGETcx8S0oS67oMZsPKk6_MGAtygoHEf_LN1gbcNDEBqRJ4PPg@mail.gmail.com>
Date: Wed, 31 Jan 2024 10:23:03 -0800
From: Saravana Kannan <saravanak@...gle.com>
To: Rob Herring <robh@...nel.org>
Cc: David Dai <davidai@...gle.com>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Viresh Kumar <viresh.kumar@...aro.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	Sudeep Holla <sudeep.holla@....com>, Quentin Perret <qperret@...gle.com>, 
	Masami Hiramatsu <mhiramat@...gle.com>, Will Deacon <will@...nel.org>, 
	Peter Zijlstra <peterz@...radead.org>, Vincent Guittot <vincent.guittot@...aro.org>, 
	Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>, 
	Dietmar Eggemann <dietmar.eggemann@....com>, Pavan Kondeti <quic_pkondeti@...cinc.com>, 
	Gupta Pankaj <pankaj.gupta@....com>, Mel Gorman <mgorman@...e.de>, kernel-team@...roid.com, 
	linux-pm@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/2] dt-bindings: cpufreq: add virtual cpufreq device

On Wed, Jan 31, 2024 at 9:06 AM Rob Herring <robh@...nel.org> wrote:
>
> On Fri, Jan 26, 2024 at 04:43:15PM -0800, David Dai wrote:
> > Adding bindings to represent a virtual cpufreq device.
> >
> > Virtual machines may expose MMIO regions for a virtual cpufreq device
> > for guests to read frequency information or to request frequency
> > selection. The virtual cpufreq device has an individual controller for
> > each frequency domain. Performance points for a given domain can be
> > normalized across all domains for ease of allowing for virtual machines
> > to migrate between hosts.
> >
> > Co-developed-by: Saravana Kannan <saravanak@...gle.com>
> > Signed-off-by: Saravana Kannan <saravanak@...gle.com>
> > Signed-off-by: David Dai <davidai@...gle.com>
> > ---
> >  .../cpufreq/qemu,cpufreq-virtual.yaml         | 110 ++++++++++++++++++
>
> > +    const: qemu,virtual-cpufreq
>
> Well, the filename almost matches the compatible.
>
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description:
> > +      Address and size of region containing frequency controls for each of the
> > +      frequency domains. Regions for each frequency domain is placed
> > +      contiguously and contain registers for controlling DVFS(Dynamic Frequency
> > +      and Voltage) characteristics. The size of the region is proportional to
> > +      total number of frequency domains. This device also needs the CPUs to
> > +      list their OPPs using operating-points-v2 tables. The OPP tables for the
> > +      CPUs should use normalized "frequency" values where the OPP with the
> > +      highest performance among all the vCPUs is listed as 1024 KHz. The rest
> > +      of the frequencies of all the vCPUs should be normalized based on their
> > +      performance relative to that 1024 KHz OPP. This makes it much easier to
> > +      migrate the VM across systems which might have different physical CPU
> > +      OPPs.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    // This example shows a two CPU configuration with a frequency domain
> > +    // for each CPU showing normalized performance points.
> > +    cpus {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      cpu@0 {
> > +        compatible = "arm,armv8";
> > +        device_type = "cpu";
> > +        reg = <0x0>;
> > +        operating-points-v2 = <&opp_table0>;
> > +      };
> > +
> > +      cpu@1 {
> > +        compatible = "arm,armv8";
> > +        device_type = "cpu";
> > +        reg = <0x0>;
> > +        operating-points-v2 = <&opp_table1>;
> > +      };
> > +    };
> > +
> > +    opp_table0: opp-table-0 {
> > +      compatible = "operating-points-v2";
> > +
> > +      opp64000 { opp-hz = /bits/ 64 <64000>; };
>
> opp-64000 is the preferred form.
>
> > +      opp128000 { opp-hz = /bits/ 64 <128000>; };
> > +      opp192000 { opp-hz = /bits/ 64 <192000>; };
> > +      opp256000 { opp-hz = /bits/ 64 <256000>; };
> > +      opp320000 { opp-hz = /bits/ 64 <320000>; };
> > +      opp384000 { opp-hz = /bits/ 64 <384000>; };
> > +      opp425000 { opp-hz = /bits/ 64 <425000>; };
> > +    };
> > +
> > +    opp_table1: opp-table-1 {
> > +      compatible = "operating-points-v2";
> > +
> > +      opp64000 { opp-hz = /bits/ 64 <64000>; };
> > +      opp128000 { opp-hz = /bits/ 64 <128000>; };
> > +      opp192000 { opp-hz = /bits/ 64 <192000>; };
> > +      opp256000 { opp-hz = /bits/ 64 <256000>; };
> > +      opp320000 { opp-hz = /bits/ 64 <320000>; };
> > +      opp384000 { opp-hz = /bits/ 64 <384000>; };
> > +      opp448000 { opp-hz = /bits/ 64 <448000>; };
> > +      opp512000 { opp-hz = /bits/ 64 <512000>; };
> > +      opp576000 { opp-hz = /bits/ 64 <576000>; };
> > +      opp640000 { opp-hz = /bits/ 64 <640000>; };
> > +      opp704000 { opp-hz = /bits/ 64 <704000>; };
> > +      opp768000 { opp-hz = /bits/ 64 <768000>; };
> > +      opp832000 { opp-hz = /bits/ 64 <832000>; };
> > +      opp896000 { opp-hz = /bits/ 64 <896000>; };
> > +      opp960000 { opp-hz = /bits/ 64 <960000>; };
> > +      opp1024000 { opp-hz = /bits/ 64 <1024000>; };
> > +
> > +    };
>
> I don't recall your prior versions having an OPP table. Maybe it was
> incomplete. You are designing the "h/w" interface. Why don't you make it
> discoverable or implicit (fixed for the h/w)?

We also need the OPP tables to indicate which CPUs are part of the
same cluster, etc. Don't want to invent a new "protocol" and just use
existing DT bindings.

> Do you really need it if the frequency is normalized?

Yeah, we can have little and big CPUs and want to emulate different
performance levels. So while the Fmax on big is 1024, we still want to
be able to say little is 425. So we definitely need frequency tables.

> Also, we have "opp-level" for opaque values that aren't Hz.

Still want to keep it Hz to be compatible with arch_freq_scale and
when virtualized CPU perf counters are available.

-Saravana

Powered by blists - more mailing lists