Re: [PATCH][pvops_dom0][4/4] use physical acpi_id in acpi processor parsing logic

[Jeremy Fitzhardinge <jeremy@goop.org>]

On 07/18/09 23:46, Yu, Ke wrote:
> use physical acpi_id in acpi processor parsing logic
>
> xen domain0 is seeing vCPU, while xen need the acpi info of physical CPU, so this patch hack the acpi processor parsing logic to use physical acpi_id instead vcpu id
>   

Would there be a problem with making this always use the acpi id?  That
would get rid of the Xen-specific stuff, and makes logical sense (at
least to me) since this code is dealing with APIC processor objects.

I guess this goes back to my earlier comment; could this code be defined
in terms of ACPI processors rather than general CPUs, so that the Xen
(or any virtual) case can distinguish the two notions, while they can be
mapped 1:1 for the native case?  (Or perhaps there are reasons you might
want to have ACPI control processor power states which are logically
offline to the kernel?)

    J

> From: Yu Ke <ke.yu@intel.com>
>
> Signed-off-by: Yu Ke <ke.yu@intel.com>
> ---
>
>  arch/x86/kernel/acpi/processor.c |    4 ++++
>  drivers/acpi/processor_core.c    |   11 ++++++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
>
> diff --git a/arch/x86/kernel/acpi/processor.c b/arch/x86/kernel/acpi/processor.c
> index 9f0b296..d4d57a9 100644
> --- a/arch/x86/kernel/acpi/processor.c
> +++ b/arch/x86/kernel/acpi/processor.c
> @@ -11,6 +11,7 @@
>  
>  #include <acpi/processor.h>
>  #include <asm/acpi.h>
> +#include <asm/xen/hypervisor.h>
>  
>  static void init_intel_pdc(struct acpi_processor *pr, struct cpuinfo_x86 *c)
>  {
> @@ -82,6 +83,9 @@ void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
>  {
>  	struct cpuinfo_x86 *c = &cpu_data(pr->id);
>  
> +	if (xen_pv_domain())
> +		c = &cpu_data(0);
> +
>  	pr->pdc = NULL;
>  	if (c->x86_vendor == X86_VENDOR_INTEL)
>  		init_intel_pdc(pr, c);
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 0b6facc..a124c4a 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -638,6 +638,9 @@ static int acpi_processor_get_info(struct acpi_device *device)
>  
>  	pr->id = cpu_index;
>  
> +	if (xen_pv_domain())
> +		pr->id = pr->acpi_id;
> +
>  	/*
>  	 *  Extra Processor objects may be enumerated on MP systems with
>  	 *  less than the max # of CPUs. They should be ignored _iff
> @@ -703,14 +706,16 @@ static int __cpuinit acpi_processor_start(struct acpi_device *device)
>  		return 0;
>  	}
>  
> -	BUG_ON((pr->id >= nr_cpu_ids) || (pr->id < 0));
> +	if (!xen_pv_domain())
> +		BUG_ON((pr->id >= nr_cpu_ids) || (pr->id < 0));
>  
>  	/*
>  	 * Buggy BIOS check
>  	 * ACPI id of processors can be reported wrongly by the BIOS.
>  	 * Don't trust it blindly
>  	 */
> -	if (per_cpu(processor_device_array, pr->id) != NULL &&
> +	if (!xen_pv_domain() &&
> +		per_cpu(processor_device_array, pr->id) != NULL &&
>  	    per_cpu(processor_device_array, pr->id) != device) {
>  		printk(KERN_WARNING "BIOS reported wrong ACPI id "
>  			"for the processor\n");
> @@ -725,7 +730,7 @@ static int __cpuinit acpi_processor_start(struct acpi_device *device)
>  		goto end;
>  
>  	sysdev = get_cpu_sysdev(pr->id);
> -	if (sysfs_create_link(&device->dev.kobj, &sysdev->kobj, "sysdev"))
> +	if (sysdev != NULL && sysfs_create_link(&device->dev.kobj, &sysdev->kobj, "sysdev"))
>  		return -EFAULT;
>  
>  	/* _PDC call should be done before doing anything else (if reqd.). */
>
>