linux-kernel - Re: [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs

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] [day] [month] [year] [list]

Message-ID: <5015634E.5050007@gmail.com>
Date:	Mon, 30 Jul 2012 00:22:38 +0800
From:	Jiang Liu <liuj97@...il.com>
To:	Don Dutile <ddutile@...hat.com>
CC:	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jiang Liu <jiang.liu@...wei.com>,
	Yinghai Lu <yinghai@...nel.org>,
	Taku Izumi <izumi.taku@...fujitsu.com>,
	"Rafael J . Wysocki" <rjw@...k.pl>,
	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>,
	Yijing Wang <wangyijing@...wei.com>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions
 to hide differences among PCIe specs

On 07/25/2012 05:12 AM, Don Dutile wrote:
> On 07/24/2012 12:31 PM, Jiang Liu wrote:
>> +int pci_pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
>> +{
>> +    int ret = 0;
>> +
>> +    *val = 0;
>> +    if (pos&  1)
>> +        return -EINVAL;
>> +
>> +    if (pci_pcie_capability_reg_implemented(dev, pos)) {
>> +        ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
>> +        /*
>> +         * Reset *val to 0 if pci_read_config_word() fails, it may
>> +         * have been written as 0xFFFF if hardware error happens
>> +         * during pci_read_config_word().
>> +         */
>> +        if (ret)
>> +            *val = 0;
>> +    } else if (pos == PCI_EXP_SLTSTA&&
>> +         pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) {
>> +        *val = PCI_EXP_SLTSTA_PDS;
>> +    }
> Don't you want the above if check done 1st, and not the
> pci_pcie_capability_reg_implemented(dev, pos) check ?
> Isn't PCI_EXP_SLTCTL an implemented register, looking at this snippet?:
>> +    switch (pos) {
>         <snip>
>> +    case PCI_EXP_SLTCAP:
>> +    case PCI_EXP_SLTCTL:
>> +    case PCI_EXP_SLTSTA:
>> +        return pci_pcie_cap_has_sltctl(dev);
>   and the above function is:
> 
>> +static inline bool pci_pcie_cap_has_sltctl(const struct pci_dev *dev)
>> +{
>> +    int type = pci_pcie_type(dev);
>> +
>> +    return pci_pcie_cap_version(dev)>  1 ||
>> +           type == PCI_EXP_TYPE_ROOT_PORT ||
>> +           (type == PCI_EXP_TYPE_DOWNSTREAM&&
>> +        dev->pcie_flags_reg&  PCI_EXP_FLAGS_SLOT);
>> +}
> 
> or is PCI_EXP_FLAGS_SLOT not set when it should be, then the first condition
> fails, and this function forces the val to 1b ?
Hi Don,
	Yes, that's the purpose. PCIe spec v2/v3 defines that hardware should return
a value with bit PCI_EXP_SLTSTA_PDS set if PCI_EXP_SLTSTA register is not implemented.
So for PCIe v1 hardwares, we try to behave in the same way as v2/v3.
	Regards!
	Gerry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/