Re: [Xen-devel] [RFC PATCH 06/31] cpufreq: make cpufreq driver more generalizable

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

Hi, Stefano, Jan

On Thu, Dec 7, 2017 at 10:45 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 07.12.17 at 00:44, <sstabellini@xxxxxxxxxx> wrote:
>> Oleksandr would like to call set_px_pminfo from a non-hypercall context,
>> meaning that there are no XEN_GUEST_HANDLE parameters. Today, struct
>> xen_processor_performance contains a
>>
>>   XEN_GUEST_HANDLE(xen_processor_px_t) states;
>>
>> field. Instead of "faking" the XEN_GUEST_HANDLE field from Xen, I
>> suggested to modify set_px_pminfo to take a different struct, one
>> without any XEN_GUEST_HANDLE field. For example:
>>
>>  struct xen_processor_performance_internal {
>>      uint32_t flags;     /* flag for Px sub info type */
>>      uint32_t platform_limit;  /* Platform limitation on freq usage */
>>      struct xen_pct_register control_register;
>>      struct xen_pct_register status_register;
>>      uint32_t state_count;     /* total available performance states */
>>      struct xen_processor_px states;   <---- this is the interesting change
>>      struct xen_psd_package domain_info;
>>      uint32_t shared_type;     /* coordination type of this processor */
>>  };
>>
>> The caller, in the x86 case is
>> xen/arch/x86/platform_hypercall.c:do_platform_op, would be resposible
>> for issuing the copy_from_guest.
Stefano, thank you for the detailed clarification.

>
> I think we don't want yet another variant of the structure: I'd
> then prefer to have a function doing the translation from struct
> xen_processor_performance to struct processor_performance,
> and hand the result to set_px_pminfo(). For consistency I'd then
> like to ask though that the same be done for set_cx_pminfo().

Jan, Stefano, thank you for suggestions.

Have questions which need to be clarified:

If I understood correctly, new variant of set_px_pminfo is going to
have an extra "flag" argument, since
struct processor_performance doesn't have "flag" field (it contains
"state" field instead, which has yet another meaning).
Something like that:
int set_px_pminfo(uint32_t acpi_id, uint32_t flag, struct
processor_performance *dom0_px_info)
Is my understanding correct?

As for set_cx_pminfo(). To what struct we should do translation from
struct xen_processor_power? (struct acpi_processor_power?)

Briefly looking at set_cx_pminfo(), I got a feeling, that in order to
modify it in a "set_px_pminfo() manner"
we need to rework print_cx_pminfo(),  set_cx(), check_cx(),
acpi_processor_ffh_cstate_probe() too, since
all these function have arguments which contain XEN_GUEST_HANDLE. I am
wondering is it worth
doing such rework taking into the account that set_cx_pminfo() is not
going to be called from the non-hypercall context.
Or I missed something?

>
> Jan
>



-- 
Regards,

Oleksandr Tyshchenko

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel