| 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 |
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
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
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
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.
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.
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
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
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
On Thu, Feb 15, 2024 at 3:26 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > 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. > Hi Sudeep, We are thinking of a discoverable interface like this, where the performance info and performance domain mappings are discoverable through the device registers. This should make it more portable across firmwares. Would this address your concerns? Also, you asked to document this. Where exactly would you want to document this? AFAIK the DT bindings documentation is not supposed to include this level of detail. Would a comment in the driver be sufficient? CPU0..CPUn +-------------+-------------------------------+--------+-------+ | Register | Description | Offset | Len | +-------------+-------------------------------+--------+-------+ | cur_perf | read this register to get | 0x0 | 0x4 | | | the current perf (integer val | | | | | representing perf relative to | | | | | max performance) | | | | | that vCPU is running at | | | +-------------+-------------------------------+--------+-------+ | set_perf | write to this register to set | 0x4 | 0x4 | | | perf value of the vCPU | | | +-------------+-------------------------------+--------+-------+ | perftbl_len | number of entries in perf | 0x8 | 0x4 | | | table. A single entry in the | | | | | perf table denotes no table | | | | | and the entry contains | | | | | the maximum perf value | | | | | that this vCPU supports. | | | | | The guest can request any | | | | | value between 1 and max perf. | | | +---------------------------------------------+--------+-------+ | perftbl_sel | write to this register to | 0xc | 0x4 | | | select perf table entry to | | | | | read from | | | +---------------------------------------------+--------+-------+ | perftbl_rd | read this register to get | 0x10 | 0x4 | | | perf value of the selected | | | | | entry based on perftbl_sel | | | +---------------------------------------------+--------+-------+ | perf_domain | performance domain number | 0x14 | 0x4 | | | that this vCPU belongs to. | | | | | vCPUs sharing the same perf | | | | | domain number are part of the | | | | | same performance domain. | | | +-------------+-------------------------------+--------+-------+ Thanks, David > -- > Regards, > Sudeep
On Thu, May 02, 2024 at 01:17:57PM -0700, David Dai wrote: > On Thu, Feb 15, 2024 at 3:26 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > 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. > > > > Hi Sudeep, > > We are thinking of a discoverable interface like this, where the > performance info and performance domain mappings are discoverable > through the device registers. This should make it more portable across > firmwares. Would this address your concerns? Yes. > Also, you asked to document this. > Where exactly would you want to document this? IMO it could go under Documentation/firmware-guide ? Unless someone has any other suggestions. > AFAIK the DT bindings documentation is not supposed to include this level of > detail. Would a comment in the driver be sufficient? Agree, DT bindings is not the right place. May be even comment in the driver would be sufficient. Overall it looks good and on the right path IMO. > > CPU0..CPUn > +-------------+-------------------------------+--------+-------+ > | Register | Description | Offset | Len | > +-------------+-------------------------------+--------+-------+ > | cur_perf | read this register to get | 0x0 | 0x4 | > | | the current perf (integer val | | | > | | representing perf relative to | | | > | | max performance) | | | > | | that vCPU is running at | | | > +-------------+-------------------------------+--------+-------+ > | set_perf | write to this register to set | 0x4 | 0x4 | > | | perf value of the vCPU | | | > +-------------+-------------------------------+--------+-------+ > | perftbl_len | number of entries in perf | 0x8 | 0x4 | > | | table. A single entry in the | | | > | | perf table denotes no table | | | > | | and the entry contains | | | > | | the maximum perf value | | | > | | that this vCPU supports. | | | > | | The guest can request any | | | > | | value between 1 and max perf. | | | Does this have to be per cpu ? It can be simplified by keeping just cur_perf, set_perf and perf_domain in per-cpu entries and this per domain entries separate. But I am not against per cpu entries as well. Also why do you need the table if the guest can request any value from 1 to max perf ? The table will have discrete OPPs ? If so, how to they map to the perf range [1 - maxperf] ? > +---------------------------------------------+--------+-------+ > | perftbl_sel | write to this register to | 0xc | 0x4 | > | | select perf table entry to | | | > | | read from | | | > +---------------------------------------------+--------+-------+ > | perftbl_rd | read this register to get | 0x10 | 0x4 | > | | perf value of the selected | | | > | | entry based on perftbl_sel | | | > +---------------------------------------------+--------+-------+ > | perf_domain | performance domain number | 0x14 | 0x4 | > | | that this vCPU belongs to. | | | > | | vCPUs sharing the same perf | | | > | | domain number are part of the | | | > | | same performance domain. | | | > +-------------+-------------------------------+--------+-------+ The above are couple of high level questions I have ATM. -- Regards, Sudeep
On Tue, May 7, 2024 at 3:21 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Thu, May 02, 2024 at 01:17:57PM -0700, David Dai wrote: > > On Thu, Feb 15, 2024 at 3:26 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > 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. > > > > > > > Hi Sudeep, > > > > We are thinking of a discoverable interface like this, where the > > performance info and performance domain mappings are discoverable > > through the device registers. This should make it more portable across > > firmwares. Would this address your concerns? > > Yes. > > > Also, you asked to document this. > > Where exactly would you want to document this? > > IMO it could go under Documentation/firmware-guide ? Unless someone > has any other suggestions. > > > AFAIK the DT bindings documentation is not supposed to include this level of > > detail. Would a comment in the driver be sufficient? > > Agree, DT bindings is not the right place. May be even comment in the > driver would be sufficient. Alright, I’ll make this into a comment in the driver itself. > > Overall it looks good and on the right path IMO. > Okay, I’ll submit V6 patches and continue from there. > > > > CPU0..CPUn > > +-------------+-------------------------------+--------+-------+ > > | Register | Description | Offset | Len | > > +-------------+-------------------------------+--------+-------+ > > | cur_perf | read this register to get | 0x0 | 0x4 | > > | | the current perf (integer val | | | > > | | representing perf relative to | | | > > | | max performance) | | | > > | | that vCPU is running at | | | > > +-------------+-------------------------------+--------+-------+ > > | set_perf | write to this register to set | 0x4 | 0x4 | > > | | perf value of the vCPU | | | > > +-------------+-------------------------------+--------+-------+ > > | perftbl_len | number of entries in perf | 0x8 | 0x4 | > > | | table. A single entry in the | | | > > | | perf table denotes no table | | | > > | | and the entry contains | | | > > | | the maximum perf value | | | > > | | that this vCPU supports. | | | > > | | The guest can request any | | | > > | | value between 1 and max perf. | | | > > Does this have to be per cpu ? It can be simplified by keeping > just cur_perf, set_perf and perf_domain in per-cpu entries and this > per domain entries separate. But I am not against per cpu entries > as well. I think separating out the perf domain entries may make the device emulation and the driver slightly more complicated. Emulating the perf domain regions per CPU is a simpler layout if we need to install eBPF programs to handle the backend per vCPU. Each vCPU looking up its own frequency information in its own MMIO region is a bit easier too when initializing the driver. Also each vCPU will be in its own perf domain for the majority of the use cases, so it won’t make much of a difference most of the time. > > Also why do you need the table if the guest can request any value from > 1 to max perf ? The table will have discrete OPPs ? If so, how to they > map to the perf range [1 - maxperf] ? Let me clarify this in the comment, the perf range [1 - maxperf] is only applicable in the case where the frequency table is not supported. The cpufreq driver will still vote for discrete levels if tables are used. The VMM(Virtual Machine Manager) may choose to use tables depending on the use case and the driver will support both cases. Thanks, David > > > +---------------------------------------------+--------+-------+ > > | perftbl_sel | write to this register to | 0xc | 0x4 | > > | | select perf table entry to | | | > > | | read from | | | > > +---------------------------------------------+--------+-------+ > > | perftbl_rd | read this register to get | 0x10 | 0x4 | > > | | perf value of the selected | | | > > | | entry based on perftbl_sel | | | > > +---------------------------------------------+--------+-------+ > > | perf_domain | performance domain number | 0x14 | 0x4 | > > | | that this vCPU belongs to. | | | > > | | vCPUs sharing the same perf | | | > > | | domain number are part of the | | | > > | | same performance domain. | | | > > +-------------+-------------------------------+--------+-------+ > > The above are couple of high level questions I have ATM. > > -- > Regards, > Sudeep
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>; + }; + };