Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

2012-05-04 Thread Kevin O'Connor
On Wed, May 02, 2012 at 03:42:51PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
> > 0) break" from pci_bios_init_devices()?
> 
> Seems to do the trick, at least the disks connected appear in the boot
> menu now and the seabios log file looks sane.
> 
> The guest kernel has no virtio-scsi drivers though, need to update it
> for more testing.
> 
> > The code should probably handle the irq swizzling that pci bridges do
> > though.
> 
> i.e. add bridge handling to pci_slot_get_irq() ?

Yes.

-Kevin



Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

2012-05-02 Thread Gerd Hoffmann
  Hi,

> Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
> 0) break" from pci_bios_init_devices()?

Seems to do the trick, at least the disks connected appear in the boot
menu now and the seabios log file looks sane.

The guest kernel has no virtio-scsi drivers though, need to update it
for more testing.

> The code should probably handle the irq swizzling that pci bridges do
> though.

i.e. add bridge handling to pci_slot_get_irq() ?

cheers,
  Gerd




Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

2012-04-29 Thread Alexey Korolev
On 27/04/12 00:45, Kevin O'Connor wrote:
> On Wed, Apr 25, 2012 at 05:29:04PM +0200, Gerd Hoffmann wrote:
>> 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.
> Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
> 0) break" from pci_bios_init_devices()?
This helps in my tests. The devices are no longer disabled. Not sure
about virtio-scsi test though.
> The code should probably handle the irq swizzling that pci bridges do
> though.
>




Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

2012-04-26 Thread Kevin O'Connor
On Thu, Apr 26, 2012 at 07:12:22PM +1200, Alexey Korolev wrote:
> On 26/04/12 03:29, Gerd Hoffmann wrote:
> >> 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?
> > These are certainly no blockers and can be discussed and solved after
> > committing this series.
> 
> I've just fixed what you have suggested to fix.
> Just very minor changes in patches 10/12 and 11/12:
> - Simplify a bit list operation.
> - Remove function calls away of macros
> - Add condition to check crossing the end of 64bit PCI range

Thanks.  I committed this series.

-Kevin



Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

2012-04-26 Thread Kevin O'Connor
On Wed, Apr 25, 2012 at 05:29:04PM +0200, Gerd Hoffmann wrote:
> 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.

Is this just a matter of removing the "if (pci_bdf_to_bus(pci->bdf) !=
0) break" from pci_bios_init_devices()?

The code should probably handle the irq swizzling that pci bridges do
though.

> 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, ...

I'd vote for 2.  There's nearly 500MB of space for PCI devices under
4G - small regions (say under 1MB) are unlikely to be the cause of an
allocation failure (1MB would be .2% of total space).

-Kevin



Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

2012-04-26 Thread Gerd Hoffmann
  Hi,

>> 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.
> I observed similar but ignored since "bridge test" requires special qemu 
> build.
> It's a general bridge support issue. This series don't address this issue.
> This issue occurs regardless if the patches applied or not.

FYI: Meanwhile the bridge patches have been merged, latest qemu master
branch will do.

cheers,
  Gerd




Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions

2012-04-25 Thread Gerd Hoffmann
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