| Message ID | 20221110163145.80374-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2dbfb3f33350e1e868d3d7ed4c176d8777150878 |
| Headers | show |
| Series | [v3] platform/x86: don't unconditionally attach Intel PMC when virtualized | expand |
On Thu, Nov 10, 2022 at 05:31:44PM +0100, Roger Pau Monne wrote: > The current logic in the Intel PMC driver will forcefully attach it > when detecting any CPU on the intel_pmc_core_platform_ids array, > even if the matching ACPI device is not present. > > There's no checking in pmc_core_probe() to assert that the PMC device > is present, and hence on virtualized environments the PMC device > probes successfully, even if the underlying registers are not present. > Previous to 21ae43570940 the driver would check for the presence of a > specific PCI device, and that prevented the driver from attaching when > running virtualized. > > Fix by only forcefully attaching the PMC device when not running > virtualized. Note that virtualized platforms can still get the device > to load if the appropriate ACPI device is present on the tables > provided to the VM. > > Make an exception for the Xen initial domain, which does have full > hardware access, and hence can attach to the PMC if present. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Fixes: 21ae43570940 ('platform/x86: intel_pmc_core: Substitute PCI with CPUID enumeration') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Acked-by: David E. Box <david.e.box@linux.intel.com> > --- > Changes since v2: > - Don't split condition line. > > Changes since v1: > - Use cpu_feature_enabled() instead of boot_cpu_has(). > --- > drivers/platform/x86/intel/pmc/pltdrv.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/platform/x86/intel/pmc/pltdrv.c b/drivers/platform/x86/intel/pmc/pltdrv.c > index 15ca8afdd973..ddfba38c2104 100644 > --- a/drivers/platform/x86/intel/pmc/pltdrv.c > +++ b/drivers/platform/x86/intel/pmc/pltdrv.c > @@ -18,6 +18,8 @@ > #include <asm/cpu_device_id.h> > #include <asm/intel-family.h> > > +#include <xen/xen.h> > + > static void intel_pmc_core_release(struct device *dev) > { > kfree(dev); > @@ -53,6 +55,13 @@ static int __init pmc_core_platform_init(void) > if (acpi_dev_present("INT33A1", NULL, -1)) > return -ENODEV; > > + /* > + * Skip forcefully attaching the device for VMs. Make an exception for > + * Xen dom0, which does have full hardware access. > + */ > + if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) && !xen_initial_domain()) > + return -ENODEV; > + > if (!x86_match_cpu(intel_pmc_core_platform_ids)) > return -ENODEV; > > -- > 2.37.3 >
Hi, On 11/10/22 17:31, Roger Pau Monne wrote: > The current logic in the Intel PMC driver will forcefully attach it > when detecting any CPU on the intel_pmc_core_platform_ids array, > even if the matching ACPI device is not present. > > There's no checking in pmc_core_probe() to assert that the PMC device > is present, and hence on virtualized environments the PMC device > probes successfully, even if the underlying registers are not present. > Previous to 21ae43570940 the driver would check for the presence of a > specific PCI device, and that prevented the driver from attaching when > running virtualized. > > Fix by only forcefully attaching the PMC device when not running > virtualized. Note that virtualized platforms can still get the device > to load if the appropriate ACPI device is present on the tables > provided to the VM. > > Make an exception for the Xen initial domain, which does have full > hardware access, and hence can attach to the PMC if present. > > Fixes: 21ae43570940 ('platform/x86: intel_pmc_core: Substitute PCI with CPUID enumeration') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Acked-by: David E. Box <david.e.box@linux.intel.com> Thank you for your patch, I've applied this patch to my fixes branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes Note it will show up in my fixes branch once I've pushed my local branch there, which might take a while. I will include this patch in my next fixes pull-req to Linus for the current kernel development cycle. Regards, Hans > --- > Changes since v2: > - Don't split condition line. > > Changes since v1: > - Use cpu_feature_enabled() instead of boot_cpu_has(). > --- > drivers/platform/x86/intel/pmc/pltdrv.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/platform/x86/intel/pmc/pltdrv.c b/drivers/platform/x86/intel/pmc/pltdrv.c > index 15ca8afdd973..ddfba38c2104 100644 > --- a/drivers/platform/x86/intel/pmc/pltdrv.c > +++ b/drivers/platform/x86/intel/pmc/pltdrv.c > @@ -18,6 +18,8 @@ > #include <asm/cpu_device_id.h> > #include <asm/intel-family.h> > > +#include <xen/xen.h> > + > static void intel_pmc_core_release(struct device *dev) > { > kfree(dev); > @@ -53,6 +55,13 @@ static int __init pmc_core_platform_init(void) > if (acpi_dev_present("INT33A1", NULL, -1)) > return -ENODEV; > > + /* > + * Skip forcefully attaching the device for VMs. Make an exception for > + * Xen dom0, which does have full hardware access. > + */ > + if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) && !xen_initial_domain()) > + return -ENODEV; > + > if (!x86_match_cpu(intel_pmc_core_platform_ids)) > return -ENODEV; >
diff --git a/drivers/platform/x86/intel/pmc/pltdrv.c b/drivers/platform/x86/intel/pmc/pltdrv.c index 15ca8afdd973..ddfba38c2104 100644 --- a/drivers/platform/x86/intel/pmc/pltdrv.c +++ b/drivers/platform/x86/intel/pmc/pltdrv.c @@ -18,6 +18,8 @@ #include <asm/cpu_device_id.h> #include <asm/intel-family.h> +#include <xen/xen.h> + static void intel_pmc_core_release(struct device *dev) { kfree(dev); @@ -53,6 +55,13 @@ static int __init pmc_core_platform_init(void) if (acpi_dev_present("INT33A1", NULL, -1)) return -ENODEV; + /* + * Skip forcefully attaching the device for VMs. Make an exception for + * Xen dom0, which does have full hardware access. + */ + if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) && !xen_initial_domain()) + return -ENODEV; + if (!x86_match_cpu(intel_pmc_core_platform_ids)) return -ENODEV;
The current logic in the Intel PMC driver will forcefully attach it when detecting any CPU on the intel_pmc_core_platform_ids array, even if the matching ACPI device is not present. There's no checking in pmc_core_probe() to assert that the PMC device is present, and hence on virtualized environments the PMC device probes successfully, even if the underlying registers are not present. Previous to 21ae43570940 the driver would check for the presence of a specific PCI device, and that prevented the driver from attaching when running virtualized. Fix by only forcefully attaching the PMC device when not running virtualized. Note that virtualized platforms can still get the device to load if the appropriate ACPI device is present on the tables provided to the VM. Make an exception for the Xen initial domain, which does have full hardware access, and hence can attach to the PMC if present. Fixes: 21ae43570940 ('platform/x86: intel_pmc_core: Substitute PCI with CPUID enumeration') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: David E. Box <david.e.box@linux.intel.com> --- Changes since v2: - Don't split condition line. Changes since v1: - Use cpu_feature_enabled() instead of boot_cpu_has(). --- drivers/platform/x86/intel/pmc/pltdrv.c | 9 +++++++++ 1 file changed, 9 insertions(+)