Re: [RFC PATCH V2] acpi/irq: Add stacked IRQ domain support to PCI interrupt link

From: Chen Baozi <chenbaozi@phytium.com.cn>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Guohanjun <guohanjun@huawei.com>, Marc Zyngier <maz@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Subject: Re: [RFC PATCH V2] acpi/irq: Add stacked IRQ domain support to PCI interrupt link
Date: Wed, 18 Nov 2020 18:37:49 +0800	[thread overview]
Message-ID: <C1B76F68-1692-4E46-90BD-33BE74B445FF@phytium.com.cn> (raw)
In-Reply-To: <20201117185720.GA1397876@bjorn-Precision-5520>

Hi Bjorn,

Thanks for reviewing. I’ll try to fix all existing issues and send a new
version later.

> On Nov 18, 2020, at 2:57 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> Nit: please don't just make up random styles for the subject.  Run
> "git log --oneline" on the file and/or the directory and try to follow
> the existing convention.  Using random styles adds noise to the
> system.
> 
> On Tue, Nov 17, 2020 at 09:42:14PM +0800, Chen Baozi wrote:
>> 
>> Therefore, we introduce an stacked IRQ domain support to PCI interrupt
>> link for ACPI. With this support, we can populate the ResourceSource
>> to refer to a device object that describes an interrupt controller.
>> That would allow us to refer to a dedicated driver which implements
>> the logic needed to manage the interrupt state. With this patch,
>> those PCI interrupt links can be supported by describing the INTx
>> in ACPI table as the following example:
> 
> "Stacked IRQ domain" sounds like a detail of how you're implementing
> support for the Resource Source field for PCI Interrupt Links.
> 
> I don't know what the dedicated driver refers to.  This *should* be
> all generic code the follows the ACPI spec (which is pretty sketchy in
> this area).  But I assume that there's no special driver needed for
> devices like \SB.IXIU, and the logic associated with the interrupt
> controller is in the AML associated with IXIU.  It would probably be
> useful to mention the relevant methods in the IXIU methods in the
> example below.
> 
> From ACPI v6.3, Table 6-200, it looks like this patch should include
> changes to acpi_bus_osc_support() to advertise "Interrupt
> ResourceSource support".

According to my understanding, does it mean to add something like：

+       capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_INTR_RS_SUPPORT;

and check whether the platform supports usage of ResourceSource after
acpi_run_osc() returns successfully:

+		osc_sb_intr_rs_support_confirmed =
+			capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_INTR_RS_SUPPORT;

with this bool value, we can then determine if we would ignore the
ResourceSource field later.

Or we just advertise this capability from the OS side without introducing
the ‘osc_sb_intr_rs_support_confirmed’?

I am not certain about it because:

1) If we strictly flow the spec, which says “the platform will indicate to
OS whether ... If not set, the OS may choose to ignore the ResourceSource
parameter in the extended interrupt descirptor”, this means this capability
can be used to determine whether we would ignore to parse the field later.

2) On the other hand, Since the ResourceSource has already been used to
create hierarchical domain for platform device (introduced by 621dc2fdcea1)
and previous driver does not check this capability, I am not sure whether
it would break the existing platforms. 

Fix me if I am wrong.

Cheers,

Chen Baozi.