[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240322160104.22af19bd@jacob-builder>
Date: Fri, 22 Mar 2024 16:01:04 -0700
From: Jacob Pan <jacob.jun.pan@...ux.intel.com>
To: Dimitri Sivanich <sivanich@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Joerg Roedel <joro@...tes.org>,
Suravee Suthikulpanit <suravee.suthikulpanit@....com>, Will Deacon
<will@...nel.org>, Robin Murphy <robin.murphy@....com>, David Woodhouse
<dwmw2@...radead.org>, Lu Baolu <baolu.lu@...ux.intel.com>, Mark Rutland
<mark.rutland@....com>, Peter Zijlstra <peterz@...radead.org>, Arnd
Bergmann <arnd@...db.de>, YueHaibing <yuehaibing@...wei.com>,
iommu@...ts.linux.dev, linux-kernel@...r.kernel.org, Steve Wahl
<steve.wahl@....com>, Russ Anderson <russ.anderson@....com>,
jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH v2] iommu/vt-d: Allocate DMAR fault interrupts locally
Hi Dimitri,
On Thu, 21 Mar 2024 15:50:46 -0500, Dimitri Sivanich <sivanich@....com>
wrote:
> The Intel IOMMU code currently tries to allocate all DMAR fault interrupt
> vectors on the boot cpu. On large systems with high DMAR counts this
> results in vector exhaustion, and most of the vectors are not initially
> allocated socket local.
>
> Instead, have a cpu on each node do the vector allocation for the DMARs on
> that node. The boot cpu still does the allocation for its node during its
> boot sequence.
>
> Signed-off-by: Dimitri Sivanich <sivanich@....com>
> ---
>
> v2: per Thomas Gleixner, implement this from a DYN CPU hotplug state,
> though this implementation runs in CPUHP_AP_ONLINE_DYN space rather than
> CPUHP_BP_PREPARE_DYN space.
>
I tested on a dual socket system (192 core) successfully with the following:
1. After boot DMAR-MSI spread from BSP to the first CPU of the second numa
node
2. Offline/Online all CPUs in the 2nd node
Code looks good to me.
Thanks,
Jacob
> drivers/iommu/amd/amd_iommu.h | 2 +-
> drivers/iommu/amd/init.c | 2 +-
> drivers/iommu/intel/dmar.c | 9 +++++++--
> drivers/iommu/irq_remapping.c | 5 ++++-
> drivers/iommu/irq_remapping.h | 2 +-
> include/linux/dmar.h | 2 +-
> 6 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index f482aab420f7..410c360e7e24 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -33,7 +33,7 @@ int amd_iommu_prepare(void);
> int amd_iommu_enable(void);
> void amd_iommu_disable(void);
> int amd_iommu_reenable(int mode);
> -int amd_iommu_enable_faulting(void);
> +int amd_iommu_enable_faulting(unsigned int cpu);
> extern int amd_iommu_guest_ir;
> extern enum io_pgtable_fmt amd_iommu_pgtable;
> extern int amd_iommu_gpt_level;
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index e7a44929f0da..4782f690ed97 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -3389,7 +3389,7 @@ int amd_iommu_reenable(int mode)
> return 0;
> }
>
> -int __init amd_iommu_enable_faulting(void)
> +int __init amd_iommu_enable_faulting(unsigned int cpu)
> {
> /* We enable MSI later when PCI is initialized */
> return 0;
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 36d7427b1202..7644a42f283c 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -2122,7 +2122,7 @@ int dmar_set_interrupt(struct intel_iommu *iommu)
> return ret;
> }
>
> -int __init enable_drhd_fault_handling(void)
> +int enable_drhd_fault_handling(unsigned int cpu)
> {
> struct dmar_drhd_unit *drhd;
> struct intel_iommu *iommu;
> @@ -2132,7 +2132,12 @@ int __init enable_drhd_fault_handling(void)
> */
> for_each_iommu(iommu, drhd) {
> u32 fault_status;
> - int ret = dmar_set_interrupt(iommu);
> + int ret;
> +
> + if (iommu->irq || iommu->node != cpu_to_node(cpu))
> + continue;
> +
> + ret = dmar_set_interrupt(iommu);
>
> if (ret) {
> pr_err("DRHD %Lx: failed to enable fault,
> interrupt, ret %d\n", diff --git a/drivers/iommu/irq_remapping.c
> b/drivers/iommu/irq_remapping.c index ee59647c2050..2f7281ccc05f 100644
> --- a/drivers/iommu/irq_remapping.c
> +++ b/drivers/iommu/irq_remapping.c
> @@ -151,7 +151,10 @@ int __init irq_remap_enable_fault_handling(void)
> if (!remap_ops->enable_faulting)
> return -ENODEV;
>
> - return remap_ops->enable_faulting();
> + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> "dmar:enable_fault_handling",
> + remap_ops->enable_faulting, NULL);
> +
> + return remap_ops->enable_faulting(smp_processor_id());
> }
>
> void panic_if_irq_remap(const char *msg)
> diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
> index 8c89cb947cdb..0d6f140b5e01 100644
> --- a/drivers/iommu/irq_remapping.h
> +++ b/drivers/iommu/irq_remapping.h
> @@ -41,7 +41,7 @@ struct irq_remap_ops {
> int (*reenable)(int);
>
> /* Enable fault handling */
> - int (*enable_faulting)(void);
> + int (*enable_faulting)(unsigned int);
> };
>
> extern struct irq_remap_ops intel_irq_remap_ops;
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index e34b601b71fd..499bb2c63483 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -117,7 +117,7 @@ extern int dmar_remove_dev_scope(struct
> dmar_pci_notify_info *info, int count);
> /* Intel IOMMU detection */
> void detect_intel_iommu(void);
> -extern int enable_drhd_fault_handling(void);
> +extern int enable_drhd_fault_handling(unsigned int cpu);
> extern int dmar_device_add(acpi_handle handle);
> extern int dmar_device_remove(acpi_handle handle);
>
Thanks,
Jacob
Powered by blists - more mailing lists