Re: [PATCH 6/9] thermal: intel: hfi: Enable Intel Thread Director

From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
To: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Len Brown <len.brown@intel.com>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	Zhao Liu <zhao1.liu@intel.com>,
	Zhuocheng Ding <zhuocheng.ding@intel.com>,
	x86@kernel.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Zhao Liu <zhao1.liu@linux.intel.com>
Subject: Re: [PATCH 6/9] thermal: intel: hfi: Enable Intel Thread Director
Date: Mon, 5 Feb 2024 18:57:00 -0800	[thread overview]
Message-ID: <20240206025700.GE17602@ranerica-svr.sc.intel.com> (raw)
In-Reply-To: <ZcC4Xy9xBtBsCDLq@linux.intel.com>

On Mon, Feb 05, 2024 at 11:28:47AM +0100, Stanislaw Gruszka wrote:
> On Fri, Feb 02, 2024 at 08:05:12PM -0800, Ricardo Neri wrote:
> > Enable Intel Thread Director (ITD) from the CPU hotplug callback: globally
> > from CPU0 and then enable the thread-classification hardware in each
> > logical processor individually.
> > 
> > Also, initialize the number of classes supported.
> > 
> > Currently, a bare-metal machine does not use ITD, but KVM uses the
> > attributes of the bare-metal machine to virtualize HFI.
> > 
> > Cc: Len Brown <len.brown@intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Cc: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> > Cc: Zhao Liu <zhao1.liu@linux.intel.com>
> > Cc: Zhuocheng Ding <zhuocheng.ding@intel.com>
> > Cc: x86@kernel.org
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> # intel_hfi.c
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Discussion:
> > 
> > This patch conflicts with a patchset from Stanislaw Gruszka to enable HFI
> > only if there are user space entities listening to the thermal netlink
> > events. ITD requires that HFI is enabled to function. ITD needs to be
> > unconditionally enabled for virtual machines.
> 
> Why unconditionally? From what I can tell from KVM patches (please correct
> me if I'm wrong) guests need to be modified to utilize HFI/ITD. Do we
> also have to enable HFI/ITD if no such guest run on virtual machine ?

You are correct. ITD needs to be enabled unconditionally iff we can't know
when a guest machine is running.

> 
> > Options to resolve this conflict include a command-line argument for users
> > wanting to virtualize HFI or a CONFIG_ option for the same effect. QEMU
> > could also learn to listen to thermal netlink event. A blunter option is
> > to unconditionally enable HFI when KVM is enabled at build time.
> 
> In general similar principle should be applied - do not enable if not
> needed. We should be able to get information from KVM when there is
> actual need.

Agreed, Zhao had suggested to enable/disable when virtual machines are
built/destroyed.

> QEMU registering to thermal events seems to be odd for
> me, and I think there must be better solution.

Right.