linux-kernel - Re: [Query] ACS enablement in the DT based boot flow

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: <f320e311-89ba-4a92-a231-1298ecc94c0b@quicinc.com>
Date: Wed, 6 Nov 2024 11:43:30 +0530
From: Pavan Kondeti <quic_pkondeti@...cinc.com>
To: Pavan Kondeti <quic_pkondeti@...cinc.com>
CC: Will Deacon <will@...nel.org>, Vidya Sagar <vidyas@...dia.com>,
        "Bjorn
 Helgaas" <helgaas@...nel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Lorenzo Pieralisi
	<lpieralisi@...nel.org>,
        "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Manikanta Maddireddy
	<mmaddireddy@...dia.com>,
        Shanker Donthineni <sdonthineni@...dia.com>,
        Krishna Thota <kthota@...dia.com>, Joerg Roedel <joro@...tes.org>
Subject: Re: [Query] ACS enablement in the DT based boot flow

Hi Will,

On Thu, Jul 18, 2024 at 03:43:18PM +0530, Pavan Kondeti wrote:
> > > The pci_request_acs() in of_iommu_configure(), which happens too late
> > > to affect pci_enable_acs(), was added by 6bf6c24720d3 ("iommu/of:
> > > Request ACS from the PCI core when configuring IOMMU linkage"), so I
> > > cc'd Will and Joerg.  I don't know if that *used* to work and got
> > > broken somehow, or if it never worked as intended.
> > 
> > I don't have any way to test this, but I'm supportive of having the same
> > flow for DT and ACPI-based flows. Vidya, are you able to cook a patch?
> > 
> 
> I ran into a similar observation while testing a PCI device assignment
> to a VM. In my configuration, the virtio-iommu is enumerated over the
> PCI transport. So, I am thinking we can't hook pci_request_acs() to an
> IOMMU driver. Does the below patch makes sense?
> 
> The patch is tested with a VM and I could see ACS getting enabled and
> separate IOMMU groups are created for the devices attached under
> PCIe root port(s).
> 
> The RC/devices with ACS quirks are not suffering from this problem as we 
> short circuit ACS capability detection checking in
> pci_acs_enabled()->pci_dev_specific_acs_enabled() . May be this is one
> of the reason why this was not reported/observed by some platforms with
> DT.
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index b908fe1ae951..0eeb7abfbcfa 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -123,6 +123,13 @@ bool pci_host_of_has_msi_map(struct device *dev)
>  	return false;
>  }
>  
> +bool pci_host_of_has_iommu_map(struct device *dev)
> +{
> +	if (dev && dev->of_node)
> +		return of_get_property(dev->of_node, "iommu-map", NULL);
> +	return false;
> +}
> +
>  static inline int __of_pci_pci_compare(struct device_node *node,
>  				       unsigned int data)
>  {
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4c367f13acdc..ea6fcdaf63e2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -889,6 +889,7 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
>  	dev_set_msi_domain(&bus->dev, d);
>  }
>  
> +bool pci_host_of_has_iommu(struct device *dev);
>  static int pci_register_host_bridge(struct pci_host_bridge *bridge)
>  {
>  	struct device *parent = bridge->dev.parent;
> @@ -951,6 +952,9 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
>  	    !pci_host_of_has_msi_map(parent))
>  		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>  
> +	if (pci_host_of_has_iommu_map(parent))
> +		pci_request_acs();
> +
>  	if (!parent)
>  		set_dev_node(bus->bridge, pcibus_to_node(bus));
>  

I see that this problem is reproducible with the kernel tip. While preparing
patch submission, I found there was an attempt to fix [1] this problem
earlier but later reverted due to issues reported on linux-next. I did
not see any follow up on the issues. I would like to resend this patch
again as it was acked by people.

Thanks,
Pavan

[1]
https://lore.kernel.org/all/1621566204-37456-1-git-send-email-wangxingang5@huawei.com/