From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
linux-pm@vger.kernel.org, x86@kernel.org,
linux-doc@vger.kernel.org, Len Brown <len.brown@intel.com>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
Aubrey Li <aubrey.li@linux.intel.com>,
Amit Kucheria <amitk@kernel.org>, Andi Kleen <ak@linux.intel.com>,
Tim Chen <tim.c.chen@linux.intel.com>,
"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
Ricardo Neri <ricardo.neri@intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/7] thermal: intel: hfi: Minimally initialize the Hardware Feedback Interface
Date: Mon, 8 Nov 2021 18:28:41 -0800 [thread overview]
Message-ID: <20211109022841.GB16930@ranerica-svr.sc.intel.com> (raw)
In-Reply-To: <YYjkOkFmDG9IMPHu@hirez.programming.kicks-ass.net>
On Mon, Nov 08, 2021 at 09:47:54AM +0100, Peter Zijlstra wrote:
> On Fri, Nov 05, 2021 at 06:33:08PM -0700, Ricardo Neri wrote:
> > +static __init int hfi_parse_features(void)
> > +{
> > + unsigned int nr_capabilities, reg;
> > +
>
> > + /*
> > + * If we are here we know that CPUID_HFI_LEAF exists. Parse the
> > + * supported capabilities and the size of the HFI table.
> > + */
> > + reg = cpuid_edx(CPUID_HFI_LEAF);
> > +
> > + hfi_features.capabilities = reg & HFI_CAPABILITIES_MASK;
> > + if (!(hfi_features.capabilities & HFI_CAPABILITIES_PERFORMANCE)) {
> > + pr_err("Performance reporting not supported! Not using HFI\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* The number of 4KB pages required by the table */
> > + hfi_features.nr_table_pages = ((reg & CPUID_HFI_TABLE_SIZE_MASK) >>
> > + CPUID_HFI_TABLE_SIZE_SHIFT) + 1;
> > +
>
> > +/* Hardware Feedback Interface Enumeration */
> > +#define CPUID_HFI_LEAF 6
> > +#define CPUID_HFI_CAP_MASK 0xff
> > +#define CPUID_HFI_TABLE_SIZE_MASK 0x0f00
> > +#define CPUID_HFI_TABLE_SIZE_SHIFT 8
> > +#define CPUID_HFI_CPU_INDEX_MASK 0xffff0000
>
> Also, *if* you're going to do something like this, then at least write
> out the masks in full so you can easily see how they relate. The above
> is crap.
>
> > +#define CPUID_HFI_CPU_INDEX_SHIFT 16
> > +
> > +/* Hardware Feedback Interface Pointer */
> > +#define HFI_PTR_VALID_BIT BIT(0)
> > +#define HFI_PTR_ADDR_SHIFT 12
> > +
> > +/* Hardware Feedback Interface Configuration */
> > +#define HFI_CONFIG_ENABLE_BIT BIT(0)
> > +
> > +/* Hardware Feedback Interface Capabilities */
> > +#define HFI_CAPABILITIES_MASK 0xff
> > +#define HFI_CAPABILITIES_NR 8
> > +#define HFI_CAPABILITIES_PERFORMANCE BIT(0)
> > +#define HFI_CAPABILITIES_ENERGY_EFF BIT(1)
>
>
> So personally I prefer a bitfield union a-la cpuid10_eax, cpuid10_ebx
> cpuid10_edx etc.. Barring that, the above can also be written more
> concise using FIELD_GET() from bitfields.
>
> union cpuid6_edx {
> struct {
> unsigned int capabilities : 8;
> unsigned int table_size : 4;
> unsigned int __reserved : 4;
> unsigned int cpu_index : 16;
> };
> unsigned int full;
> };
Sure Peter. This looks more readable. I'll implement it like this.
Thanks and BR,
Ricardo
next prev parent reply other threads:[~2021-11-09 2:29 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-06 1:33 [PATCH 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
2021-11-06 1:33 ` [PATCH 1/7] x86/Documentation: Describe the Intel Hardware Feedback Interface Ricardo Neri
2021-11-30 9:24 ` Daniel Lezcano
2021-11-30 10:20 ` Srinivas Pandruvada
2021-11-06 1:33 ` [PATCH 2/7] x86: Add definitions for " Ricardo Neri
2021-11-06 10:30 ` Borislav Petkov
2021-11-06 22:01 ` Ricardo Neri
2021-11-06 1:33 ` [PATCH 3/7] thermal: intel: hfi: Minimally initialize the " Ricardo Neri
2021-11-08 8:47 ` Peter Zijlstra
2021-11-09 2:28 ` Ricardo Neri [this message]
2021-11-24 14:09 ` Rafael J. Wysocki
2021-11-30 3:20 ` Ricardo Neri
2021-11-30 3:55 ` Srinivas Pandruvada
2021-11-30 13:45 ` Ricardo Neri
2021-11-06 1:33 ` [PATCH 4/7] thermal: intel: hfi: Handle CPU hotplug events Ricardo Neri
2021-11-24 14:48 ` Rafael J. Wysocki
2021-11-30 13:21 ` Ricardo Neri
2021-11-30 13:32 ` Rafael J. Wysocki
2021-12-02 23:43 ` Ricardo Neri
2021-11-06 1:33 ` [PATCH 5/7] thermal: intel: hfi: Enable notification interrupt Ricardo Neri
2021-11-08 9:01 ` Peter Zijlstra
2021-11-09 15:00 ` Ricardo Neri
2021-11-08 9:07 ` Peter Zijlstra
2021-11-09 2:26 ` Ricardo Neri
2021-11-09 8:48 ` Peter Zijlstra
2021-11-09 12:54 ` Srinivas Pandruvada
2021-11-06 1:33 ` [PATCH 6/7] thermal: netlink: Add a new event to notify CPU capabilities change Ricardo Neri
2021-11-09 12:39 ` Lukasz Luba
2021-11-09 13:23 ` Srinivas Pandruvada
2021-11-09 13:53 ` Lukasz Luba
2021-11-09 14:15 ` Srinivas Pandruvada
2021-11-09 17:51 ` Lukasz Luba
2021-11-09 21:25 ` Srinivas Pandruvada
2021-11-30 9:29 ` Daniel Lezcano
2021-12-09 16:03 ` Ricardo Neri
2021-12-09 16:57 ` Daniel Lezcano
2021-12-09 17:39 ` Srinivas Pandruvada
2021-11-06 1:33 ` [PATCH 7/7] thermal: intel: hfi: Notify user space for HFI events Ricardo Neri
2021-11-24 15:18 ` Rafael J. Wysocki
2021-11-26 6:23 ` Srinivas Pandruvada
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211109022841.GB16930@ranerica-svr.sc.intel.com \
--to=ricardo.neri-calderon@linux.intel.com \
--cc=ak@linux.intel.com \
--cc=amitk@kernel.org \
--cc=aubrey.li@linux.intel.com \
--cc=daniel.lezcano@linaro.org \
--cc=len.brown@intel.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rafael.j.wysocki@intel.com \
--cc=ravi.v.shankar@intel.com \
--cc=ricardo.neri@intel.com \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=tim.c.chen@linux.intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).