linux-kernel - Re: [PATCH] drivers: pci: Clear ACS state at kexec

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: <724d80ee-3a81-23bc-74b0-4b786b3ace53@deltatee.com>
Date:   Mon, 6 Jan 2020 09:51:52 -0700
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Deepa Dinamani <deepa.kernel@...il.com>, bhelgaas@...gle.com
Cc:     mika.westerberg@...ux.intel.com, alex.williamson@...hat.com,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers: pci: Clear ACS state at kexec

On 2020-01-04 3:51 p.m., Deepa Dinamani wrote:
> ACS bits remain sticky through kexec reset. This is not really a
> problem for Linux because turning IOMMU on assumes ACS on. But,
> this becomes a problem if we kexec into something other than
> Linux and that does not turn ACS on always.
> 
> Reset the ACS bits to default before kexec or device remove.

Hmm, I'm slightly hesitant about disabling ACS on a device's unbind...
Not sure if that's going to open up a hole on us.

> +
> +/* Standard PCI ACS capailities
> + * Source Validation | P2P Request Redirect | P2P Completion Redirect | Upstream Forwarding
> + */
> +#define PCI_STD_ACS_CAP (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
> +
>  /**
> - * pci_std_enable_acs - enable ACS on devices using standard ACS capabilities
> + * pci_std_enable_disable_acs - enable/disable ACS on devices using standard
> + * ACS capabilities
>   * @dev: the PCI device
>   */
> -static void pci_std_enable_acs(struct pci_dev *dev)
> +static void pci_std_enable_disable_acs(struct pci_dev *dev, int enable)
>  {
>  	int pos;
>  	u16 cap;
>  	u16 ctrl;
> +	u16 val = 0;
>  
>  	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
>  	if (!pos)
> @@ -3278,19 +3286,26 @@ static void pci_std_enable_acs(struct pci_dev *dev)
>  	pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
>  	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
>  
> -	/* Source Validation */
> -	ctrl |= (cap & PCI_ACS_SV);
> +	val = (cap & PCI_STD_ACS_CAP);

Can we open code PCI_STD_ACS_CAP? I don't see any value in it being
defined above the function, it just makes the code harder to read.

Logan