linux-kernel - Re: [PATCH 2/12] pci: add pci_bridge_release_unused_res and pci_bus_release_unused_bridge

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: <4B2C2A63.9000408@kernel.org>
Date:	Fri, 18 Dec 2009 17:20:35 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Ingo Molnar <mingo@...e.hu>,
	Ivan Kokshaysky <ink@...assic.park.msu.ru>,
	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>,
	Alex Chiang <achiang@...com>,
	Bjorn Helgaas <bjorn.helgaas@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Subject: Re: [PATCH 2/12] pci: add pci_bridge_release_unused_res and pci_bus_release_unused_bridge_res
 -v2

Linus Torvalds wrote:
> 
> On Fri, 18 Dec 2009, Yinghai Lu wrote:
>> ok, please check attached is right or not
>>  
>> +static void __release_child_resources(struct resource *r)
>> +{
>> +	struct resource *tmp, *p;
>> +	resource_size_t size;
>> +
>> +	p = r->child;
>> +	r->child = NULL;
>> +	while (p) {
>> +		tmp = p;
>> +		p = p->sibling;
>> +
>> +		tmp->parent = NULL;
>> +		tmp->sibling = NULL;
>> +		__release_child_resources(tmp);
>> +
>> +		printk(KERN_DEBUG "release child resource %pR\n", tmp);
>> +		/* need to restore size, and keep flags */
>> +		size = resource_size(tmp);
>> +		tmp->start = 0;
>> +		tmp->end = size - 1;
>> +	}
>> +}
> 
> Ok, this looks mostly right. I do worry about the alignment information: 
> you lose that thing for any resource that had IORESOURCE_STARTALIGN set 
> when you do this thing. That's pretty fundamental to the whole resource 
> code, I suspect we should just finally add a 'alignment' field to the 
> resource struct, so that alignment doesn't get lost when a resource is 
> allocated.
> 
> (Do a "git grep IORESOURCE_.*ALIGN" to see the kind of stuff I'm talking 
> about, and look at he PCI 'setup-bus.c' code that sets that STARTALIGN 
> thing).
> 
> So a preliminary ack on the resource.c parts. The rest I'm still a bit 
> dubious about, and the whole "we've lost alignment on the resources" is 
> probably indicative of how none of the resource code has ever really been 
> designed for this kind of "tear down and build back up again" behavior.

arch/powerpc/kernel/pci_of_scan.c:              flags |= IORESOURCE_SIZEALIGN;
drivers/pci/probe.c:            res->flags |= pci_calc_resource_flags(l) | IORESOURCE_SIZEALIGN;
drivers/pci/probe.c:                            IORESOURCE_SIZEALIGN;
drivers/pci/setup-bus.c:        b_res->flags |= IORESOURCE_STARTALIGN;
drivers/pci/setup-bus.c:        b_res->flags |= IORESOURCE_STARTALIGN;
drivers/pci/setup-bus.c:        b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
drivers/pci/setup-bus.c:        b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
drivers/pci/setup-bus.c:                b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
drivers/pci/setup-bus.c:                b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
drivers/pci/setup-bus.c:                b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
drivers/pci/setup-res.c:                res->flags &= ~IORESOURCE_STARTALIGN;
drivers/staging/b3dfg/b3dfg.c:                          != (IORESOURCE_MEM | IORESOURCE_SIZEALIGN)) {
include/linux/ioport.h:#define IORESOURCE_SIZEALIGN     0x00020000      /* size indicates alignment */
include/linux/ioport.h:#define IORESOURCE_STARTALIGN    0x00040000      /* start field is alignment */
kernel/resource.c:      switch (res->flags & (IORESOURCE_SIZEALIGN | IORESOURCE_STARTALIGN)) {
kernel/resource.c:      case IORESOURCE_SIZEALIGN:
kernel/resource.c:      case IORESOURCE_STARTALIGN:

looks like IORESOURCE_SIZEALIGN is only used by bridge.

and next round. pbus_size_mem will add that back again.

YH

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