Received-SPF: pass (google.com: domain of linux-kernel+bounces-46966-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240127004321.1902477-1-davidai@google.com> <20240127004321.1902477-2-davidai@google.com>
 <20240131170608.GA1441369-robh@kernel.org>
In-Reply-To: <20240131170608.GA1441369-robh@kernel.org>
From: Saravana Kannan <saravanak@google.com>
Date: Wed, 31 Jan 2024 10:23:03 -0800
Message-ID: <CAGETcx8S0oS67oMZsPKk6_MGAtygoHEf_LN1gbcNDEBqRJ4PPg@mail.gmail.com>
Subject: Re: [PATCH v5 1/2] dt-bindings: cpufreq: add virtual cpufreq device
To: Rob Herring <robh@kernel.org>
Cc: David Dai <davidai@google.com>, "Rafael J. Wysocki" <rafael@kernel.org>, 
	Viresh Kumar <viresh.kumar@linaro.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Sudeep Holla <sudeep.holla@arm.com>, Quentin Perret <qperret@google.com>, 
	Masami Hiramatsu <mhiramat@google.com>, Will Deacon <will@kernel.org>, 
	Peter Zijlstra <peterz@infradead.org>, Vincent Guittot <vincent.guittot@linaro.org>, 
	Marc Zyngier <maz@kernel.org>, Oliver Upton <oliver.upton@linux.dev>, 
	Dietmar Eggemann <dietmar.eggemann@arm.com>, Pavan Kondeti <quic_pkondeti@quicinc.com>, 
	Gupta Pankaj <pankaj.gupta@amd.com>, Mel Gorman <mgorman@suse.de>, kernel-team@android.com, 
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 31, 2024 at 9:06=E2=80=AFAM Rob Herring <robh@kernel.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@google.com>
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > Signed-off-by: David Dai <davidai@google.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 eac=
h 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 proporti=
onal to
> > +      total number of frequency domains. This device also needs the CP=
Us 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. T=
he rest
> > +      of the frequencies of all the vCPUs should be normalized based o=
n their
> > +      performance relative to that 1024 KHz OPP. This makes it much ea=
sier to
> > +      migrate the VM across systems which might have different physica=
l CPU
> > +      OPPs.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    // This example shows a two CPU configuration with a frequency dom=
ain
> > +    // for each CPU showing normalized performance points.
> > +    cpus {
> > +      #address-cells =3D <1>;
> > +      #size-cells =3D <0>;
> > +
> > +      cpu@0 {
> > +        compatible =3D "arm,armv8";
> > +        device_type =3D "cpu";
> > +        reg =3D <0x0>;
> > +        operating-points-v2 =3D <&opp_table0>;
> > +      };
> > +
> > +      cpu@1 {
> > +        compatible =3D "arm,armv8";
> > +        device_type =3D "cpu";
> > +        reg =3D <0x0>;
> > +        operating-points-v2 =3D <&opp_table1>;
> > +      };
> > +    };
> > +
> > +    opp_table0: opp-table-0 {
> > +      compatible =3D "operating-points-v2";
> > +
> > +      opp64000 { opp-hz =3D /bits/ 64 <64000>; };
>
> opp-64000 is the preferred form.
>
> > +      opp128000 { opp-hz =3D /bits/ 64 <128000>; };
> > +      opp192000 { opp-hz =3D /bits/ 64 <192000>; };
> > +      opp256000 { opp-hz =3D /bits/ 64 <256000>; };
> > +      opp320000 { opp-hz =3D /bits/ 64 <320000>; };
> > +      opp384000 { opp-hz =3D /bits/ 64 <384000>; };
> > +      opp425000 { opp-hz =3D /bits/ 64 <425000>; };
> > +    };
> > +
> > +    opp_table1: opp-table-1 {
> > +      compatible =3D "operating-points-v2";
> > +
> > +      opp64000 { opp-hz =3D /bits/ 64 <64000>; };
> > +      opp128000 { opp-hz =3D /bits/ 64 <128000>; };
> > +      opp192000 { opp-hz =3D /bits/ 64 <192000>; };
> > +      opp256000 { opp-hz =3D /bits/ 64 <256000>; };
> > +      opp320000 { opp-hz =3D /bits/ 64 <320000>; };
> > +      opp384000 { opp-hz =3D /bits/ 64 <384000>; };
> > +      opp448000 { opp-hz =3D /bits/ 64 <448000>; };
> > +      opp512000 { opp-hz =3D /bits/ 64 <512000>; };
> > +      opp576000 { opp-hz =3D /bits/ 64 <576000>; };
> > +      opp640000 { opp-hz =3D /bits/ 64 <640000>; };
> > +      opp704000 { opp-hz =3D /bits/ 64 <704000>; };
> > +      opp768000 { opp-hz =3D /bits/ 64 <768000>; };
> > +      opp832000 { opp-hz =3D /bits/ 64 <832000>; };
> > +      opp896000 { opp-hz =3D /bits/ 64 <896000>; };
> > +      opp960000 { opp-hz =3D /bits/ 64 <960000>; };
> > +      opp1024000 { opp-hz =3D /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