On 19.06.2025 10:43, Penny, Zheng wrote:
> [Public]
>
>> -----Original Message-----
>> From: Jan Beulich <jbeul...@suse.com>
>> Sent: Thursday, June 12, 2025 6:42 PM
>> To: Penny, Zheng <penny.zh...@amd.com>
>> Cc: Huang, Ray <ray.hu...@amd.com>; Andrew Cooper
>> <andrew.coop...@citrix.com>; Anthony PERARD <anthony.per...@vates.tech>;
>> Orzel, Michal <michal.or...@amd.com>; Julien Grall <jul...@xen.org>; Roger
>> Pau
>> Monné <roger....@citrix.com>; Stefano Stabellini <sstabell...@kernel.org>;
>> xen-
>> de...@lists.xenproject.org
>> Subject: Re: [PATCH v5 06/18] xen/x86: introduce "cpufreq=amd-cppc" xen
>> cmdline
>>
>> On 27.05.2025 10:48, Penny Zheng wrote:
>>> --- a/xen/arch/x86/platform_hypercall.c
>>> +++ b/xen/arch/x86/platform_hypercall.c
>>> @@ -94,6 +95,8 @@ static int __init handle_cpufreq_cmdline(enum
>>> cpufreq_xen_opt option) {
>>> int ret = 0;
>>>
>>> + /* Do not occupy bits reserved for public xen-pm */
>>> + BUILD_BUG_ON(MASK_INSR(XEN_PROCESSOR_PM_CPPC,
>> SIF_PM_MASK));
>>
>> This looks like an abuse of MASK_INSR(). Why not simply
>>
>> BUILD_BUG_ON(XEN_PROCESSOR_PM_CPPC & SIF_PM_MASK);
>>
>> ?
>
> Because in SIF_PM_MASK, it's bit 8 to 15 reserved for xen-pm options,
> See "
> #define SIF_PM_MASK (0xFF<<8) /* reserve 1 byte for xen-pm options */
> "
> So I'm trying to use MASK_INSR() to do the necessary right shift (other than
> using 8 directly, in case SIF_PM_MASK changes in the future...)
Oh, right, so my replacement suggestion was wrong. But XEN_PROCESSOR_PM_CPPC
isn't contained within the 8 bits. MASK_INSR() could conceivably have a
(debug) check that the passed value actually fits the mask.
BUILD_BUG_ON(XEN_PROCESSOR_PM_CPPC & MASK_EXTR(~0, SIF_PM_MASK));
?
Jan
