Message-ID: <e1fae51d-3511-4bcf-afbf-4e690905ccda@rivosinc.com>
Date: Mon, 10 Feb 2025 15:20:34 +0100
From: Clément Léger <cleger@...osinc.com>
To: Andrew Jones <ajones@...tanamicro.com>
Cc: Anup Patel <anup@...infault.org>, Charlie Jenkins <charlie@...osinc.com>,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
 paul.walmsley@...ive.com, palmer@...belt.com, jesse@...osinc.com,
 Anup Patel <apatel@...tanamicro.com>
Subject: Re: [PATCH 7/9] riscv: Prepare for unaligned access type table
 lookups



On 10/02/2025 15:06, Andrew Jones wrote:
> On Mon, Feb 10, 2025 at 12:07:40PM +0100, Clément Léger wrote:
>>
>>
>> On 10/02/2025 11:16, Anup Patel wrote:
>>> On Sat, Feb 8, 2025 at 6:53 AM Charlie Jenkins <charlie@...osinc.com> wrote:
>>>>
>>>> On Fri, Feb 07, 2025 at 05:19:47PM +0100, Andrew Jones wrote:
>>>>> Probing unaligned accesses on boot is time consuming. Provide a
>>>>> function which will be used to look up the access type in a table
>>>>> by id registers. Vendors which provide table entries can then skip
>>>>> the probing.
>>>>
>>>> The access checker in my experience is only time consuming on slow
>>>> hardware. Hardware that supports fast unaligned accesses isn't really
>>>> impacted by this? Avoiding a list of hardware that has slow/fast
>>>> unaligned accesses in the kernel was the main reason for dynamically
>>>> checking. We did introduce the config option to compile the kernel with
>>>> assumed slow/fast accesses, which of course has the downside of
>>>> recompiling the kernel and I assume that you already considered that.
>>>
>>> The kconfig option does not align with the vision of running the same
>>> kernel image across platforms.
>>
>> I'd would be advocating to remove compile time options as well and use
>> another way to skip the probe (see below).
>>
>>>
>>>>
>>>> Instead of having a table in the kernel, something that would be more
>>>> platform agnostic would be to have an extension that signals this
>>>> information. That seems like it would accomplish the same goal and
>>>> leverage the existing infrastructure in the kernel, albeit with the need
>>>> to make a new extension.
>>>>
>>>
>>> IMO, expecting an ISA extension to be defined for all possible
>>> microarchitectural choices is not going to scale so it is better
>>> to have infrastructure in kernel itself to infer microarchitectural
>>> choices based on RISC-V implementation ID.
>>
>> Since adding an extension seems quite unlikely, and that a device-tree
>> property is likely DT centric and not applicable to ACPI as well, was a
>> command line argument considered ?
>>
> 
> I did consider adding a command line option in addition to the table,
> allowing platforms which neither have a table entry [yet] nor want to do
> the speed test, to set whatever they like. In the end, I dropped it, since
> I don't have a use case at this time. However, if we really don't want a
> table, then I can look into the command line option instead.

Sorry if I wasn't clear, I wasn't considering this as a replacement for
your table but rather as a replacement to Charlie's compile time define
to skip misaligned speed probing since it is like "lpj=<x>". You can
specify it on command line if you want to skip the loop time detection
of loops per jiffies and have faster boot.

Regarding your table, it feels like a bit going back to old hardcoded
platform description ;). I think some kind of auto-detection of speed
(not builtin the kernel) for platforms could be good as well to skip
probing.

A DT property also seems ok to me since the goal is to describe
hardware. Would a common DT/ACPI property be appropriate ? The
device_property API unified both so if we used some common property to
describe the misaligned access speed (both in DT cpu node/ ACPI CPU
device package), we could keep a single parsing method. But I'm no ACPI
expert so I don't know if that really make sense.

Thanks,

Clément

> 
> Thanks,
> drew

Powered by blists - more mailing lists