diff mbox series

[v5,1/2] dt-bindings: cpufreq: add virtual cpufreq device

Message ID 20240127004321.1902477-2-davidai@google.com (mailing list archive)
State Not Applicable, archived
Headers show
Series Improve VM CPUfreq and task placement behavior | expand

Commit Message

David Dai Jan. 27, 2024, 12:43 a.m. UTC
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 ++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/qemu,cpufreq-virtual.yaml

Comments

Rob Herring (Arm) Jan. 31, 2024, 5:06 p.m. UTC | #1
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 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)? Do you really need it if 
the frequency is normalized?

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

Rob
Saravana Kannan Jan. 31, 2024, 6:23 p.m. UTC | #2
On Wed, Jan 31, 2024 at 9:06 AM 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 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
Rob Herring (Arm) Feb. 2, 2024, 3:53 p.m. UTC | #3
On Wed, Jan 31, 2024 at 10:23:03AM -0800, Saravana Kannan wrote:
> On Wed, Jan 31, 2024 at 9:06 AM 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 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.

Topology binding is for that.

What about when x86 and other ACPI systems need to do this too? You 
define a discoverable interface, then it works regardless of firmware. 
KVM, Virtio, VFIO, etc. are all their own protocols.

> > 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.

You need per CPU Fmax, sure. But all the frequencies? I don't follow why 
you don't just have a max available capacity and then request the 
desired capacity. Then the host maps that to an underlying OPP. Why have 
an intermediate set of fake frequencies?

As these are normalized, I guess you are normalizing for capacity as 
well? Or you are using "capacity-dmips-mhz"? 

I'm also lost how this would work when you migrate and the underlying 
CPU changes. The DT is fixed.

> > 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.

Seems like no one would want "opp-level" then. Shrug.

Anyway, if Viresh and Marc are fine with all this, I'll shut up.

Rob
Marc Zyngier Feb. 4, 2024, 10:23 a.m. UTC | #4
On Fri, 02 Feb 2024 15:53:52 +0000,
Rob Herring <robh@kernel.org> wrote:
> 
> On Wed, Jan 31, 2024 at 10:23:03AM -0800, Saravana Kannan wrote:
> > On Wed, Jan 31, 2024 at 9:06 AM 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 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.
> 
> Topology binding is for that.
> 
> What about when x86 and other ACPI systems need to do this too? You 
> define a discoverable interface, then it works regardless of firmware. 
> KVM, Virtio, VFIO, etc. are all their own protocols.

Describing the emulated HW in ACPI would seem appropriate to me. We
are talking about the guest here, so whether this is KVM or not is not
relevant. If this was actually using any soft of data transfer, using
virtio would have been acceptable. But given how simple this is,
piggybacking on virtio is hardly appropriate.

> 
> > > 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.
> 
> You need per CPU Fmax, sure. But all the frequencies? I don't follow why 
> you don't just have a max available capacity and then request the 
> desired capacity. Then the host maps that to an underlying OPP. Why have 
> an intermediate set of fake frequencies?
> 
> As these are normalized, I guess you are normalizing for capacity as 
> well? Or you are using "capacity-dmips-mhz"? 
> 
> I'm also lost how this would work when you migrate and the underlying 
> CPU changes. The DT is fixed.
> 
> > > 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.
> 
> Seems like no one would want "opp-level" then. Shrug.
> 
> Anyway, if Viresh and Marc are fine with all this, I'll shut up.

Well, I've said it before, and I'll say it again: the use of
*frequencies* makes no sense. It is a lie (it doesn't describe any
hardware, physical nor virtual), and doesn't reflect the way the
emulated cpufreq controller behaves either (since it scales everything
back to what the host can potentially do)

The closest abstraction we have to this is the unit-less capacity. And
*that* reflects the way the emulated cpufreq controller works while
avoiding lying to the guest about some arbitrary frequency.

In practice, this changes nothing to either the code or the behaviour.
But it changes the binding.

	M.
Viresh Kumar Feb. 5, 2024, 8:38 a.m. UTC | #5
On 02-02-24, 09:53, Rob Herring wrote:
> On Wed, Jan 31, 2024 at 10:23:03AM -0800, Saravana Kannan wrote:
> > 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.
> 
> Topology binding is for that.

This one, right ?

Documentation/devicetree/bindings/dvfs/performance-domain.yaml

> You need per CPU Fmax, sure. But all the frequencies? I don't follow why 
> you don't just have a max available capacity and then request the 
> desired capacity. Then the host maps that to an underlying OPP. Why have 
> an intermediate set of fake frequencies?

+1

> As these are normalized, I guess you are normalizing for capacity as 
> well? Or you are using "capacity-dmips-mhz"? 
> 
> I'm also lost how this would work when you migrate and the underlying 
> CPU changes. The DT is fixed.
> 
> > > 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.

