Re: [coreboot] patch: v3 dts support for multiple link devices such as k8 north

[Carl-Daniel Hailfinger]

On 16.09.2008 01:31, ron minnich wrote:
> attached.

Thanks.

That patch still has the NULL pointer dereference bug I pointed out
earlier. Use the attached serengeti dts to expose the bug.
> struct device dev_domain_0_pci0_18_0 = {
>    .path = {.type=DEVICE_PATH_PCI,{.pci={ .devfn = PCI_DEVFN(0x18, 0x0)}}},
>    .device_configuration = &domain_0_pci0_18_0,
>    .ops = &k8_ops,
>    .sibling = &dev_domain_0_pci1_18_0,
>    .link = {
>    	[0] = {
>    		.dev = &dev_domain_0_pci0_18_0,
>    		.link = 0,
>    		.children = &dev_domain_0_pci0_18_0_pci_0_0
>    	},
>    	[1] = {
>    		.dev = &dev_domain_0_pci1_18_0,
>    		.link = 0,

.link=1 would be correct.
> },
> [2] = {
> 	.dev = &dev_domain_0_pci2_18_0,
> 	.link = 0,

.link=2 would be correct.
> 	.children = &dev_domain_0_pci2_18_0_pci_2_0
> },
>
> },
>    .links = 3,
>    .bus = &dev_domain_0.link[0],
>    .next = &dev_domain_0_pci1_18_0,
>    .dtsname = "domain_0_pci0_18_0",
>    .enabled = 1
> };
> struct device dev_domain_0_pci2_18_0 = {
>    .path = {.type=DEVICE_PATH_PCI,{.pci={ .devfn = PCI_DEVFN(0x18, 0x0)}}},
>    .device_configuration = &domain_0_pci2_18_0,
>    .ops = &k8_ops,
>    .sibling = &dev_domain_0_ioport_2e,
>    .links = 0,
>    .bus = &dev_domain_0.link[0],
>    .next = &dev_domain_0_ioport_2e,
>    .dtsname = "domain_0_pci2_18_0",
>    .enabled = 1
> };
> struct device dev_domain_0_pci2_18_0_pci_2_0 = {
>    .path = {.type=DEVICE_PATH_PCI,{.pci={ .devfn = PCI_DEVFN(0x2, 0x0)}}},
>    .links = 0,
>    .bus = &dev_domain_0_pci2_18_0.link[0],

.bus is invalid here because dev_domain_0_pci2_18_0 has zero link
members and we use the first one here. If anyone ever accesses
dev_domain_0_pci2_18_0_pci_2_0.bus->dev he will get a less than nice
NULL pointer.
> .next = &dev_domain_0_pci2_18_0,
>    .dtsname = "domain_0_pci2_18_0_pci_2_0",
>    .enabled = 1
> };

Sorry, no Ack for now. I kind of dislike NULL pointer dereferences
during runtime.

There are various ways to code around that problem, one of them is to
make links explicit. Another one would be to kill the "helper
(pci1,pci2)" structs for the domain.

Regards,
Carl-Daniel