On 04/25/12 14:51, Kevin O'Connor wrote:
On Wed, Apr 25, 2012 at 06:25:07PM +1200, Alexey Korolev wrote:
On 25/04/12 13:48, Kevin O'Connor wrote:
On Tue, Apr 24, 2012 at 06:25:39PM +1200, Alexey Korolev wrote:
pci_region_map_entries(busses, &r64_mem);pci_region_map_entries(busses, &r64_pref);- } // Map regions on each device.
This doesn't look right to me. This will map the devices on bus 0 to the proper >4g address, but devices on any subsequent bus will use busses[0].r[].base which will be reset to the <4gig address. Perhaps pull base out of pci_region and make pci_region_map_entries() recursive?
No recursion is need here! We map all entries which are 64bit on root bus. If entry is a bridge region - a corresponding bus address will be updated. Region won't be reseted to <4gig address as address is derived from parent region only.
Okay - I missed that. I think the patches look okay to be committed - any additional changes can be made on top. Gerd - do you have any comments?
Great job overall. Can't spot issues in the code. And the patches do fine in testing too. Some minor issues poped up:
Issue #1: seabios can't boot from a virtio-scsi disk if the controller is behind a pci bridge. I think the reason simply is that (according to the seabios log) only root bus pci devices are initialized. Probably even isn't related to this patch set, just trapped into this while testing the bridge mapping code of the patch series.
Issue #2: root bus (non-pref) memory regions are mapped above 4G if they are 64bit capable. That happened to include the xhci usb controller. I don't think we want that. Some day seabios will get xhci support, and having the bar above 4G makes it unreachable in 32bit mode. So this needs some refinement. Options I can think of:
(1) Don't bother mapping non-prefmem bars above 4G. (2) Only map them above 4G if they are larger than a certain limit. (3) Allow devices to be excluded on certain conditions, for example when seabios has a driver, when they have an option rom, when they have a specific pci id, ...
These are certainly no blockers and can be discussed and solved after committing this series.
cheers, Gerd