linux-kernel - Re: [PATCH RESEND v3 3/6] i3c: mipi-i3c-hci: Add a quirk to set PIO mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <94e789ab-2e64-4a6e-804f-b5e7b2a952cb@amd.com>
Date: Fri, 9 Aug 2024 21:14:48 +0530
From: Shyam Sundar S K <Shyam-sundar.S-k@....com>
To: Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: Guruvendra Punugupati <Guruvendra.Punugupati@....com>,
 Krishnamoorthi M <krishnamoorthi.m@....com>, linux-i3c@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v3 3/6] i3c: mipi-i3c-hci: Add a quirk to set PIO
 mode



On 8/9/2024 19:25, Jarkko Nikula wrote:
> Hi
> 
> On 8/7/24 8:23 AM, Shyam Sundar S K wrote:
>> The AMD HCI controller currently only supports PIO mode but exposes DMA
>> rings to the OS, which leads to the controller being configured in DMA
>> mode. To address this, add a quirk to avoid configuring the
>> controller in
>> DMA mode and default to PIO mode.
>>
>> Additionally, introduce a generic quirk infrastructure to the
>> mipi-i3c-hci
>> driver to facilitate seamless future quirk additions.
>>
>> Co-developed-by: Krishnamoorthi M <krishnamoorthi.m@....com>
>> Signed-off-by: Krishnamoorthi M <krishnamoorthi.m@....com>
>> Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@....com>
>> Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@....com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@....com>
>> ---
> 
> ...
> 
>> +void amd_i3c_hci_quirks_init(struct i3c_hci *hci)
>> +{
>> +#if defined(CONFIG_X86)
>> +    if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>> +        hci->quirks |= HCI_QUIRK_PIO_MODE;
>> +#endif
>> +}
> 
> I was thinking these quirks can be passed as driver_data more cleanly
> and be specific only to affected HW if AMD HW would have an unique
> ACPI ID for each HW version.
> 
> Above X86_VENDOR_AMD might be too generic if and when quirks are fixed
> in the future HW :-)
> 
> So something like:
> 
> static const struct acpi_device_id i3c_hci_acpi_match[] = {
>     {"AMDI1234", HCI_QUIRK_PIO_MODE | HCI_QUIRK_OD_PP_TIMING |
> HCI_QUIRK_RESP_BUF_THLD},
>     {}
> };
> 
> and set them in the i3c_hci_probe() as:
> 
> hci->quirks = (unsigned long)device_get_match_data(&pdev->dev);

Nice idea. But only problem is that MSFT wants to have the same ACPI
ID present in the specification.

I have replied to Andy on patch 1/6. Can you please put your remarks
there?

Yeah, agreed that having X86_VENDOR_AMD is too generic, but felt its
good to have additional checks only after the HW is fixed, rather than
being speculative now.. :-)

What would you advise?

Thanks,
Shyam