On 10.05.2023 20:11, Jason Andryuk wrote:
> On Mon, May 8, 2023 at 6:43 AM Jan Beulich <jbeul...@suse.com> wrote:
>> On 01.05.2023 21:30, Jason Andryuk wrote:
>>> --- a/tools/misc/xenpm.c
>>> +++ b/tools/misc/xenpm.c
>>> @@ -708,6 +708,44 @@ void start_gather_func(int argc, char *argv[])
>>> pause();
>>> }
>>>
>>> +static void calculate_hwp_activity_window(const xc_hwp_para_t *hwp,
>>> + unsigned int *activity_window,
>>> + const char **units)
>>> +{
>>> + unsigned int mantissa = hwp->activity_window &
>>> HWP_ACT_WINDOW_MANTISSA_MASK;
>>> + unsigned int exponent =
>>> + (hwp->activity_window >> HWP_ACT_WINDOW_EXPONENT_SHIFT) &
>>> + HWP_ACT_WINDOW_EXPONENT_MASK;
>>
>> I wish we had MASK_EXTR() in common-macros.h. While really a comment on
>> patch 7 - HWP_ACT_WINDOW_EXPONENT_SHIFT is redundant information and
>> should imo be omitted from the public interface, in favor of just a
>> (suitably shifted) mask value. Also note how those constants all lack
>> proper XEN_ prefixes.
>
> I'll add a patch adding MASK_EXTR() & MASK_INSR() to common-macros.h
> and use those - is there any reason not to do that?
I don't think there is, but I'm also not a maintainer of that code.
>>> + unsigned int multiplier = 1;
>>> + unsigned int i;
>>> +
>>> + if ( hwp->activity_window == 0 )
>>> + {
>>> + *units = "hardware selected";
>>> + *activity_window = 0;
>>> +
>>> + return;
>>> + }
>>
>> While in line with documentation, any mantissa of 0 results in a 0us
>> window, which I assume would then also mean "hardware selected".
>
> I hadn't considered that. The hardware seems to allow you to write a
> 0 mantissa, non-0 exponent. From the SDM, it's unclear what that
> would mean. The code as written would display "0 us", "0 ms", or "0
> s" - not "0 hardware selected". Do you want more explicity printing
> for those cases? I think it's fine to have a distinction between the
> output. "0 hardware selected" is the known valid value that is
> working as expected. The other ones being something different seems
> good to me since we don't really know what they mean.
Keeping things - apart from perhaps adding a respective comment - is
okay, as long as we don't know any better.
Jan
