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]

Message-ID: <20141119162914.GB9162@bart.dudau.co.uk>
Date:	Wed, 19 Nov 2014 16:29:14 +0000
From:	Liviu Dudau <liviu@...au.co.uk>
To:	Yijing Wang <wangyijing@...wei.com>
Cc:	Liviu Dudau <Liviu.Dudau@....com>, Arnd Bergmann <arnd@...db.de>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Tony Luck <tony.luck@...el.com>,
	Russell King <linux@....linux.org.uk>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"huxinwei@...wei.com" <huxinwei@...wei.com>,
	Thierry Reding <thierry.reding@...il.com>,
	"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
	"linux-ia64@...r.kernel.org" <linux-ia64@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Wuyun <wuyun.wu@...wei.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of
 pci_create_root_bus()

On Wed, Nov 19, 2014 at 10:24:52AM +0800, Yijing Wang wrote:
> >> We need, some platforms pass NULL pointer as host bridge parent.
> > 
> > Yijing,
> > 
> > May I suggest a different approach here? Rather than having to pass an opaque
> > pointer that gets converted by the host bridge driver back to the private
> > structure, what about promoting a new style of usage, that is similar to the
> > way device drivers work? Lets try to promote the embedding of the generic
> > pci_host_bridge structure in the host bridge specific structure! Then we can
> > access the private data doing container_of().
> > 
> > Something like this:
> > 
> > struct pci_controller {
> > 	struct pci_host_bridge bridge;
> > 	/* private host bridge data here */
> > 	.....
> > };
> > 
> > #define PCI_CONTROLLER(bus)	({
> > 	struct pci_host_bridge *hb = to_pci_host_bridge(bus->bridge); \
> > 	container_of(hb, struct pci_controller, bridge); })
> > 
> > 
> > Then we can retrieve the host bridge structure from everywhere we have a device.
> 
> Hi Liviu, it looks good to me, because this change will involve lots platforms,
> I would think more about it. Thanks for your suggestion very much! :)

Given that I also look at this area maybe we should join forces and divide the problem?

Best regards,
Liviu

> 
> 
> Thanks!
> Yijing.
> 
> > 
> > Best regards,
> > Liviu
> > 
> >>
> >>>
> >>>> +	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-pci" in
> >> the body of a message to majordomo@...r.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> > 
> 
> 
> -- 
> 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/
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

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