linux-kernel - Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root

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, 18 Nov 2014 16:32:26 +0800
From:	Yijing Wang <wangyijing@...wei.com>
To:	Arnd Bergmann <arnd@...db.de>, <linuxppc-dev@...ts.ozlabs.org>
CC:	Bjorn Helgaas <bhelgaas@...gle.com>,
	Liviu Dudau <liviu@...au.co.uk>,
	Tony Luck <tony.luck@...el.com>,
	Russell King <linux@....linux.org.uk>,
	<linux-pci@...r.kernel.org>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>, Xinwei Hu <huxinwei@...wei.com>,
	"Thierry Reding" <thierry.reding@...il.com>,
	<Suravee.Suthikulpanit@....com>, <linux-ia64@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Wuyun <wuyun.wu@...wei.com>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out
 of pci_create_root_bus()


>> +LIST_HEAD(pci_host_bridge_list);
>> +DECLARE_RWSEM(pci_host_bridge_sem);
> 
> Unless the pci_host_bridge_sem is accessed thousands of times per second,
> it's normally better to use a simple mutex instead.

OK, I will use simple mutex instead.

> 
>> +static struct resource busn_resource = {
>> +	.name	= "PCI busn",
>> +	.start	= 0,
>> +	.end	= 255,
>> +	.flags	= IORESOURCE_BUS,
>> +};
> 
> I think it would be better to require callers to pass the bus resource
> down to the function.

Hmm, I think most of caller will provide the bus resource, but some others
will not give any bus resource, extremely, no any resources :(. But we still
need properly configure their resources for compatibility.

> 
>> +struct pci_host_bridge *pci_create_host_bridge(
>> +		struct device *parent, u32 db, 
>> +		struct pci_ops *ops, void *sysdata, 
>> +		struct list_head *resources)
>> +{
> 
> Do we still need to pass the 'sysdata' in here? If we are guaranteed to
> have a device pointer, we should always be able to get the driver
> private data from dev_get_drvdata(host->dev->parent).

We need, some platforms pass NULL pointer as host bridge parent.

> 
>> +	host = kzalloc(sizeof(*host), GFP_KERNEL);
>> +	if (!host)
>> +		return NULL;
> 
> devm_kzalloc maybe?

I don't know much detail about devm_kzalloc(), but we have no pci host driver
here, and I found no devm_kzalloc() uses in core PCI code before.

> 
>> +	if (!resources) {
>> +		/* Use default IO/MEM/BUS resources*/
>> +		pci_add_resource(&host->windows, &ioport_resource);
>> +		pci_add_resource(&host->windows, &iomem_resource);
>> +		pci_add_resource(&host->windows, &busn_resource);
>> +	} else {
>> +		list_for_each_entry_safe(window, n, resources, list)
>> +			list_move_tail(&window->list, &host->windows);
>> +	}
> 
> I think we should assume that the correct resources are passed. You
> could add a wrapper around this function to convert old platforms
> though.

OK, I will move these code out of pci_create_host_bridge, and add a wrapper
to setup the default resources.

> 
>> +EXPORT_SYMBOL(pci_create_host_bridge);
> 
> EXPORT_SYMBOL_GPL() maybe?

OK, will update it.

> 
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 8b11b38..daa7f40 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -402,7 +402,12 @@ struct pci_host_bridge_window {
>>  struct pci_host_bridge {
>>  	struct device dev;
>>  	struct pci_bus *bus;		/* root bus */
>> +	struct list_head list;
>>  	struct list_head windows;	/* pci_host_bridge_windows */
>> +	int busnum;
> 
> The busnum should already be implied through the bus resource.

Yes, I will consider remove it and introduce a helper function to get the root bus number, thanks!

Thanks!
Yijing.

> 
> 	Arnd
> 
> .
> 


-- 
Thanks!
Yijing

--
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/