On Tue, Feb 21, 2012 at 05:49:58AM +0000, Liu, Jinsong wrote: > 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. OK, or in osl.c. We need Len to chime in here as I can see this expanding in the future. > > Another choice is to define config ACPI_PROCESSOR_AGGREGATOR as 'bool', which would disable native acpi_pad module. Ewww. No. > > 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