linux-kernel - Re: [PATCH v9 2/6] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87v7w0s9a8.ffs@tglx>
Date: Tue, 03 Dec 2024 23:15:27 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Frank Li <Frank.Li@....com>, Manivannan Sadhasivam
 <manivannan.sadhasivam@...aro.org>, Krzysztof Wilczyński
 <kw@...ux.com>,
 Kishon Vijay Abraham I <kishon@...nel.org>, Bjorn Helgaas
 <bhelgaas@...gle.com>, Arnd Bergmann <arnd@...db.de>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Anup Patel <apatel@...tanamicro.com>
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
 imx@...ts.linux.dev, Niklas Cassel <cassel@...nel.org>,
 dlemoal@...nel.org, maz@...nel.org, jdmason@...zu.us, Frank Li
 <Frank.Li@....com>
Subject: Re: [PATCH v9 2/6] PCI: endpoint: Add RC-to-EP doorbell support
 using platform MSI controller

On Tue, Dec 03 2024 at 15:36, Frank Li wrote:
> +static void pci_epc_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
> +{
> +	struct pci_epc *epc;
> +	struct pci_epf *epf;
> +
> +	epc = pci_epc_get(dev_name(msi_desc_to_dev(desc)));
> +	if (!epc)

This is wrong as pci_epc_get() never returns NULL on failure. It returns
an error pointer.

> +		return;
> +
> +	epf = list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list);

How can the list be empty?

> +	if (epf && epf->db_msg && desc->msi_index < epf->num_db)
> +		memcpy(&epf->db_msg[desc->msi_index].msg, msg, sizeof(*msg));

So now the message is copied out into that db_msg array which is
somewhere in the memory which was allocated on the EP side.

How is the host side supposed to know about the change of the message?

This only works reliably if:

  1) the message address/data pair is immutable once it is set up and
     subsequent affinity changes are not affecting it

  2) The ordering on the EP driver is:

     request_irq()
     publish_msg_to_host()
     tell_host_that_message_is_ready()

#2 is a documentation problem, but #1 needs some thought.

It only works for MSI parent domains which use a translation table and
affinity changes only happen at the translation table level, which means
the address/data pair is unaffected.

Sure GIC-ITS, AMD/Intel remap domains work that way, but what happens if
the underlying MSI parent domain actually changes the message
(address/data pair) during an affinity change?

These domains expect that the message is known to the other side at the
time when irq_set_affinity() returns. In case of regular PCI/MSI this is
not a problem because the message is written to the device before the
function returns, but in this EP case nothing guarantees that the
modified message is host visible at that point.

The fact that a MSI parent domain supports DOMAIN_BUS_DEVICE_MSI does
not guarantee that the parent is translation table based.

As this is intended to be a generic library for all sorts of EP
implementations, there needs to be

  - either a mechanism to prevent the initialization if the underlying
    MSI parent domain does not provide immutable messages

  - or support for endpoint specific msi_write_msg() implementations

Thanks,

        tglx