I've been looking at v2 and v3 HT code. It seems like the biggest problem is that we changed the representation of the device tree and didn't change the code.
I think it would be best to have the dts match an lspci as closely as possible. That means the code has to be a little smarter so that it can fill in the missing information.
If we start with a dts which describes the Serengeti machine, we have:
/{ device_operations="serengeti"; mainboard_vendor = "AMD"; mainboard_name = "Serengeti"; cpus { }; apic@0 { }; domain@0 { /config/("northbridge/amd/k8/domain"); pci0@18,0 { /config/("northbridge/amd/k8/pci"); pci@0,0 { /config/("southbridge/amd/amd8111/pci.dts"); pci@1,0{ /config/("southbridge/amd/amd8111/nic.dts"); }; }; pci@1,0 { /config/("southbridge/amd/amd8111/lpc.dts"); }; pci@1,1 { /config/("southbridge/amd/amd8111/ide.dts"); }; pci@1,0{ /config/("southbridge/amd/amd8132/pcix.dts"); };
}; pci1@18,0 { /config/("northbridge/amd/k8/pci"); }; pci2@18,0 { /config/("northbridge/amd/k8/pci"); }; pci@18,1 {}; pci@18,2 {}; pci@18,3 {}; ioport@2e { /config/("superio/winbond/w83627hf/dts"); com1enable = "1"; }; }; };
It doesn't exactly match an lspci, because lspci doesn't care what link the devices are on. I think we have to keep that difference.
Then when the hypertransport enumeration code runs, the bridges appear to be siblings of their children. With v2 there was another layer of hierarchy so that the children of link 0 would just be the 8132 and the 8111.
So the new code should: 1. Scan the list of children for bridges 2. Assign the devices bus numbers and attach them correctly
Does that mean that we need to create dynamic bus devices or structures to hold that information? Probably not. We can just change pointer assignments.
I don't think we need to expand the dts to fix this, we just need to make the code match the structure.
Thanks, Myles
On Mon, Oct 27, 2008 at 7:48 AM, Myles Watson mylesgw@gmail.com wrote:
I don't think we need to expand the dts to fix this, we just need to make the code match the structure.
Do you have a rough idea of how this would look in the code? This is a good catch.
ron
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 8:57 AM To: Myles Watson Cc: Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 7:48 AM, Myles Watson mylesgw@gmail.com wrote:
I don't think we need to expand the dts to fix this, we just need to
make
the code match the structure.
Do you have a rough idea of how this would look in the code? This is a good catch.
Right now the code goes through the list of children and initializes them as bridges, without checking to see that they are. Then it enumerates them with the pci code.
I think we'd have to find the children that are bridges and then enumerate them with the list of children, so that we can pass parameters correctly. I don't think it would be too hard.
1. Find bridges and set them to not decode 2. Set first bridge to decode all device numbers (HT Unit IDs) 3. Do PCI scan on bridge with remaining non-bridge children 4. Find next bridge 5. Set to decode next device numbers 6. Goto 3
Thanks, Myles
Myles Watson wrote:
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 8:57 AM To: Myles Watson Cc: Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 7:48 AM, Myles Watson mylesgw@gmail.com wrote:
I don't think we need to expand the dts to fix this, we just need to
make
the code match the structure.
Do you have a rough idea of how this would look in the code? This is a good catch.
Right now the code goes through the list of children and initializes them as bridges, without checking to see that they are. Then it enumerates them with the pci code.
I think we'd have to find the children that are bridges and then enumerate them with the list of children, so that we can pass parameters correctly. I don't think it would be too hard.
- Find bridges and set them to not decode
- Set first bridge to decode all device numbers (HT Unit IDs)
- Do PCI scan on bridge with remaining non-bridge children
- Find next bridge
- Set to decode next device numbers
- Goto 3
I think that the dts is can/should to handle arbitrary bus numbering (since a bridge can be plugged into any slot making everything change). The numbers are not so important if as long as the tree has the same connections (bridgeA has device 1, 2, 3 and bridgeB has x, y, z).
I don't think that you need a bridge scan before the device scan. It can be done as one. The device tree just needs to note the appropriate decode (bus number) for the bridge and the devices under it. Also, If the bus is already numbered we we shouldn't change it. Just use what is set.
Marc
-----Original Message----- From: Marc Jones [mailto:Marc.Jones@amd.com] Sent: Monday, October 27, 2008 3:10 PM To: Myles Watson Cc: 'ron minnich'; 'Coreboot' Subject: Re: [coreboot] v3 HT
Myles Watson wrote:
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 8:57 AM To: Myles Watson Cc: Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 7:48 AM, Myles Watson mylesgw@gmail.com
wrote:
I don't think we need to expand the dts to fix this, we just need to
make
the code match the structure.
Do you have a rough idea of how this would look in the code? This is a good catch.
Right now the code goes through the list of children and initializes
them as
bridges, without checking to see that they are. Then it enumerates them with the pci code.
I think we'd have to find the children that are bridges and then
enumerate
them with the list of children, so that we can pass parameters
correctly. I
don't think it would be too hard.
- Find bridges and set them to not decode
- Set first bridge to decode all device numbers (HT Unit IDs)
- Do PCI scan on bridge with remaining non-bridge children
- Find next bridge
- Set to decode next device numbers
- Goto 3
I think that the dts is can/should to handle arbitrary bus numbering (since a bridge can be plugged into any slot making everything change). The numbers are not so important if as long as the tree has the same connections (bridgeA has device 1, 2, 3 and bridgeB has x, y, z).
I don't think that you need a bridge scan before the device scan. It can be done as one. The device tree just needs to note the appropriate decode (bus number) for the bridge and the devices under it. Also, If the bus is already numbered we we shouldn't change it. Just use what is set.
So the collapse existing enumeration needs to be taken out?
Thanks, Myles
Myles Watson wrote:
-----Original Message----- From: Marc Jones [mailto:Marc.Jones@amd.com] Sent: Monday, October 27, 2008 3:10 PM To: Myles Watson Cc: 'ron minnich'; 'Coreboot' Subject: Re: [coreboot] v3 HT
Myles Watson wrote:
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 8:57 AM To: Myles Watson Cc: Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 7:48 AM, Myles Watson mylesgw@gmail.com
wrote:
I don't think we need to expand the dts to fix this, we just need to
make
the code match the structure.
Do you have a rough idea of how this would look in the code? This is a good catch.
Right now the code goes through the list of children and initializes
them as
bridges, without checking to see that they are. Then it enumerates them with the pci code.
I think we'd have to find the children that are bridges and then
enumerate
them with the list of children, so that we can pass parameters
correctly. I
don't think it would be too hard.
- Find bridges and set them to not decode
- Set first bridge to decode all device numbers (HT Unit IDs)
- Do PCI scan on bridge with remaining non-bridge children
- Find next bridge
- Set to decode next device numbers
- Goto 3
I think that the dts is can/should to handle arbitrary bus numbering (since a bridge can be plugged into any slot making everything change). The numbers are not so important if as long as the tree has the same connections (bridgeA has device 1, 2, 3 and bridgeB has x, y, z).
I don't think that you need a bridge scan before the device scan. It can be done as one. The device tree just needs to note the appropriate decode (bus number) for the bridge and the devices under it. Also, If the bus is already numbered we we shouldn't change it. Just use what is set.
So the collapse existing enumeration needs to be taken out?
I didn't know that there was a collapse. That might not be good since a bridge could be "hard coded" to a location. That is ok if bus numbers are arbitrary. It they get colapsed that would be bad. I need to go understand this part of the dts/scan more.
Marc
-----Original Message----- From: Marc Jones [mailto:Marc.Jones@amd.com] Sent: Monday, October 27, 2008 3:26 PM To: Myles Watson Cc: 'ron minnich'; 'Coreboot' Subject: Re: [coreboot] v3 HT
Myles Watson wrote:
-----Original Message----- From: Marc Jones [mailto:Marc.Jones@amd.com] Sent: Monday, October 27, 2008 3:10 PM To: Myles Watson Cc: 'ron minnich'; 'Coreboot' Subject: Re: [coreboot] v3 HT
Myles Watson wrote:
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 8:57 AM To: Myles Watson Cc: Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 7:48 AM, Myles Watson mylesgw@gmail.com
wrote:
I don't think we need to expand the dts to fix this, we just need to
make
the code match the structure.
Do you have a rough idea of how this would look in the code? This is
a
good catch.
Right now the code goes through the list of children and initializes
them as
bridges, without checking to see that they are. Then it enumerates
them
with the pci code.
I think we'd have to find the children that are bridges and then
enumerate
them with the list of children, so that we can pass parameters
correctly. I
don't think it would be too hard.
- Find bridges and set them to not decode
- Set first bridge to decode all device numbers (HT Unit IDs)
- Do PCI scan on bridge with remaining non-bridge children
- Find next bridge
- Set to decode next device numbers
- Goto 3
I think that the dts is can/should to handle arbitrary bus numbering (since a bridge can be plugged into any slot making everything change). The numbers are not so important if as long as the tree has the same connections (bridgeA has device 1, 2, 3 and bridgeB has x, y, z).
I don't think that you need a bridge scan before the device scan. It
can
be done as one. The device tree just needs to note the appropriate decode (bus number) for the bridge and the devices under it. Also, If the bus is already numbered we we shouldn't change it. Just use what is set.
So the collapse existing enumeration needs to be taken out?
I didn't know that there was a collapse. That might not be good since a bridge could be "hard coded" to a location. That is ok if bus numbers are arbitrary. It they get colapsed that would be bad. I need to go understand this part of the dts/scan more.
It's not part of the dts. It's also in v2 ht_collapse_early_enumeration.
Thanks, Myles
Myles Watson wrote:
-----Original Message----- From: Marc Jones [mailto:Marc.Jones@amd.com] Sent: Monday, October 27, 2008 3:26 PM To: Myles Watson Cc: 'ron minnich'; 'Coreboot' Subject: Re: [coreboot] v3 HT
Myles Watson wrote:
-----Original Message----- From: Marc Jones [mailto:Marc.Jones@amd.com] Sent: Monday, October 27, 2008 3:10 PM To: Myles Watson Cc: 'ron minnich'; 'Coreboot' Subject: Re: [coreboot] v3 HT
Myles Watson wrote:
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 8:57 AM To: Myles Watson Cc: Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 7:48 AM, Myles Watson mylesgw@gmail.com
wrote:
> I don't think we need to expand the dts to fix this, we just need to make > the code match the structure. Do you have a rough idea of how this would look in the code? This is
a
good catch.
Right now the code goes through the list of children and initializes
them as
bridges, without checking to see that they are. Then it enumerates
them
with the pci code.
I think we'd have to find the children that are bridges and then
enumerate
them with the list of children, so that we can pass parameters
correctly. I
don't think it would be too hard.
- Find bridges and set them to not decode
- Set first bridge to decode all device numbers (HT Unit IDs)
- Do PCI scan on bridge with remaining non-bridge children
- Find next bridge
- Set to decode next device numbers
- Goto 3
I think that the dts is can/should to handle arbitrary bus numbering (since a bridge can be plugged into any slot making everything change). The numbers are not so important if as long as the tree has the same connections (bridgeA has device 1, 2, 3 and bridgeB has x, y, z).
I don't think that you need a bridge scan before the device scan. It
can
be done as one. The device tree just needs to note the appropriate decode (bus number) for the bridge and the devices under it. Also, If the bus is already numbered we we shouldn't change it. Just use what is set.
So the collapse existing enumeration needs to be taken out?
I didn't know that there was a collapse. That might not be good since a bridge could be "hard coded" to a location. That is ok if bus numbers are arbitrary. It they get colapsed that would be bad. I need to go understand this part of the dts/scan more.
It's not part of the dts. It's also in v2 ht_collapse_early_enumeration.
Oh, It is fine for the HT code to do a collapse before dts/pci enumeration. The dts should assume that its correct. The dts/device code should be able to handle any arbitrary bus numbering that was set prior to it running.
Marc
also, do lspci under v2 and v3 really look different?
ron
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 8:58 AM To: Myles Watson Cc: Coreboot Subject: Re: [coreboot] v3 HT
also, do lspci under v2 and v3 really look different?
Right now the best I can do is dynamic detection of all the devices. That looks the same, but it doesn't reflect anything I'm doing in the dts.
Thanks, Myles
Myles Watson wrote:
I've been looking at v2 and v3 HT code. It seems like the biggest problem is that we changed the representation of the device tree and didn't change the code.
I think it would be best to have the dts match an lspci as closely as possible. That means the code has to be a little smarter so that it can fill in the missing information.
If we start with a dts which describes the Serengeti machine, we have:
/{ device_operations="serengeti"; mainboard_vendor = "AMD"; mainboard_name = "Serengeti"; cpus { }; apic@0 { }; domain@0 { /config/("northbridge/amd/k8/domain"); pci0@18,0 { /config/("northbridge/amd/k8/pci"); pci@0,0 { /config/("southbridge/amd/amd8111/pci.dts"); pci@1,0{
/config/("southbridge/amd/amd8111/nic.dts"); }; }; pci@1,0 { /config/("southbridge/amd/amd8111/lpc.dts"); }; pci@1,1 { /config/("southbridge/amd/amd8111/ide.dts"); }; pci@1,0{
/config/("southbridge/amd/amd8132/pcix.dts"); };
}; pci1@18,0 { /config/("northbridge/amd/k8/pci"); }; pci2@18,0 { /config/("northbridge/amd/k8/pci"); }; pci@18,1 {}; pci@18,2 {}; pci@18,3 {}; ioport@2e { /config/("superio/winbond/w83627hf/dts"); com1enable = "1"; };}; };
It doesn't exactly match an lspci, because lspci doesn't care what link the devices are on. I think we have to keep that difference.
The dts is wrong here. Like you said, it shouldn't care about the link. They are not bridges. Devices like the 8111 are the bridge.
Then when the hypertransport enumeration code runs, the bridges appear to be siblings of their children. With v2 there was another layer of hierarchy so that the children of link 0 would just be the 8132 and the 8111.
So the new code should:
- Scan the list of children for bridges
- Assign the devices bus numbers and attach them correctly
I'm not sure what you intend. I think that the bus numbers can be set as they are found in the depth first scan. I don't think you have to do a bridge only scan.
Does that mean that we need to create dynamic bus devices or structures to hold that information? Probably not. We can just change pointer assignments.
Each bridge device holds its assigned number and decodes config space accesses for devices below it. The dts pointers need to be connected in the same way. (I think that is what you were saying)
I don't think we need to expand the dts to fix this, we just need to make the code match the structure.
I think so too.
Marc
-----Original Message----- From: Marc Jones [mailto:Marc.Jones@amd.com] Sent: Monday, October 27, 2008 2:58 PM To: Myles Watson Cc: 'Coreboot' Subject: Re: [coreboot] v3 HT
Myles Watson wrote:
I've been looking at v2 and v3 HT code. It seems like the biggest
problem
is that we changed the representation of the device tree and didn't
change
the code.
I think it would be best to have the dts match an lspci as closely as possible. That means the code has to be a little smarter so that it can fill in the missing information.
If we start with a dts which describes the Serengeti machine, we have:
/{ device_operations="serengeti"; mainboard_vendor = "AMD"; mainboard_name = "Serengeti"; cpus { }; apic@0 { }; domain@0 { /config/("northbridge/amd/k8/domain"); pci0@18,0 { /config/("northbridge/amd/k8/pci"); pci@0,0 { /config/("southbridge/amd/amd8111/pci.dts"); pci@1,0{
/config/("southbridge/amd/amd8111/nic.dts"); }; }; pci@1,0 { /config/("southbridge/amd/amd8111/lpc.dts"); }; pci@1,1 { /config/("southbridge/amd/amd8111/ide.dts"); }; pci@1,0{
/config/("southbridge/amd/amd8132/pcix.dts");
};
}; pci1@18,0 { /config/("northbridge/amd/k8/pci"); }; pci2@18,0 { /config/("northbridge/amd/k8/pci"); }; pci@18,1 {}; pci@18,2 {}; pci@18,3 {}; ioport@2e { /config/("superio/winbond/w83627hf/dts"); com1enable = "1"; };}; };
It doesn't exactly match an lspci, because lspci doesn't care what link
the
devices are on. I think we have to keep that difference.
The dts is wrong here. Like you said, it shouldn't care about the link. They are not bridges. Devices like the 8111 are the bridge.
Can we agree on a simplified dts to start with, then? It would help me out a lot.
Then when the hypertransport enumeration code runs, the bridges appear
to be
siblings of their children. With v2 there was another layer of
hierarchy so
that the children of link 0 would just be the 8132 and the 8111.
So the new code should:
- Scan the list of children for bridges
- Assign the devices bus numbers and attach them correctly
I'm not sure what you intend. I think that the bus numbers can be set as they are found in the depth first scan. I don't think you have to do a bridge only scan.
This is where it gets ugly.
As far as I can tell, the 8111 is not visible until after the 8132 gets initialized enough to not mask the 8111's unitid. This is the reason for all of the #defines from v2.
The code doesn't seem to be organized in a depth-first manner. It would be nice, but then wouldn't you have to move the unitids so that the southbridge could stay where it was?
I don't understand enough to know why the 8111 needs to be at 6. I'm assuming that it is wired to be there. It doesn't look like the same is true for the 8132.
As a side note: The 8132 doesn't have the "bridge" bit set (maybe an oversight, maybe not.) It's not bridging to the 8111, but it is a bridge.
Does that mean that we need to create dynamic bus devices or structures
to
hold that information? Probably not. We can just change pointer assignments.
Each bridge device holds its assigned number and decodes config space accesses for devices below it. The dts pointers need to be connected in the same way. (I think that is what you were saying)
Yes. The problem is that the code isn't set up to take the depth-first information and propagate it.
Thanks, Myles
On Mon, Oct 27, 2008 at 2:11 PM, Myles Watson mylesgw@gmail.com wrote:
Yes. The problem is that the code isn't set up to take the depth-first information and propagate it.
I'll try to look at it again, but what information do you see it not propagating?
ron
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 3:17 PM To: Myles Watson Cc: Marc Jones; Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 2:11 PM, Myles Watson mylesgw@gmail.com wrote:
Yes. The problem is that the code isn't set up to take the depth-first information and propagate it.
I'll try to look at it again, but what information do you see it not propagating?
There is no real propagation. It just uses the #defines to put things in their places. I think it makes it fragile.
Have you applied my patch to add the 8132 so you can see it working/breaking?
Thanks, Myles
On Mon, Oct 27, 2008 at 2:37 PM, Myles Watson mylesgw@gmail.com wrote:
Have you applied my patch to add the 8132 so you can see it working/breaking?
can you give me a diff against current tree?
thanks
ron
On Mon, Oct 27, 2008 at 3:43 PM, ron minnich rminnich@gmail.com wrote:
On Mon, Oct 27, 2008 at 2:37 PM, Myles Watson mylesgw@gmail.com wrote:
Have you applied my patch to add the 8132 so you can see it working/breaking?
can you give me a diff against current tree?
thanks
ron
Here it is:
Signed-off-by: Myles Watson mylesgw@gmail.com
I'm sorry it doesn't build but it's close and I have to go.
Thanks, Myles
Sorry. Here's the full patch. This builds.
Signed-off-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
On Mon, Oct 27, 2008 at 4:09 PM, Myles Watson mylesgw@gmail.com wrote:
On Mon, Oct 27, 2008 at 3:43 PM, ron minnich rminnich@gmail.com wrote:
On Mon, Oct 27, 2008 at 2:37 PM, Myles Watson mylesgw@gmail.com wrote:
Have you applied my patch to add the 8132 so you can see it working/breaking?
can you give me a diff against current tree?
thanks
ron
Here it is:
Signed-off-by: Myles Watson mylesgw@gmail.com
I'm sorry it doesn't build but it's close and I have to go.
Thanks, Myles
On Mon, Oct 27, 2008 at 3:25 PM, Myles Watson mylesgw@gmail.com wrote:
Sorry. Here's the full patch. This builds.
Signed-off-by: Myles Watson mylesgw@gmail.com
just commit. The other one would not even compile.
Acked-by: Ronald G. Minnich rminnich@gmail.com
On 27.10.2008 23:54, ron minnich wrote:
On Mon, Oct 27, 2008 at 3:25 PM, Myles Watson mylesgw@gmail.com wrote:
Sorry. Here's the full patch. This builds.
Signed-off-by: Myles Watson mylesgw@gmail.com
just commit. The other one would not even compile.
Acked-by: Ronald G. Minnich rminnich@gmail.com
I'm convinced that the dts is still wrong, but in a different way.
That's why I'd like to NACK the dts part. No objection to the rest. I'm currently trying to get the dts right. Give me a few more minutes.
Regards, Carl-Daniel
The explanations by Tom and Marc made a few things clear for me: - The physical HT structure is not what we want to model. - The appearance of HT and topology in PCI config space is what matters. - 18.0 is not a PCI bridge, don't pretend it is one
With that in mind, I'd like to propose another dts. I know that it has its own quirks, but it can serve as a discussion point.
I'm pretty sure someone already posted a lspci -tn and lspci -n for the Serengeti, but I can't find it in my mails right now. A pointer to the archives and/or a repost would be appreciated.
Regards, Carl-Daniel
/{ device_operations="serengeti"; mainboard_vendor = "AMD"; mainboard_name = "Serengeti"; cpus { }; apic@0 { }; domain@0 { /config/("northbridge/amd/k8/domain"); bus@0{ pci@18,0 { /config/("northbridge/amd/k8/pci"); }; pci@18,1 {}; pci@18,2 {}; pci@18,3 {}; }; bus@1{ pci@0,0 { /config/("southbridge/amd/amd8111/pci.dts"); pci@0,0{ /config/("southbridge/amd/amd8111/usb.dts"); }; pci@0,1{ /config/("southbridge/amd/amd8111/usb.dts"); }; pci@0,2{ /config/("southbridge/amd/amd8111/usb2.dts"); disable; }; pci@1,0{ /config/("southbridge/amd/amd8111/nic.dts"); disable; }; }; pci@1,0 { /config/("southbridge/amd/amd8111/lpc.dts"); }; pci@1,1 { /config/("southbridge/amd/amd8111/ide.dts"); }; pci@1,2 { /config/("southbridge/amd/amd8111/smbus.dts"); }; pci@1,3 { /config/("southbridge/amd/amd8111/acpi.dts"); }; pci@1,5 { /config/("southbridge/amd/amd8111/ac97audio.dts"); }; pci@1,6 { /config/("southbridge/amd/amd8111/ac97modem.dts"); }; pci@2,0 { /config/("southbridge/amd/amd8132/pcix.dts"); }; }; ioport@2e { /config/("superio/winbond/w83627hf/dts"); com1enable = "1"; }; }; };
On Mon, Oct 27, 2008 at 4:25 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
The explanations by Tom and Marc made a few things clear for me:
- The physical HT structure is not what we want to model.
- The appearance of HT and topology in PCI config space is what matters.
- 18.0 is not a PCI bridge, don't pretend it is one
With that in mind, I'd like to propose another dts. I know that it has its own quirks, but it can serve as a discussion point.
I'm beginning to feel like we're thinking too much.
The fact is that whether or not the three HT links are a bridge, there is routing in there that makes each link logically the parent of a different set of devices. do we call that a bridge?
I don't know what to do with bus@1.
ah well it's been a long day, will look at this later. For now let's get myles patch in and try to see where that goes. I think I'm not ready for this dts. But if myles wants to try it that's fine too.
ron
On 28.10.2008 00:40, ron minnich wrote:
On Mon, Oct 27, 2008 at 4:25 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
The explanations by Tom and Marc made a few things clear for me:
- The physical HT structure is not what we want to model.
- The appearance of HT and topology in PCI config space is what matters.
- 18.0 is not a PCI bridge, don't pretend it is one
With that in mind, I'd like to propose another dts. I know that it has its own quirks, but it can serve as a discussion point.
I'm beginning to feel like we're thinking too much.
We were trying to coax physical connections into the dts, but then we had to ignore them in the code because the logical connections and their placements were different.
The fact is that whether or not the three HT links are a bridge, there is routing in there that makes each link logically the parent of a different set of devices. do we call that a bridge?
Do we ever walk the HT links explicitly in the code?
I don't know what to do with bus@1.
I fully agree. That's the weakest point of my dts. We can place all those devices at bus 1, but for legacy reasons we may want to have them at bus 0. Our choice.
ah well it's been a long day, will look at this later. For now let's get myles patch in and try to see where that goes. I think I'm not ready for this dts. But if myles wants to try it that's fine too.
Sure, as long as the dts is not set in stone, I retract my NACK and encourage Myles to commit.
Regards, Carl-Daniel
On Mon, Oct 27, 2008 at 4:56 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Do we ever walk the HT links explicitly in the code?
yes. see what gets put into static.c.
stefan and I are going to walk this code tomorrow. We feel that somehow there has to be a better way.
ron
Carl-Daniel Hailfinger wrote:
On 28.10.2008 00:40, ron minnich wrote:
On Mon, Oct 27, 2008 at 4:25 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
The explanations by Tom and Marc made a few things clear for me:
- The physical HT structure is not what we want to model.
- The appearance of HT and topology in PCI config space is what matters.
- 18.0 is not a PCI bridge, don't pretend it is one
With that in mind, I'd like to propose another dts. I know that it has its own quirks, but it can serve as a discussion point.
I'm beginning to feel like we're thinking too much.
We were trying to coax physical connections into the dts, but then we had to ignore them in the code because the logical connections and their placements were different.
This is the right track. Addressing is what is important for resource allocation. Later we can add a physical connection for ht that can be auto-generated runtime if we decide we want to know that as well.
The fact is that whether or not the three HT links are a bridge, there is routing in there that makes each link logically the parent of a different set of devices. do we call that a bridge?
Do we ever walk the HT links explicitly in the code?
Not in device code. All that is done in Stage1 early init.
I don't know what to do with bus@1.
I fully agree. That's the weakest point of my dts. We can place all those devices at bus 1, but for legacy reasons we may want to have them at bus 0. Our choice.
With the assumption that bus@X should be any number found that matches the device. Maybe remove the @X. I don't think it does anything. I think that this is more correct:
bus{ pci@6,0 { /config/("southbridge/amd/amd8111/pci.dts"); bus{ pci@0,0{ /config/("southbridge/amd/amd8111/usb.dts");
Does the bus indicate anything? Would this work?
pci@6,0 { /config/("southbridge/amd/amd8111/pci.dts"); pci@0,0{ /config/("southbridge/amd/amd8111/usb.dts");
ah well it's been a long day, will look at this later. For now let's get myles patch in and try to see where that goes. I think I'm not ready for this dts. But if myles wants to try it that's fine too.
Sure, as long as the dts is not set in stone, I retract my NACK and encourage Myles to commit.
This isn't even set in jello, never mind stone! :)
Attached lspci from Serengeti (FAM10 CPU but buses should be the same).
Marc
I think before we go much further you should all review: statictree.c from serengeti static.c from serengeti
how the links[] array works.
How the config space addressing is setup in in stage1 and how it is used in stage2.
the interaction of the 18.1 routing tables and the allocation of mem, prefmem, and IO spaces by the resource code.
Why there are three 18.1 devices and that interaction with resource allocators.
From my reading there are a few little bits there you have to get
right. I may be wrong but I am not sure all those issues are being taken into consideration.
A writeup of all this by a person who'd never looked at it before would be really useful. Why? because they won't take things for granted -- they'll look at it with a fresh perspective.
Stefan and I are going to walk this stuff as well.
ron
Here's a start on a few of these topics:
I think before we go much further you should all review: statictree.c from serengeti
This file is generated by the code in util/dts - especially see util/dts/flattree.c It contains the logical structure of the device tree, including parents, links, busses, and any other information we include in the dts. Each entry in the dts becomes a device struct, with each device having 3 links.
My observations: 1. It's awkward to have 3 HT busses because they each create a device structure, even though they're all part of the same device. I always end up with the other two links as "left over" devices.
2. It's not clear to me when the "bridge" property should be set. The 8132 is a bridge, but if you set that property it breaks the dts, by making a loop. I think part of the problem is that it's a bridge and a tunnel, which complicates it.
static.c from serengeti
? I'm assuming you mean stage1.c?
This stage writes initial values into the HT routing registers, calls enumerate_ht, calls enable_rom, and enable_serial. I'm not sure what enable_rom does since we're already executing out of it.
In stage1.c I'd love it if the information there could be generated. It seems like it could be if we started from a base configuration and added interesting features like southbridge number and a few bus numbers.
how the links[] array works.
The links array is an array of bus structures. Bus structures hold information like the device that controls the bus, the operations for the bus, etc.
How the config space addressing is setup in in stage1 and how it is used in stage2.
Config space should be set up in stage1 so that all configuration reads go to the link that the southbridge is on. After the southbridge gets enumerated, then the other busses can get enumerated so that the southbridge stays on bus 0 at a low device.
This is very hard to follow in the code because there are so many #defines. I'd love to see them all go away.
the interaction of the 18.1 routing tables and the allocation of mem, prefmem, and IO spaces by the resource code.
This is another area where the #defines make the code ugly. It seems like we should be able to support 64-bit BARs and relocating them by just changing the tolm. The fact that it is a lot more complicated then that in the code makes me worry.
Why there are three 18.1 devices and that interaction with resource allocators.
Where would I find the answer to this question?
From my reading there are a few little bits there you have to get right. I may be wrong but I am not sure all those issues are being taken into consideration.
A writeup of all this by a person who'd never looked at it before would be really useful. Why? because they won't take things for granted -- they'll look at it with a fresh perspective.
Feel free to ask more questions and I'll try to find more answers.
Stefan and I are going to walk this stuff as well.
Great!
Thanks, Myles
On 28.10.2008 01:39, Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
On 28.10.2008 00:40, ron minnich wrote:
On Mon, Oct 27, 2008 at 4:25 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
The explanations by Tom and Marc made a few things clear for me:
- The physical HT structure is not what we want to model.
- The appearance of HT and topology in PCI config space is what
matters.
- 18.0 is not a PCI bridge, don't pretend it is one
With that in mind, I'd like to propose another dts. I know that it has its own quirks, but it can serve as a discussion point.
I'm beginning to feel like we're thinking too much.
We were trying to coax physical connections into the dts, but then we had to ignore them in the code because the logical connections and their placements were different.
This is the right track. Addressing is what is important for resource allocation. Later we can add a physical connection for ht that can be auto-generated runtime if we decide we want to know that as well.
My biggest problem with all this stuff was that I thought we'd model the device tree after the logical PCI topology. That would make scanning easier and resource allocation more difficult. However, your suggestion to model the device tree after resource allocation topology is probably more promising. It does complicate scanning, though.
The fact is that whether or not the three HT links are a bridge, there is routing in there that makes each link logically the parent of a different set of devices. do we call that a bridge?
Do we ever walk the HT links explicitly in the code?
Not in device code. All that is done in Stage1 early init.
Hm. Since stage1 doesn't care about the device tree (yet), this allows us to use either approach.
I don't know what to do with bus@1.
I fully agree. That's the weakest point of my dts. We can place all those devices at bus 1, but for legacy reasons we may want to have them at bus 0. Our choice.
With the assumption that bus@X should be any number found that matches the device. Maybe remove the @X. I don't think it does anything. I think that this is more correct:
My original idea was that we'd be able to specify bus numbers if we wanted and some sort of placeholder if we didn't care.
bus{ pci@6,0 { /config/("southbridge/amd/amd8111/pci.dts"); bus{ pci@0,0{ /config/("southbridge/amd/amd8111/usb.dts");
Does the bus indicate anything? Would this work?
The bus element itself is needed IMO. It allows us to group devices together which can be discovered/scanned in one go. Instead of
pci0@18,0{ pci@1,0{}; }; pci1@18,0{ //some stuff behind the second link }; pci2@18,0{ //some stuff behind the third link }; pci@18,1{}; pci@18,2{};
we'd have pci@18,0{ bus@0{ pci@1,0{}; }; bus@1{ //some stuff behind the second link }; bus@2{ //some stuff behind the third link }; }; pci@18,1{}; pci@18,2{};
and that would remove the need for special magic notation in the dts.
However, it is also an interesting question on which "layer" we want to keep the bus devices.
pci@6,0 { /config/("southbridge/amd/amd8111/pci.dts"); pci@0,0{ /config/("southbridge/amd/amd8111/usb.dts");
ah well it's been a long day, will look at this later. For now let's get myles patch in and try to see where that goes. I think I'm not ready for this dts. But if myles wants to try it that's fine too.
Sure, as long as the dts is not set in stone, I retract my NACK and encourage Myles to commit.
This isn't even set in jello, never mind stone! :)
Attached lspci from Serengeti (FAM10 CPU but buses should be the same).
Thanks!
From that lspci it becomes clear that the resource tree is mostly
congruent with the logical PCI tree, but they differ significantly in a few places. It is desirable to have both trees available: One for resource allocation, one for device discovery. Killing either is calling for problems down the road. Besides that, in reality we're neither dealing with a real tree nor a completely acyclic graph. There are easy ways out which either make the dts completely unreadable or beget some unholy device tree code. Neither is acceptable if we consider maintainability to be important.
The good thing is that my diploma thesis provides solutions for exactly that class of problems. It's not about coreboot, but the problems we face right now are so strikingly similar that I regret not writing the whole thing in English. Anyway, I'll try to create a short writeup which captures the essential results, and how to apply them without losing mental sanity (or maintainability, for that matter).
The trick is to declare a preferred and readable form for storing device relations (the existing dts structure works quite well in that regard) and to handle different requirements for that representation like database views. With a nice set of node properties and the right amount of abstraction, we can keep both the code and the dts human readable and have them provide easily understood views of their respective purposes (resource allocation vs. device discovery). If we do it right, even such horrors as wandering PATA/SATA PCI devices (after disabling PATA, SATA will change its PCI devfn) are easily expressed and handled.
Regards, Carl-Daniel
-----Original Message----- From: ron minnich [mailto:rminnich@gmail.com] Sent: Monday, October 27, 2008 4:54 PM To: Myles Watson Cc: Marc Jones; Coreboot Subject: Re: [coreboot] v3 HT
On Mon, Oct 27, 2008 at 3:25 PM, Myles Watson mylesgw@gmail.com wrote:
Sorry. Here's the full patch. This builds.
Signed-off-by: Myles Watson mylesgw@gmail.com
just commit. The other one would not even compile.
Yep. I forgot the pcix diff with the last one.
Acked-by: Ronald G. Minnich rminnich@gmail.com
Rev 956.
Thanks, Myles
-
Carl-Daniel Hailfinger -
Marc Jones -
Myles Watson -
ron minnich