Received-SPF: pass (google.com: domain of linux-kernel+bounces-89052-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Thomas Gleixner <tglx@linutronix.de>
To: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Dimitri Sivanich <sivanich@hpe.com>, David Woodhouse
 <dwmw2@infradead.org>, Lu Baolu <baolu.lu@linux.intel.com>, Joerg Roedel
 <joro@8bytes.org>, Will Deacon <will@kernel.org>, Robin Murphy
 <robin.murphy@arm.com>, iommu@lists.linux.dev,
 linux-kernel@vger.kernel.org, Steve Wahl <steve.wahl@hpe.com>, Russ
 Anderson <russ.anderson@hpe.com>, jacob.jun.pan@linux.intel.com
Subject: Re: [PATCH] Allocate DMAR fault interrupts locally
In-Reply-To: <20240301115000.4ba2e411@jacob-builder>
References: <ZeDj/LK56borSxO4@hpe.com> <87plwe7w3m.ffs@tglx>
 <20240301115000.4ba2e411@jacob-builder>
Date: Fri, 01 Mar 2024 20:59:22 +0100
Message-ID: <87msrhwwo5.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Jacob!

On Fri, Mar 01 2024 at 11:50, Jacob Pan wrote:
> On Thu, 29 Feb 2024 23:18:37 +0100, Thomas Gleixner <tglx@linutronix.de>
> wrote:
>> On Thu, Feb 29 2024 at 14:07, Dimitri Sivanich wrote:
>> The real problems are:
>> 
>>  1) This approach only works when _ALL_ APs have been brought up during
>>     boot. With 'maxcpus=N' on the command line this will fail to enable
>>     fault handling when the APs which have not been brought up initially
>>     are onlined later on.
>> 
>>     This might be working in practice because intel_iommu_init() will
>>     enable the interrupts later on via init_dmars() unconditionally, but
>>     that's far from correct because IRQ_REMAP does not depend on
>>     INTEL_IOMMU.
> The dmar fault interrupt is VT-d's own interrupt, not subject to IRQ_REMAP.
> So this set up has nothing to do with IR, right?

Both interrupt remap and the IOMMU use DMAR and both set the DMAR's
fault interrupt up, whatever comes first. If IR is enabled then IR does
it, if not then the IOMMU init handles it.

But yes, the interrupt is part of DMAR.

> Maybe we should not call it irq_remap_work, call it dmar_fault_irq_work
> instead?

This work thing does not work at all as I explained in detail. No matter
how you name it. The only sane way to do that is with a hotplug state.

Thanks,

        tglx