linux-kernel - Re: [PATCH v8 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports

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]

Date:   Sat, 25 Jul 2020 13:35:43 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        linux-pci <linux-pci@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Raj Ashok <ashok.raj@...el.com>
Subject: Re: [PATCH v8 2/5] ACPI/PCI: Ignore _OSC negotiation result if
 pcie_ports_native is set.

On Sat, Jul 25, 2020 at 7:01 AM
<sathyanarayanan.kuppuswamy@...ux.intel.com> wrote:
>
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
>
> pcie_ports_native is set only if user requests native handling

the user

> of PCIe capabilities via pcie_port_setup command line option.
> User input takes precedence over _OSC based control negotiation
> result. So consider the _OSC negotiated result only if
> pcie_ports_native is unset.
>
> Also, since struct pci_host_bridge ->native_* members caches the
> ownership status of various PCIe capabilities, use them instead
> of distributed checks for pcie_ports_native.

...

> +static char *get_osc_desc(u32 bit)
> +{

> +       int i = 0;

Unneeded assignment.

> +       for (i = 0; i < ARRAY_SIZE(pci_osc_control_bit); i++)
> +               if (bit == pci_osc_control_bit[i].bit)
> +                       return pci_osc_control_bit[i].desc;
> +
> +       return NULL;
> +}

...

>         host_bridge = to_pci_host_bridge(bus->bridge);
> -       if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
> -               host_bridge->native_pcie_hotplug = 0;
> +       if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) {
> +               if (!pcie_ports_native)
> +                       host_bridge->native_pcie_hotplug = 0;
> +               else

> +                       dev_warn(&bus->dev, "OS overrides %s firmware control",

pci_warn() ?

> +                       get_osc_desc(OSC_PCI_EXPRESS_NATIVE_HP_CONTROL));
> +       }
> +

>         if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
>                 host_bridge->native_shpc_hotplug = 0;

Group them in the same way you did in the previous patch.

> -       if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL))
> -               host_bridge->native_aer = 0;
> -       if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL))
> -               host_bridge->native_pme = 0;
> -       if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL))
> -               host_bridge->native_ltr = 0;
> -       if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
> -               host_bridge->native_dpc = 0;
> +
> +       if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) {

> +               if (!pcie_ports_native)
> +                       host_bridge->native_aer = 0;
> +               else
> +                       dev_warn(&bus->dev, "OS overrides %s firmware control",
> +                       get_osc_desc(OSC_PCI_EXPRESS_AER_CONTROL));

Looks like a lot of duplication here. Perhaps split out a helper ?

> +       }

-- 
With Best Regards,
Andy Shevchenko