linux-kernel - Re: [PATCH 1/2] PCI: Add support for VF Resizable Bar extended cap

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: <YbqGplTKl5i/1/kY@rocinante>
Date:   Thu, 16 Dec 2021 01:21:58 +0100
From:   Krzysztof Wilczyński <kw@...ux.com>
To:     Michał Winiarski <michal.winiarski@...el.com>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Christian König <christian.koenig@....com>,
        Ard Biesheuvel <ardb@...nel.org>,
        "Michael J . Ruhl" <michael.j.ruhl@...el.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>
Subject: Re: [PATCH 1/2] PCI: Add support for VF Resizable Bar extended cap

Hi Michał,

[...]
> +static int pci_memory_decoding(struct pci_dev *dev)
> +{
> +	u16 cmd;
> +
> +	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		return -EBUSY;
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PCI_IOV
> +static int pci_vf_memory_decoding(struct pci_dev *dev)
> +{
> +	u16 cmd;
> +
> +	pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd);
> +	if (cmd & PCI_SRIOV_CTRL_MSE)
> +		return -EBUSY;
> +
> +	return 0;
> +}
> +#endif
> +
>  int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  {
>  	struct resource *res = dev->resource + resno;
>  	struct pci_host_bridge *host;
>  	int old, ret;
>  	u32 sizes;
> -	u16 cmd;
>  
>  	/* Check if we must preserve the firmware's resource assignment */
>  	host = pci_find_host_bridge(dev->bus);
> @@ -424,9 +447,14 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
>  	if (!(res->flags & IORESOURCE_UNSET))
>  		return -EBUSY;
>  
> -	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> -	if (cmd & PCI_COMMAND_MEMORY)
> -		return -EBUSY;
> +#ifdef CONFIG_PCI_IOV
> +	if (resno >= PCI_IOV_RESOURCES)
> +		ret = pci_vf_memory_decoding(dev);
> +	else
> +#endif
> +	ret = pci_memory_decoding(dev);
> +	if (ret)
> +		return ret;

The above else works, however, it does trip our static analysis tools, and
lack of the indentation makes it slightly confusing to read, at least at
the first glance.  Thus, I wonder whether it would be possible to combine
the pci_vf_memory_decoding() and pci_memory_decoding() somehow neatly into
a private helper that takes a boolean to indicate whether it's a VF or not.
Then, we could drop the if-statement, since some of the logic could live
within the helper.

What do you think?

	Krzysztof