linux-kernel - Re: [PATCH] cdx: add MSI support for CDX bus

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]

Date:   Fri, 12 May 2023 20:15:05 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Nipun Gupta <nipun.gupta@....com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "maz@...nel.org" <maz@...nel.org>, "jgg@...pe.ca" <jgg@...pe.ca>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc:     "git (AMD-Xilinx)" <git@....com>,
        "Anand, Harpreet" <harpreet.anand@....com>,
        "Jansen Van Vuuren, Pieter" <pieter.jansen-van-vuuren@....com>,
        "Agarwal, Nikhil" <nikhil.agarwal@....com>,
        "Simek, Michal" <michal.simek@....com>,
        "Gangurde, Abhijit" <abhijit.gangurde@....com>,
        "Cascon, Pablo" <pablo.cascon@....com>
Subject: Re: [PATCH] cdx: add MSI support for CDX bus

Nipun!

On Fri, May 12 2023 at 19:50, Nipun Gupta wrote:
> On 5/11/2023 3:59 AM, Thomas Gleixner wrote:
>> CDX is not any different than PCI. The actual "interrupt chip" is not
>> part of the bus, it's part of the device and pretending that it is a bus
>> specific thing is just running in to the same cul-de-sac sooner than
>> later.
>
> I understand your viewpoint, but would state that CDX bus is somewhat 
> different than PCI in the sense that firmware is a controller for
> all the devices and their configuration. CDX bus controller sends all 
> the write_msi_msg commands to firmware running on RPU over the RPmsg and 
> it is the firmware which interfaces with actual devices to pass this 
> information to devices in a way agreed between firmware and device. The 
> only way to pass MSI information to device is via firmware and CDX bus 
> controller is only entity which can communicate with the firmware for
> this.

Fair enough, but we wouldn't had this dicussion if the above information
would have been part of the changelog. See?

>> IIRC, there is a gap vs. interrupt affinity setting from user space,
>> which is irrelevant for I2C, SPI etc. configured interrupt chips as they
>> raise interrupt via an SoC interrupt pin and that's the entity which
>> does the affinity management w/o requiring I2C/SPI. IIRC I posted a
>> patch snippet to that effect in one of those lengthy PCI/MSI/IMS threads
>> because that is also required for MSI storage which happens to be in
>> queue memory and needs to be synchronized via some command channel. But
>> I can't be bothered to search for it as it's a no-brainer to fix that
>> up.
>
> Thanks for this analysis and pointing the hidden crucial issues with the 
> implementation. These needs to be fixed.
>
> As per your suggestion, we can add Firmware interaction code in the
> irq_bus_sync_xx APIs. Another option is to change the
> cdx_mcdi_rpc_async() API to atomic synchronous API.

I'm not a great fan of that. Depending on how long this update takes the
CPU will busy wait for it to complete with interrupts disabled and locks
held.

> We are evaluating both the solutions and will update the
> implementation accordingly.

Thanks,
 
         tglx