linux-kernel - Re: [PATCH v3] PCI: loongson: skip scanning unavailable child device

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]

Date:   Tue, 8 Nov 2022 11:57:49 +0800
From:   Liu Peibao <liupeibao@...ngson.cn>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Huacai Chen <chenhuacai@...ngson.cn>,
        Jianmin Lv <lvjianmin@...ngson.cn>,
        Yinbo Zhu <zhuyinbo@...ngson.cn>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] PCI: loongson: skip scanning unavailable child device

On 11/8/22 5:15 AM, Bjorn Helgaas wrote:
> Capitalize subject line to match other commits:
> 
>   930c6074d7dd ("PCI: loongson: Work around LS7A incorrect Interrupt Pin registers")
>   2410e3301fcc ("PCI: loongson: Don't access non-existent devices")
>   cd89edda4002 ("PCI: loongson: Add ACPI init support")
>   dee449aafd48 ("PCI: loongson: Use generic 8/16/32-bit config ops on LS2K/LS7A")
> 

OK, I will do this in the next version patch.

> On Fri, Nov 04, 2022 at 06:53:40PM +0800, Liu Peibao wrote:
>> The PCI Controller of 2k1000 could not mask devices by
>> setting vender id or device id in configuration space header
>> as invalid values. When there are pins shareble between
>> the platform device and PCI device, if the platform device
>> is preferred, we should not scan this PCI device. In the
>> above scene, add `status = "disabled"` property in DT node
>> of this PCI device.
> 
> Rewrap this to fill 75 columns.
> > s/id/ID/
> s/shareble/shareable/
> 

OK, I will take care in the next version patch.

>> Signed-off-by: Liu Peibao <liupeibao@...ngson.cn>
>> ---
>> V2 -> V3: 1. use list_for_each_entry() for more clearly.
>>           2. fix wrong use of sizeof().
>> V1 -> V2: use existing property "status" instead of adding new property.
>>
>>
>>  drivers/pci/controller/pci-loongson.c | 55 +++++++++++++++++++++++++++
>>  1 file changed, 55 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
>> index 05c50408f13b..c7dd88eac885 100644
>> --- a/drivers/pci/controller/pci-loongson.c
>> +++ b/drivers/pci/controller/pci-loongson.c
>> @@ -40,11 +40,21 @@ struct loongson_pci_data {
>>  	struct pci_ops *ops;
>>  };
>>  
>> +#ifdef CONFIG_OF
>> +struct mask_entry {
>> +	struct list_head entry;
>> +	unsigned int devfn;
>> +};
>> +#endif
>> +
>>  struct loongson_pci {
>>  	void __iomem *cfg0_base;
>>  	void __iomem *cfg1_base;
>>  	struct platform_device *pdev;
>>  	const struct loongson_pci_data *data;
>> +#ifdef CONFIG_OF
>> +	struct list_head masklist;
>> +#endif
>>  };
>>  
>>  /* Fixup wrong class code in PCIe bridges */
>> @@ -194,6 +204,18 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus,
>>  			return NULL;
>>  	}
>>  
>> +#ifdef CONFIG_OF
>> +	/* Don't access devices in masklist */
>> +	if (pci_is_root_bus(bus)) {
>> +		struct mask_entry *entry;
>> +
>> +		list_for_each_entry(entry, &priv->masklist, entry) {
>> +			if (devfn == entry->devfn)
>> +				return NULL;
>> +		}
>> +	}
>> +#endif
> 
> I would probably get rid of the masklist and just search for a disabled
> property when reading config offset 0 (vendor ID).  That's not a
> performance path anyway.  And this seems similar to the
> FLAG_DEV_HIDDEN path where you probably don't need to do it for all
> controllers.
> 

Thanks for your idea! This really helps a lot!
I will follow your idea and rework it.

Yes, this is similar to the FLAG_DEV_HIDDEN path and currently, this path
is only needed by 2K1000 PCI controller.

BR,
Peibao