Konrad Rzeszutek Wilk wrote:
>>>>> +struct pv_pad_ops {
>>>>> + int (*acpi_pad_init)(void);
>>>>> + void (*acpi_pad_exit)(void);
>>>>> +};
>>>>> +
>>>
>>> Looking at this a bit closer I am not sure why you choose the
>>> paravirt interface for this? There is another one - the x86 that
>>> could have been choosen. Or introduce a new one that is specific to
>>> ACPI.
>>>
>>> I am curious - what was the reason for using the paravirt interface?
>>> I understand it does get the job done, but it seems a bit overkill
>>> when something simple could have been used?
>>>
>>
>> It uses paravirt interface to avoid some code like 'xen_...' in
>> native code path (acpi_pad.c).
>> I'm not quite sure what does 'x86' here mean? Adding 2 fields
>> (acpi_pad_init/exit) in arch/x86/xen/enlighten.c --> xen_cpu_ops?
>> seems it's much simpler.
>
> arch/x86/include/asm/x86_init.h
>
> But before you go that way let me ask you another question - can ACPI
> PAD
> be used on ARM or IA64? If so, wouldn't this fail compilation as this
> pvops structure is not defined on IA64?
Ideally ACPI PAD is not bound to some arch, so IMO it could be used at least on IA64 (through currently no real PAD on IA64 platform as far as I know).
However, in native acpi_pad implementation, it indeed depends on X86 for reason like mwait.
So for xen acpi_pad, I think it's OK to choose x86, defining an acpi_pad_ops at x86_init.c which would be overwritten when xen init.
Another choice is to define config ACPI_PROCESSOR_AGGREGATOR as 'bool', which would disable native acpi_pad module.
Your opinion?
Thanks,
Jinsong
>
> The other thing I am not comfortable about is that the pvops structure
> are used for low-level code. Not for higher up, like ACPI. For that
> another structure seems more prudent. Perhaps something like the x86
> one, but specific to ACPI?
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
