Received-SPF: pass (google.com: domain of linux-kernel+bounces-205688-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <d1ce4b09-c0ac-447e-8779-633bfb2bd8c3@arm.com>
Date: Fri, 7 Jun 2024 10:10:17 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v6 1/1] irqchip/gic-v3: Enable non-coherent
 redistributors/ITSes ACPI probing
To: Amit Singh Tomar <amitsinght@marvell.com>,
 Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: linux-kernel@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
 Thomas Gleixner <tglx@linutronix.de>, Mark Rutland <mark.rutland@arm.com>,
 "Rafael J. Wysocki" <rafael@kernel.org>,
 linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org,
 acpica-devel@lists.linux.dev
References: <20240123110332.112797-1-lpieralisi@kernel.org>
 <20240606094238.757649-1-lpieralisi@kernel.org>
 <20240606094238.757649-2-lpieralisi@kernel.org>
 <f901fd06-3b6b-4444-a3da-2b75c59059d3@marvell.com>
 <ZmK5Ilk37GyJ4QEa@lpieralisi>
 <02dba55e-39a3-4853-ac52-834cb1c0fc7f@marvell.com>
From: Robin Murphy <robin.murphy@arm.com>
Content-Language: en-GB
In-Reply-To: <02dba55e-39a3-4853-ac52-834cb1c0fc7f@marvell.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 2024-06-07 8:53 am, Amit Singh Tomar wrote:
>> On Fri, Jun 07, 2024 at 12:21:54AM +0530, Amit Singh Tomar wrote:
>>
>> [...]
>>
>>>> diff --git a/drivers/acpi/processor_core.c 
>>>> b/drivers/acpi/processor_core.c
>>>> index b203cfe28550..915713c0e9b7 100644
>>>> --- a/drivers/acpi/processor_core.c
>>>> +++ b/drivers/acpi/processor_core.c
>>>> @@ -215,6 +215,21 @@ phys_cpuid_t __init acpi_map_madt_entry(u32 
>>>> acpi_id)
>>>>        return rv;
>>>>    }
>>>> +int __init acpi_get_madt_revision(void)
>>>
>>> Wondering, if we can have a generic function (acpi_get_tbl_revision) to
>>> obtain the revision number for any ACPI table, not just specific to 
>>> MADT?
>>
>> We could - I don't think there would be users other than code in this
>> patch though so I thought it would not be necessary.
>>
> 
> Right, it might not be essential now but I see that MPAM will be another 
> user of it once the MPAM patches are out.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers/acpi/arm64/mpam.c?h=mpam/snapshot/v6.7-rc2#n299

Not really; there's already plenty of ACPI code which checks the 
revision of a table *while* also parsing other information from it, and 
that MPAM code is doing the same. Using a standalone function to look up 
the table, check one thing and throw it away, and then immediately have 
to look it up again to do the rest would be needlessly overcomplicated.

The thing in the GIC case is that doing this semi-redundant lookup to 
re-retrieve the top-level MADT header while we're already deep into 
parsing its subtables is still the least-worst option, because the 
alternative would be invasively churning the whole common MADT 
abstraction to pass that information all the way down just for this one 
slightly niche thing.

Thanks,
Robin.