Re: [Xen-devel] [RFC PATCH 03/31] pmstat: move pmstat.c file to the xen/drivers/pm/stat.c location

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

Hi,

On Fri, May 18, 2018 at 2:35 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 18.05.18 at 13:14, <olekstysh@xxxxxxxxx> wrote:
>> On Mon, May 7, 2018 at 6:36 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>> On 09.11.17 at 18:09, <olekstysh@xxxxxxxxx> wrote:
>>>> +int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE_PARAM(uint32) pdc)
>>>> +{
>>>> +    u32 bits[3];
>>>> +    int ret;
>>>> +
>>>> +    if ( copy_from_guest(bits, pdc, 2) )
>>>> +        ret = -EFAULT;
>>>> +    else if ( bits[0] != ACPI_PDC_REVISION_ID || !bits[1] )
>>>> +        ret = -EINVAL;
>>>> +    else if ( copy_from_guest_offset(bits + 2, pdc, 2, 1) )
>>>> +        ret = -EFAULT;
>>>> +    else
>>>> +    {
>>>> +        u32 mask = 0;
>>>> +
>>>> +        if ( xen_processor_pmbits & XEN_PROCESSOR_PM_CX )
>>>> +            mask |= ACPI_PDC_C_MASK | ACPI_PDC_SMP_C1PT;
>>>> +        if ( xen_processor_pmbits & XEN_PROCESSOR_PM_PX )
>>>> +            mask |= ACPI_PDC_P_MASK | ACPI_PDC_SMP_C1PT;
>>>> +        if ( xen_processor_pmbits & XEN_PROCESSOR_PM_TX )
>>>> +            mask |= ACPI_PDC_T_MASK | ACPI_PDC_SMP_C1PT;
>>>> +        bits[2] &= (ACPI_PDC_C_MASK | ACPI_PDC_P_MASK | ACPI_PDC_T_MASK |
>>>> +                    ACPI_PDC_SMP_C1PT) & ~mask;
>>>> +        ret = arch_acpi_set_pdc_bits(acpi_id, bits, mask);
>>>> +    }
>>>> +    if ( !ret && __copy_to_guest_offset(pdc, 2, bits + 2, 1) )
>>>> +        ret = -EFAULT;
>>>> +
>>>> +    return ret;
>>>> +}
>>>
>>> Looks quite ACPI-specific.
>> Yes, current patch does just a movement.
>>
>> Next patch [1] wraps it in #ifdef CONFIG_ACPI.
>>
>> However during patch discussion we decided to move this function to arch/x86.
>> It is called from arch/x86/platform_hypercall.c and pulls a bunch of
>> #define-s from pdc_intel.h
>
> Not sure - the function may be used by x86 only right now, but is what it
> does really x86-specific?

I am not familiar with ACPI to answer precisely.
What I see here is that these are named "Intel Processor Driver
Capabilities flags".

However, the Section 8.4.1 of document [1] doesn't explicitly say that
"_PDC" is supposed to be x86 specific thing only.

[1] http://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf

So, I can leave acpi_set_pdc_bits() in xen/drivers/pm/stat.c for now,
but definitely wrapped into #ifdef CONFIG_ACPI.

What do you think?

>
> Jan
>
>

-- 
Regards,

Oleksandr Tyshchenko

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