linux-kernel - Re: [PATCH 3/5] PCI: add functionality for resizing resources v4

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: <CAHp75Vch0ytjeSqDRDtYcMymLgDF5b5sR5W+8kdAnMSFYJgRsg@mail.gmail.com>
Date:   Sun, 7 May 2017 13:58:19 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Christian König <deathsimple@...afone.de>
Cc:     "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Bjorn Helgaas <helgaas@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/5] PCI: add functionality for resizing resources v4

On Thu, May 4, 2017 at 12:31 PM, Christian König
<deathsimple@...afone.de> wrote:
> From: Christian König <christian.koenig@....com>
>
> This allows device drivers to request resizing their BARs.
>
> The function only tries to reprogram the windows of the bridge directly above
> the requesting device and only the BAR of the same type (usually mem, 64bit,
> prefetchable). This is done to make sure not to disturb other drivers by
> changing the BARs of their devices.
>
> If reprogramming the bridge BAR fails the old status is restored and -ENOSPC
> returned to the calling device driver.

My comments below.

> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> +{
> +       struct pci_dev_resource *dev_res;
> +       LIST_HEAD(saved);
> +       LIST_HEAD(added);
> +       LIST_HEAD(failed);
> +       unsigned int i;
> +       int ret;
> +
> +       /* Walk to the root BUS, releasing bridge BARs when possible */
> +       while (1) {
> +               for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
> +                    i++) {
> +                       struct resource *res = &bridge->resource[i];
> +
> +                       if ((res->flags ^ type) & PCI_RES_TYPE_MASK)
> +                               continue;
> +
> +                       /* Ignore BARs which are still in use */
> +                       if (res->child)
> +                               continue;
> +
> +                       ret = add_to_list(&saved, bridge, res, 0, 0);
> +                       if (ret)
> +                               goto cleanup;
> +
> +                       dev_info(&bridge->dev, "BAR %d: releasing %pR\n",
> +                                i, res);
> +
> +                       if (res->parent)
> +                               release_resource(res);
> +                       res->start = 0;
> +                       res->end = 0;
> +                       break;
> +               }
> +               if (i == PCI_BRIDGE_RESOURCE_END)
> +                       break;
> +
> +               if (!bridge->bus || !bridge->bus->self)
> +                       break;
> +
> +               bridge = bridge->bus->self;
> +       }
> +

Please, consider to refactor as I proposed the other day.

> +}

> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +{
> +       struct resource *res = dev->resource + resno;

> +       int old, ret = 0;

Redundant assignment.

> +       u32 sizes;
> +       u16 cmd;
> +

> +       ret = pci_rbar_set_size(dev, resno, size);
> +       if (ret)
> +               goto error_reassign;
> +

> +       return 0;
> +
> +error_resize:
> +       pci_rbar_set_size(dev, resno, old);
> +       res->end = res->start + pci_rbar_size_to_bytes(old) - 1;
> +
> +error_reassign:
> +       pci_assign_unassigned_bus_resources(dev->bus);
> +       return ret;
> +}

-- 
With Best Regards,
Andy Shevchenko