linux-kernel - Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks

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:   Tue, 13 Sep 2016 08:37:19 +0200
From:   Tomasz Nowicki <tn@...ihalf.com>
To:     Duc Dang <dhdang@....com>
Cc:     Bjorn Helgaas <helgaas@...nel.org>,
        Will Deacon <will.deacon@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Rafael Wysocki <rafael@...nel.org>,
        Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Hanjun Guo <hanjun.guo@...aro.org>,
        Sinan Kaya <okaya@...eaurora.org>,
        Jayachandran C <jchandra@...adcom.com>,
        Christopher Covington <cov@...eaurora.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Robert Richter <robert.richter@...iumnetworks.com>,
        Marcin Wojtas <mw@...ihalf.com>,
        Liviu Dudau <Liviu.Dudau@....com>,
        David Daney <ddaney@...iumnetworks.com>,
        Yijing Wang <wangyijing@...wei.com>,
        Mark Salter <msalter@...hat.com>, linux-pci@...r.kernel.org,
        linux-arm <linux-arm-kernel@...ts.infradead.org>,
        linaro-acpi@...ts.linaro.org, Jon Masters <jcm@...hat.com>,
        Andrea Gallo <andrea.gallo@...aro.org>, jeremy.linton@....com,
        Dongdong Liu <liudongdong3@...wei.com>,
        Gabriele Paoloni <gabriele.paoloni@...wei.com>,
        Jeffrey Hugo <jhugo@...eaurora.org>,
        linux-acpi@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks

On 13.09.2016 00:24, Duc Dang wrote:
> On Fri, Sep 9, 2016 at 12:24 PM, Tomasz Nowicki <tn@...ihalf.com> wrote:
>>
>> Some platforms may not be fully compliant with generic set of PCI config
>> accessors. For these cases we implement the way to overwrite CFG accessors
>> set and configuration space range.
>>
>> In first place pci_mcfg_parse() saves machine's IDs and revision number
>> (these come from MCFG header) in order to match against known quirk entries.
>> Then the algorithm traverses available quirk list (static array),
>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>> returns custom PCI config ops and/or CFG resource structure.
>>
>> When adding new quirk there are two possibilities:
>> 1. Override default pci_generic_ecam_ops ops but CFG resource comes from MCFG
>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &foo_ops, MCFG_RES_EMPTY },
>> 2. Override default pci_generic_ecam_ops ops and CFG resource. For this case
>> it is also allowed get CFG resource from quirk entry w/o having it in MCFG.
>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &boo_ops,
>>   DEFINE_RES_MEM(START, SIZE) },
>>
>> pci_generic_ecam_ops and MCFG entries will be used for platforms
>> free from quirks.
>>
>> Signed-off-by: Tomasz Nowicki <tn@...ihalf.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@...wei.com>
>> Signed-off-by: Christopher Covington <cov@...eaurora.org>
>> ---
>>  drivers/acpi/pci_mcfg.c | 80 +++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 74 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index ffcc651..2b8acc7 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -32,6 +32,59 @@ struct mcfg_entry {
>>         u8                      bus_start;
>>         u8                      bus_end;
>>  };
>> +struct mcfg_fixup {
>> +       char oem_id[ACPI_OEM_ID_SIZE + 1];
>> +       char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
>> +       u32 oem_revision;
>> +       u16 seg;
>> +       struct resource bus_range;
>> +       struct pci_ecam_ops *ops;
>> +       struct resource cfgres;
>> +};
>> +
>> +#define MCFG_DOM_ANY                   (-1)
>> +#define MCFG_BUS_RANGE(start, end)     DEFINE_RES_NAMED((start),       \
>> +                                               ((end) - (start) + 1),  \
>> +                                               NULL, IORESOURCE_BUS)
>> +#define MCFG_BUS_ANY           MCFG_BUS_RANGE(0x0, 0xff)
>> +#define MCFG_RES_EMPTY         DEFINE_RES_NAMED(0, 0, NULL, 0)
>> +
>> +static struct mcfg_fixup mcfg_quirks[] = {
>> +/*     { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */
>> +};
>> +
>> +static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> +static char mcfg_oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
>> +static u32 mcfg_oem_revision;
>> +
>> +static void pci_mcfg_match_quirks(struct acpi_pci_root *root,
>> +                                 struct resource *cfgres,
>> +                                 struct pci_ecam_ops **ecam_ops)
>> +{
>> +       struct mcfg_fixup *f;
>> +       int i;
>> +
>> +       /*
>> +        * First match against PCI topology <domain:bus> then use OEM ID, OEM
>> +        * table ID, and OEM revision from MCFG table standard header.
>> +        */
>> +       for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
>> +               if (f->seg == root->segment &&
>
> Is dropping the comparison with MCFG_DOM_ANY intended? It is useful if
> all the controllers (segs) can use the same quirk (X-Gene case).

MCFG_DOM_ANY makes sense only if we want to use the same ops for segment 
range, but not for CFG range manipulation. So we can add MCFG_DOM_ANY 
back here in the hope it will be used properly.

Thanks,
Tomasz