| Message ID | 20230321141904.49177-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v5] ACPI: processor: Fix evaluating _PDC method when running as Xen dom0 | expand |
Hi Roger,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on rafael-pm/linux-next linus/master v6.3-rc3 next-20230321]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Roger-Pau-Monne/ACPI-processor-Fix-evaluating-_PDC-method-when-running-as-Xen-dom0/20230321-221950
base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link: https://lore.kernel.org/r/20230321141904.49177-1-roger.pau%40citrix.com
patch subject: [PATCH v5] ACPI: processor: Fix evaluating _PDC method when running as Xen dom0
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20230322/202303221035.BFY5kyh1-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/4232c8b37a0415e1e828fef4ce522c93a0b925fc
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Roger-Pau-Monne/ACPI-processor-Fix-evaluating-_PDC-method-when-running-as-Xen-dom0/20230321-221950
git checkout 4232c8b37a0415e1e828fef4ce522c93a0b925fc
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303221035.BFY5kyh1-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/x86/xen/suspend_hvm.c:4:
>> include/xen/xen.h:79:2: error: implicit declaration of function 'BUG' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
BUG();
^
1 error generated.
vim +/BUG +79 include/xen/xen.h
73
74 #if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) && defined(CONFIG_X86)
75 bool __init xen_processor_present(uint32_t acpi_id);
76 #else
77 static inline bool xen_processor_present(uint32_t acpi_id)
78 {
> 79 BUG();
80 return false;
81 }
82 #endif
83
Hi Roger,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on rafael-pm/linux-next linus/master v6.3-rc3 next-20230321]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Roger-Pau-Monne/ACPI-processor-Fix-evaluating-_PDC-method-when-running-as-Xen-dom0/20230321-221950
base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link: https://lore.kernel.org/r/20230321141904.49177-1-roger.pau%40citrix.com
patch subject: [PATCH v5] ACPI: processor: Fix evaluating _PDC method when running as Xen dom0
config: arm64-randconfig-r013-20230319 (https://download.01.org/0day-ci/archive/20230322/202303221107.hgKqaZl0-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/4232c8b37a0415e1e828fef4ce522c93a0b925fc
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Roger-Pau-Monne/ACPI-processor-Fix-evaluating-_PDC-method-when-running-as-Xen-dom0/20230321-221950
git checkout 4232c8b37a0415e1e828fef4ce522c93a0b925fc
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303221107.hgKqaZl0-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/arm64/xen/../../arm/xen/enlighten.c:2:
>> include/xen/xen.h:79:2: error: call to undeclared function 'BUG'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
BUG();
^
In file included from arch/arm64/xen/../../arm/xen/enlighten.c:4:
In file included from include/xen/grant_table.h:48:
In file included from include/xen/page.h:28:
In file included from arch/arm64/include/asm/xen/page.h:1:
In file included from include/xen/arm/page.h:9:
In file included from include/linux/dma-mapping.h:7:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/arm64/include/asm/elf.h:141:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:11: warning: array index 3 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
return (set->sig[3] | set->sig[2] |
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from arch/arm64/xen/../../arm/xen/enlighten.c:4:
In file included from include/xen/grant_table.h:48:
In file included from include/xen/page.h:28:
In file included from arch/arm64/include/asm/xen/page.h:1:
In file included from include/xen/arm/page.h:9:
In file included from include/linux/dma-mapping.h:7:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/arm64/include/asm/elf.h:141:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:25: warning: array index 2 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
return (set->sig[3] | set->sig[2] |
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from arch/arm64/xen/../../arm/xen/enlighten.c:4:
In file included from include/xen/grant_table.h:48:
In file included from include/xen/page.h:28:
In file included from arch/arm64/include/asm/xen/page.h:1:
In file included from include/xen/arm/page.h:9:
In file included from include/linux/dma-mapping.h:7:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/arm64/include/asm/elf.h:141:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:98:4: warning: array index 1 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
set->sig[1] | set->sig[0]) == 0;
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from arch/arm64/xen/../../arm/xen/enlighten.c:4:
In file included from include/xen/grant_table.h:48:
In file included from include/xen/page.h:28:
In file included from arch/arm64/include/asm/xen/page.h:1:
In file included from include/xen/arm/page.h:9:
In file included from include/linux/dma-mapping.h:7:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/arm64/include/asm/elf.h:141:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:100:11: warning: array index 1 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
return (set->sig[1] | set->sig[0]) == 0;
^ ~
include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
In file included from arch/arm64/xen/../../arm/xen/enlighten.c:4:
In file included from include/xen/grant_table.h:48:
In file included from include/xen/page.h:28:
In file included from arch/arm64/include/asm/xen/page.h:1:
In file included from include/xen/arm/page.h:9:
In file included from include/linux/dma-mapping.h:7:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/arm64/include/asm/elf.h:141:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
vim +/BUG +79 include/xen/xen.h
73
74 #if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) && defined(CONFIG_X86)
75 bool __init xen_processor_present(uint32_t acpi_id);
76 #else
77 static inline bool xen_processor_present(uint32_t acpi_id)
78 {
> 79 BUG();
80 return false;
81 }
82 #endif
83
On 21.03.23 15:19, Roger Pau Monne wrote: > In ACPI systems, the OS can direct power management, as opposed to the > firmware. This OS-directed Power Management is called OSPM. Part of > telling the firmware that the OS going to direct power management is > making ACPI "_PDC" (Processor Driver Capabilities) calls. These _PDC > methods must be evaluated for every processor object. If these _PDC > calls are not completed for every processor it can lead to > inconsistency and later failures in things like the CPU frequency > driver. > > In a Xen system, the dom0 kernel is responsible for system-wide power > management. The dom0 kernel is in charge of OSPM. However, the > number of CPUs available to dom0 can be different than the number of > CPUs physically present on the system. > > This leads to a problem: the dom0 kernel needs to evaluate _PDC for > all the processors, but it can't always see them. > > In dom0 kernels, ignore the existing ACPI method for determining if a > processor is physically present because it might not be accurate. > Instead, ask the hypervisor for this information. > > Fix this by introducing a custom function to use when running as Xen > dom0 in order to check whether a processor object matches a CPU that's > online. Such checking is done using the existing information fetched > by the Xen pCPU subsystem, extending it to also store the ACPI ID. > > This ensures that _PDC method gets evaluated for all physically online > CPUs, regardless of the number of CPUs made available to dom0. > > Fixes: 5d554a7bb064 ('ACPI: processor: add internal processor_physically_present()') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Changes since v4: > - Move definition/declaration of xen_processor_present() to different > header. > - Fold subject edit. > > Changes since v3: > - Protect xen_processor_present() definition with CONFIG_ACPI. > > Changes since v2: > - Extend and use the existing pcpu functionality. > > Changes since v1: > - Reword commit message. > --- > drivers/acpi/processor_pdc.c | 11 +++++++++++ > drivers/xen/pcpu.c | 20 ++++++++++++++++++++ > include/xen/xen.h | 10 ++++++++++ > 3 files changed, 41 insertions(+) > > diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c > index 8c3f82c9fff3..18fb04523f93 100644 > --- a/drivers/acpi/processor_pdc.c > +++ b/drivers/acpi/processor_pdc.c > @@ -14,6 +14,8 @@ > #include <linux/acpi.h> > #include <acpi/processor.h> > > +#include <xen/xen.h> > + > #include "internal.h" > > static bool __init processor_physically_present(acpi_handle handle) > @@ -47,6 +49,15 @@ static bool __init processor_physically_present(acpi_handle handle) > return false; > } > > + if (xen_initial_domain()) > + /* > + * When running as a Xen dom0 the number of processors Linux > + * sees can be different from the real number of processors on > + * the system, and we still need to execute _PDC for all of > + * them. > + */ > + return xen_processor_present(acpi_id); > + > type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; > cpuid = acpi_get_cpuid(handle, type, acpi_id); > > diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c > index fd3a644b0855..1814f8762f54 100644 > --- a/drivers/xen/pcpu.c > +++ b/drivers/xen/pcpu.c > @@ -58,6 +58,7 @@ struct pcpu { > struct list_head list; > struct device dev; > uint32_t cpu_id; > + uint32_t acpi_id; > uint32_t flags; > }; > > @@ -249,6 +250,7 @@ static struct pcpu *create_and_register_pcpu(struct xenpf_pcpuinfo *info) > > INIT_LIST_HEAD(&pcpu->list); > pcpu->cpu_id = info->xen_cpuid; > + pcpu->acpi_id = info->acpi_id; > pcpu->flags = info->flags; > > /* Need hold on xen_pcpu_lock before pcpu list manipulations */ > @@ -381,3 +383,21 @@ static int __init xen_pcpu_init(void) > return ret; > } > arch_initcall(xen_pcpu_init); > + > +#ifdef CONFIG_ACPI > +bool __init xen_processor_present(uint32_t acpi_id) > +{ > + struct pcpu *pcpu; > + bool online = false; > + > + mutex_lock(&xen_pcpu_lock); > + list_for_each_entry(pcpu, &xen_pcpus, list) > + if (pcpu->acpi_id == acpi_id) { > + online = pcpu->flags & XEN_PCPU_FLAGS_ONLINE; > + break; > + } > + mutex_unlock(&xen_pcpu_lock); > + > + return online; > +} > +#endif > diff --git a/include/xen/xen.h b/include/xen/xen.h > index 7adf59837c25..4410e74f3eb5 100644 > --- a/include/xen/xen.h > +++ b/include/xen/xen.h > @@ -71,4 +71,14 @@ static inline void xen_free_unpopulated_pages(unsigned int nr_pages, > } > #endif > > +#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) && defined(CONFIG_X86) > +bool __init xen_processor_present(uint32_t acpi_id); > +#else > +static inline bool xen_processor_present(uint32_t acpi_id) > +{ > + BUG(); Is this really a good idea? Arm64 supports ACPI, too, as well as XEN_DOM0. I think you either need to provide a stub for that case, too, or you need make this stub non-fatal for callers (I guess returning false is fine, as currently there are no hypercalls on Arm which would allow to control physical CPUs based on ACPI-Id). Stefano, can you confirm this? Juergen
On Mon, Mar 27, 2023 at 03:58:26PM +0200, Juergen Gross wrote: > On 21.03.23 15:19, Roger Pau Monne wrote: > > In ACPI systems, the OS can direct power management, as opposed to the > > firmware. This OS-directed Power Management is called OSPM. Part of > > telling the firmware that the OS going to direct power management is > > making ACPI "_PDC" (Processor Driver Capabilities) calls. These _PDC > > methods must be evaluated for every processor object. If these _PDC > > calls are not completed for every processor it can lead to > > inconsistency and later failures in things like the CPU frequency > > driver. > > > > In a Xen system, the dom0 kernel is responsible for system-wide power > > management. The dom0 kernel is in charge of OSPM. However, the > > number of CPUs available to dom0 can be different than the number of > > CPUs physically present on the system. > > > > This leads to a problem: the dom0 kernel needs to evaluate _PDC for > > all the processors, but it can't always see them. > > > > In dom0 kernels, ignore the existing ACPI method for determining if a > > processor is physically present because it might not be accurate. > > Instead, ask the hypervisor for this information. > > > > Fix this by introducing a custom function to use when running as Xen > > dom0 in order to check whether a processor object matches a CPU that's > > online. Such checking is done using the existing information fetched > > by the Xen pCPU subsystem, extending it to also store the ACPI ID. > > > > This ensures that _PDC method gets evaluated for all physically online > > CPUs, regardless of the number of CPUs made available to dom0. > > > > Fixes: 5d554a7bb064 ('ACPI: processor: add internal processor_physically_present()') > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > Changes since v4: > > - Move definition/declaration of xen_processor_present() to different > > header. > > - Fold subject edit. > > > > Changes since v3: > > - Protect xen_processor_present() definition with CONFIG_ACPI. > > > > Changes since v2: > > - Extend and use the existing pcpu functionality. > > > > Changes since v1: > > - Reword commit message. > > --- > > drivers/acpi/processor_pdc.c | 11 +++++++++++ > > drivers/xen/pcpu.c | 20 ++++++++++++++++++++ > > include/xen/xen.h | 10 ++++++++++ > > 3 files changed, 41 insertions(+) > > > > diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c > > index 8c3f82c9fff3..18fb04523f93 100644 > > --- a/drivers/acpi/processor_pdc.c > > +++ b/drivers/acpi/processor_pdc.c > > @@ -14,6 +14,8 @@ > > #include <linux/acpi.h> > > #include <acpi/processor.h> > > +#include <xen/xen.h> > > + > > #include "internal.h" > > static bool __init processor_physically_present(acpi_handle handle) > > @@ -47,6 +49,15 @@ static bool __init processor_physically_present(acpi_handle handle) > > return false; > > } > > + if (xen_initial_domain()) > > + /* > > + * When running as a Xen dom0 the number of processors Linux > > + * sees can be different from the real number of processors on > > + * the system, and we still need to execute _PDC for all of > > + * them. > > + */ > > + return xen_processor_present(acpi_id); > > + > > type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; > > cpuid = acpi_get_cpuid(handle, type, acpi_id); > > diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c > > index fd3a644b0855..1814f8762f54 100644 > > --- a/drivers/xen/pcpu.c > > +++ b/drivers/xen/pcpu.c > > @@ -58,6 +58,7 @@ struct pcpu { > > struct list_head list; > > struct device dev; > > uint32_t cpu_id; > > + uint32_t acpi_id; > > uint32_t flags; > > }; > > @@ -249,6 +250,7 @@ static struct pcpu *create_and_register_pcpu(struct xenpf_pcpuinfo *info) > > INIT_LIST_HEAD(&pcpu->list); > > pcpu->cpu_id = info->xen_cpuid; > > + pcpu->acpi_id = info->acpi_id; > > pcpu->flags = info->flags; > > /* Need hold on xen_pcpu_lock before pcpu list manipulations */ > > @@ -381,3 +383,21 @@ static int __init xen_pcpu_init(void) > > return ret; > > } > > arch_initcall(xen_pcpu_init); > > + > > +#ifdef CONFIG_ACPI > > +bool __init xen_processor_present(uint32_t acpi_id) > > +{ > > + struct pcpu *pcpu; > > + bool online = false; > > + > > + mutex_lock(&xen_pcpu_lock); > > + list_for_each_entry(pcpu, &xen_pcpus, list) > > + if (pcpu->acpi_id == acpi_id) { > > + online = pcpu->flags & XEN_PCPU_FLAGS_ONLINE; > > + break; > > + } > > + mutex_unlock(&xen_pcpu_lock); > > + > > + return online; > > +} > > +#endif > > diff --git a/include/xen/xen.h b/include/xen/xen.h > > index 7adf59837c25..4410e74f3eb5 100644 > > --- a/include/xen/xen.h > > +++ b/include/xen/xen.h > > @@ -71,4 +71,14 @@ static inline void xen_free_unpopulated_pages(unsigned int nr_pages, > > } > > #endif > > +#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) && defined(CONFIG_X86) > > +bool __init xen_processor_present(uint32_t acpi_id); > > +#else > > +static inline bool xen_processor_present(uint32_t acpi_id) > > +{ > > + BUG(); > > Is this really a good idea? > > Arm64 supports ACPI, too, as well as XEN_DOM0. I think you either need to > provide a stub for that case, too, or you need make this stub non-fatal > for callers (I guess returning false is fine, as currently there are no > hypercalls on Arm which would allow to control physical CPUs based on > ACPI-Id). Currently CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC is only selected for x86 and ia64, so I assumed if we ever needed this for Arm someone would have to write a proper handler for it for Xen. Thanks, Roger.
On 27.03.23 17:49, Roger Pau Monné wrote: > On Mon, Mar 27, 2023 at 03:58:26PM +0200, Juergen Gross wrote: >> On 21.03.23 15:19, Roger Pau Monne wrote: >>> In ACPI systems, the OS can direct power management, as opposed to the >>> firmware. This OS-directed Power Management is called OSPM. Part of >>> telling the firmware that the OS going to direct power management is >>> making ACPI "_PDC" (Processor Driver Capabilities) calls. These _PDC >>> methods must be evaluated for every processor object. If these _PDC >>> calls are not completed for every processor it can lead to >>> inconsistency and later failures in things like the CPU frequency >>> driver. >>> >>> In a Xen system, the dom0 kernel is responsible for system-wide power >>> management. The dom0 kernel is in charge of OSPM. However, the >>> number of CPUs available to dom0 can be different than the number of >>> CPUs physically present on the system. >>> >>> This leads to a problem: the dom0 kernel needs to evaluate _PDC for >>> all the processors, but it can't always see them. >>> >>> In dom0 kernels, ignore the existing ACPI method for determining if a >>> processor is physically present because it might not be accurate. >>> Instead, ask the hypervisor for this information. >>> >>> Fix this by introducing a custom function to use when running as Xen >>> dom0 in order to check whether a processor object matches a CPU that's >>> online. Such checking is done using the existing information fetched >>> by the Xen pCPU subsystem, extending it to also store the ACPI ID. >>> >>> This ensures that _PDC method gets evaluated for all physically online >>> CPUs, regardless of the number of CPUs made available to dom0. >>> >>> Fixes: 5d554a7bb064 ('ACPI: processor: add internal processor_physically_present()') >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> --- >>> Changes since v4: >>> - Move definition/declaration of xen_processor_present() to different >>> header. >>> - Fold subject edit. >>> >>> Changes since v3: >>> - Protect xen_processor_present() definition with CONFIG_ACPI. >>> >>> Changes since v2: >>> - Extend and use the existing pcpu functionality. >>> >>> Changes since v1: >>> - Reword commit message. >>> --- >>> drivers/acpi/processor_pdc.c | 11 +++++++++++ >>> drivers/xen/pcpu.c | 20 ++++++++++++++++++++ >>> include/xen/xen.h | 10 ++++++++++ >>> 3 files changed, 41 insertions(+) >>> >>> diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c >>> index 8c3f82c9fff3..18fb04523f93 100644 >>> --- a/drivers/acpi/processor_pdc.c >>> +++ b/drivers/acpi/processor_pdc.c >>> @@ -14,6 +14,8 @@ >>> #include <linux/acpi.h> >>> #include <acpi/processor.h> >>> +#include <xen/xen.h> >>> + >>> #include "internal.h" >>> static bool __init processor_physically_present(acpi_handle handle) >>> @@ -47,6 +49,15 @@ static bool __init processor_physically_present(acpi_handle handle) >>> return false; >>> } >>> + if (xen_initial_domain()) >>> + /* >>> + * When running as a Xen dom0 the number of processors Linux >>> + * sees can be different from the real number of processors on >>> + * the system, and we still need to execute _PDC for all of >>> + * them. >>> + */ >>> + return xen_processor_present(acpi_id); >>> + >>> type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; >>> cpuid = acpi_get_cpuid(handle, type, acpi_id); >>> diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c >>> index fd3a644b0855..1814f8762f54 100644 >>> --- a/drivers/xen/pcpu.c >>> +++ b/drivers/xen/pcpu.c >>> @@ -58,6 +58,7 @@ struct pcpu { >>> struct list_head list; >>> struct device dev; >>> uint32_t cpu_id; >>> + uint32_t acpi_id; >>> uint32_t flags; >>> }; >>> @@ -249,6 +250,7 @@ static struct pcpu *create_and_register_pcpu(struct xenpf_pcpuinfo *info) >>> INIT_LIST_HEAD(&pcpu->list); >>> pcpu->cpu_id = info->xen_cpuid; >>> + pcpu->acpi_id = info->acpi_id; >>> pcpu->flags = info->flags; >>> /* Need hold on xen_pcpu_lock before pcpu list manipulations */ >>> @@ -381,3 +383,21 @@ static int __init xen_pcpu_init(void) >>> return ret; >>> } >>> arch_initcall(xen_pcpu_init); >>> + >>> +#ifdef CONFIG_ACPI >>> +bool __init xen_processor_present(uint32_t acpi_id) >>> +{ >>> + struct pcpu *pcpu; >>> + bool online = false; >>> + >>> + mutex_lock(&xen_pcpu_lock); >>> + list_for_each_entry(pcpu, &xen_pcpus, list) >>> + if (pcpu->acpi_id == acpi_id) { >>> + online = pcpu->flags & XEN_PCPU_FLAGS_ONLINE; >>> + break; >>> + } >>> + mutex_unlock(&xen_pcpu_lock); >>> + >>> + return online; >>> +} >>> +#endif >>> diff --git a/include/xen/xen.h b/include/xen/xen.h >>> index 7adf59837c25..4410e74f3eb5 100644 >>> --- a/include/xen/xen.h >>> +++ b/include/xen/xen.h >>> @@ -71,4 +71,14 @@ static inline void xen_free_unpopulated_pages(unsigned int nr_pages, >>> } >>> #endif >>> +#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) && defined(CONFIG_X86) >>> +bool __init xen_processor_present(uint32_t acpi_id); >>> +#else >>> +static inline bool xen_processor_present(uint32_t acpi_id) >>> +{ >>> + BUG(); >> >> Is this really a good idea? >> >> Arm64 supports ACPI, too, as well as XEN_DOM0. I think you either need to >> provide a stub for that case, too, or you need make this stub non-fatal >> for callers (I guess returning false is fine, as currently there are no >> hypercalls on Arm which would allow to control physical CPUs based on >> ACPI-Id). > > Currently CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC is only selected for x86 and > ia64, so I assumed if we ever needed this for Arm someone would have > to write a proper handler for it for Xen. Ah, okay, I didn't check that. Sorry for the noise, Juergen
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c index 8c3f82c9fff3..18fb04523f93 100644 --- a/drivers/acpi/processor_pdc.c +++ b/drivers/acpi/processor_pdc.c @@ -14,6 +14,8 @@ #include <linux/acpi.h> #include <acpi/processor.h> +#include <xen/xen.h> + #include "internal.h" static bool __init processor_physically_present(acpi_handle handle) @@ -47,6 +49,15 @@ static bool __init processor_physically_present(acpi_handle handle) return false; } + if (xen_initial_domain()) + /* + * When running as a Xen dom0 the number of processors Linux + * sees can be different from the real number of processors on + * the system, and we still need to execute _PDC for all of + * them. + */ + return xen_processor_present(acpi_id); + type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; cpuid = acpi_get_cpuid(handle, type, acpi_id); diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c index fd3a644b0855..1814f8762f54 100644 --- a/drivers/xen/pcpu.c +++ b/drivers/xen/pcpu.c @@ -58,6 +58,7 @@ struct pcpu { struct list_head list; struct device dev; uint32_t cpu_id; + uint32_t acpi_id; uint32_t flags; }; @@ -249,6 +250,7 @@ static struct pcpu *create_and_register_pcpu(struct xenpf_pcpuinfo *info) INIT_LIST_HEAD(&pcpu->list); pcpu->cpu_id = info->xen_cpuid; + pcpu->acpi_id = info->acpi_id; pcpu->flags = info->flags; /* Need hold on xen_pcpu_lock before pcpu list manipulations */ @@ -381,3 +383,21 @@ static int __init xen_pcpu_init(void) return ret; } arch_initcall(xen_pcpu_init); + +#ifdef CONFIG_ACPI +bool __init xen_processor_present(uint32_t acpi_id) +{ + struct pcpu *pcpu; + bool online = false; + + mutex_lock(&xen_pcpu_lock); + list_for_each_entry(pcpu, &xen_pcpus, list) + if (pcpu->acpi_id == acpi_id) { + online = pcpu->flags & XEN_PCPU_FLAGS_ONLINE; + break; + } + mutex_unlock(&xen_pcpu_lock); + + return online; +} +#endif diff --git a/include/xen/xen.h b/include/xen/xen.h index 7adf59837c25..4410e74f3eb5 100644 --- a/include/xen/xen.h +++ b/include/xen/xen.h @@ -71,4 +71,14 @@ static inline void xen_free_unpopulated_pages(unsigned int nr_pages, } #endif +#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) && defined(CONFIG_X86) +bool __init xen_processor_present(uint32_t acpi_id); +#else +static inline bool xen_processor_present(uint32_t acpi_id) +{ + BUG(); + return false; +} +#endif + #endif /* _XEN_XEN_H */
In ACPI systems, the OS can direct power management, as opposed to the firmware. This OS-directed Power Management is called OSPM. Part of telling the firmware that the OS going to direct power management is making ACPI "_PDC" (Processor Driver Capabilities) calls. These _PDC methods must be evaluated for every processor object. If these _PDC calls are not completed for every processor it can lead to inconsistency and later failures in things like the CPU frequency driver. In a Xen system, the dom0 kernel is responsible for system-wide power management. The dom0 kernel is in charge of OSPM. However, the number of CPUs available to dom0 can be different than the number of CPUs physically present on the system. This leads to a problem: the dom0 kernel needs to evaluate _PDC for all the processors, but it can't always see them. In dom0 kernels, ignore the existing ACPI method for determining if a processor is physically present because it might not be accurate. Instead, ask the hypervisor for this information. Fix this by introducing a custom function to use when running as Xen dom0 in order to check whether a processor object matches a CPU that's online. Such checking is done using the existing information fetched by the Xen pCPU subsystem, extending it to also store the ACPI ID. This ensures that _PDC method gets evaluated for all physically online CPUs, regardless of the number of CPUs made available to dom0. Fixes: 5d554a7bb064 ('ACPI: processor: add internal processor_physically_present()') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v4: - Move definition/declaration of xen_processor_present() to different header. - Fold subject edit. Changes since v3: - Protect xen_processor_present() definition with CONFIG_ACPI. Changes since v2: - Extend and use the existing pcpu functionality. Changes since v1: - Reword commit message. --- drivers/acpi/processor_pdc.c | 11 +++++++++++ drivers/xen/pcpu.c | 20 ++++++++++++++++++++ include/xen/xen.h | 10 ++++++++++ 3 files changed, 41 insertions(+)