Re: [Xen-devel] [PATCH 1/3] PAD helper for native and paravirt platform

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

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?


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xensource.com/xen-devel