These are all specific to a driver only, that can be handled easily I guess. I
don't see a value to using Hz for this to be honest.
Rob Herring (Arm) Feb. 5, 2024, 4:39 p.m. UTC | #6
On Mon, Feb 05, 2024 at 02:08:30PM +0530, Viresh Kumar wrote:
> On 02-02-24, 09:53, Rob Herring wrote:
> > On Wed, Jan 31, 2024 at 10:23:03AM -0800, Saravana Kannan wrote:
> > > 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.
> > 
> > Topology binding is for that.
> 
> This one, right ?
> 
> Documentation/devicetree/bindings/dvfs/performance-domain.yaml

No, Documentation/devicetree/bindings/cpu/cpu-topology.txt (or the 
schema version of it in dtschema)

Rob
Sudeep Holla Feb. 15, 2024, 11:26 a.m. UTC | #7
On Fri, Feb 02, 2024 at 09:53:52AM -0600, Rob Herring wrote:
> On Wed, Jan 31, 2024 at 10:23:03AM -0800, Saravana Kannan wrote:
> >
> > 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.
> 
> Topology binding is for that.
> 
> What about when x86 and other ACPI systems need to do this too? You 
> define a discoverable interface, then it works regardless of firmware. 
> KVM, Virtio, VFIO, etc. are all their own protocols.
>

+1 for the above. I have mentioned the same couple of times but I am told
it can be taken up later which I fail to understand. Once we define DT
bindings, it must be supported for long time which doesn't provide any
motivation to such a discoverable interface which works on any virtual
platforms irrespective of the firmware.

--
Regards,
Sudeep
Quentin Perret March 11, 2024, 11:40 a.m. UTC | #8
On Sunday 04 Feb 2024 at 10:23:00 (+0000), Marc Zyngier wrote:
> Well, I've said it before, and I'll say it again: the use of
> *frequencies* makes no sense. It is a lie (it doesn't describe any
> hardware, physical nor virtual), and doesn't reflect the way the
> emulated cpufreq controller behaves either (since it scales everything
> back to what the host can potentially do)
> 
> The closest abstraction we have to this is the unit-less capacity. And
> *that* reflects the way the emulated cpufreq controller works while
> avoiding lying to the guest about some arbitrary frequency.
> 
> In practice, this changes nothing to either the code or the behaviour.
> But it changes the binding.

Apologies all for jumping late into this, but for what it's worth,
regardless of the unit of the binding, Linux will shove that into
cpufreq's 'frequency table' anyway, which as the name suggests is very
much assuming frequencies :/ -- see how struct cpufreq_frequency_table
explicitely requires KHz. The worst part is that this even ends up
being reported to _userspace_ as frequencies in sysfs via cpufreq's
scaling_available_frequencies file, even when they're really not...

In the case of SCMI for example, IIRC the firmware can optionally (and
in practice I think it does for all older implementations of the spec
least) report unit-less operating points to the driver, which will then
happily pretend these are KHz values when reporting that into PM_OPP and
cpufreq -- see how scmi_dvfs_device_opps_add() simply multiplies the
level's 'perf' member by 1000 when populating PM_OPP (which is then
propagated to cpufreq's freq_table'). And a small extract from the SCMI
spec:

    "Certain platforms use IMPLEMENTATION DEFINED indices to identify
     performance levels. Level Indexing Mode is used to describe such
     platform behavior. The level indices associated with performance
     levels are neither guaranteed to be contiguous nor required to be
     on a linear scale."

Not nice, but unfortunately the core cpufreq framework has way too much
historical dependencies on things being frequencies to really change it
now, so we're pretty much stuck with that :(

So, while I do agree with the sentiment that this is a non-ideal place
to be, 'faking' frequencies is how we've addressed this so far in Linux,
so I'm personally not too fussed about David's usage of a freq-based DT
binding in this particular instance. On the plus side that allows to
re-use all of PM_OPP and cpufreq infrastructure as-is, so that's cool.

I guess we could make the argument that Linux's approach to handling
frequencies shouldn't influence this given that the binding should be OS
agnostic, but I can easily see how another OS could still make use of
that binding (and in fact requiring that this other OS can deal with
unitless frequencies is most likely going to be a bigger problem), so
I'd be inclined to think this isn't a major problem either.

Thanks,
Quentin
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/cpufreq/qemu,cpufreq-virtual.yaml b/Documentation/devicetree/bindings/cpufreq/qemu,cpufreq-virtual.yaml
new file mode 100644
index 000000000000..cd617baf75e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/qemu,cpufreq-virtual.yaml
@@ -0,0 +1,110 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cpufreq/qemu,cpufreq-virtual.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtual CPUFreq
+
+maintainers:
+  - David Dai <davidai@google.com>
+  - Saravana Kannan <saravanak@google.com>
+
+description:
+  Virtual CPUFreq is a virtualized driver in guest kernels that sends frequency
+  selection of its vCPUs as a hint to the host through MMIO regions. Each vCPU
+  is associated with a frequency domain which can be shared with other vCPUs.
+  Each frequency domain has its own set of registers for frequency controls.
+
+properties:
+  compatible:
+    const: qemu,virtual-cpufreq
+
+  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>; };
+      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>; };
+
+    };
+
+    soc {
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      cpufreq@1040000 {
+        compatible = "qemu,virtual-cpufreq";
+        reg = <0x1040000 0x10>;
+      };
+    };