[Qemu-devel] [PATCH 0/2] Fix latent(?) bootindex bugs

[Qemu-devel] [PATCH 0/2] Fix latent(?) bootindex bugs

[Markus Armbruster]

I put (?) behind latent, because I didn't actually prove the reason it
can't bite USB I put forward in PATCH 1.

PATCH 1 should be picked up by the USB maintainer (Kraxel), and PATCH
2 by a block maintainer (Kevin or Stefan).  I'm posting this as a
series regardless, because I want them reviewed together.

Related: [SeaBIOS] [PATCH v3] boot: Fix boot order for SCSI target,
lun > 9
http://www.seabios.org/pipermail/seabios/2014-August/008205.html
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1096560

Markus Armbruster (2):
  usb: Fix bootindex for portnr > 9
  ide: Fix bootindex for bus_id > 9

 hw/ide/qdev.c | 2 +-
 hw/usb/bus.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/2] usb: Fix bootindex for portnr > 9

[Markus Armbruster]

We identify devices by their Open Firmware device paths.  The encoding
of the host controller and hub port numbers is incorrect:
usb_get_fw_dev_path() formats them in decimal, while SeaBIOS uses
hexadecimal.  When some port number > 9, SeaBIOS will miss the
bootindex (lucky case), or apply it to another device (unlucky case).

The relevant spec[*] agrees with SeaBIOS (and OVMF, for that matter).
Change %d to %x.

Bug can bite only with host controllers or hubs sporting more than ten
ports.  I'm not aware of any.

[*] Open Firmware Recommended Practice: Universal Serial Bus,
Version 1, Section 3.2.1 Device Node Address Representation
http://www.openfirmware.org/1275/bindings/usb/usb-1_0.ps

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/usb/bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 927a47b..516fb52 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -589,11 +589,11 @@ static char *usb_get_fw_dev_path(DeviceState *qdev)
         nr = strtol(in, &in, 10);
         if (in[0] == '.') {
             /* some hub between root port and device */
-            pos += snprintf(fw_path + pos, fw_len - pos, "hub@%ld/", nr);
+            pos += snprintf(fw_path + pos, fw_len - pos, "hub@%lx/", nr);
             in++;
         } else {
             /* the device itself */
-            pos += snprintf(fw_path + pos, fw_len - pos, "%s@%ld",
+            pos += snprintf(fw_path + pos, fw_len - pos, "%s@%lx",
                             qdev_fw_name(qdev), nr);
             break;
         }
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/2] usb: Fix bootindex for portnr > 9

[Gerd Hoffmann]

On Fr, 2014-08-15 at 13:32 +0200, Markus Armbruster wrote:
> We identify devices by their Open Firmware device paths.  The encoding
> of the host controller and hub port numbers is incorrect:
> usb_get_fw_dev_path() formats them in decimal, while SeaBIOS uses
> hexadecimal.  When some port number > 9, SeaBIOS will miss the
> bootindex (lucky case), or apply it to another device (unlucky case).
> 
> The relevant spec[*] agrees with SeaBIOS (and OVMF, for that matter).
> Change %d to %x.
> 
> Bug can bite only with host controllers or hubs sporting more than ten
> ports.  I'm not aware of any.

fyi: xhci can be configured with up to 15 ports (default is 4 ports
though).

Applied to usb patch queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] usb: Fix bootindex for portnr > 9

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On Fr, 2014-08-15 at 13:32 +0200, Markus Armbruster wrote:
>> We identify devices by their Open Firmware device paths.  The encoding
>> of the host controller and hub port numbers is incorrect:
>> usb_get_fw_dev_path() formats them in decimal, while SeaBIOS uses
>> hexadecimal.  When some port number > 9, SeaBIOS will miss the
>> bootindex (lucky case), or apply it to another device (unlucky case).
>> 
>> The relevant spec[*] agrees with SeaBIOS (and OVMF, for that matter).
>> Change %d to %x.
>> 
>> Bug can bite only with host controllers or hubs sporting more than ten
>> ports.  I'm not aware of any.
>
> fyi: xhci can be configured with up to 15 ports (default is 4 ports
> though).

Would be nice to put that into the commit message, between my S-o-B and
yours.

> Applied to usb patch queue.

Thanks!

[Qemu-devel] [PATCH 2/2] ide: Fix bootindex for bus_id > 9

[Markus Armbruster]

We identify devices by their Open Firmware device paths.  The encoding
of bus numbers is incorrect: idebus_get_fw_dev_path() formats them in
decimal, while SeaBIOS uses hexadecimal.  With bus number > 9, SeaBIOS
will miss the bootindex (lucky case), or apply it to another device
(unlucky case).

Bug can't bite right now: ich9-ahci has six ports, and the sysbus-ahci
created by Calxeda Highbank has just one.

Fix it anyway, by changing %d to %x.

I couldn't find an Open Firmware spec covering this.  For what it's
worth, OVMF agrees with SeaBIOS.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/qdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6e475e6..9c4d221 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -59,7 +59,7 @@ static char *idebus_get_fw_dev_path(DeviceState *dev)
 {
     char path[30];
 
-    snprintf(path, sizeof(path), "%s@%d", qdev_fw_name(dev),
+    snprintf(path, sizeof(path), "%s@%x", qdev_fw_name(dev),
              ((IDEBus*)dev->parent_bus)->bus_id);
 
     return g_strdup(path);
-- 
1.9.3

Re: [Qemu-devel] [PATCH 2/2] ide: Fix bootindex for bus_id > 9

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

On Fri, Aug 15, 2014 at 01:32:37PM +0200, Markus Armbruster wrote:
> We identify devices by their Open Firmware device paths.  The encoding
> of bus numbers is incorrect: idebus_get_fw_dev_path() formats them in
> decimal, while SeaBIOS uses hexadecimal.  With bus number > 9, SeaBIOS
> will miss the bootindex (lucky case), or apply it to another device
> (unlucky case).
> 
> Bug can't bite right now: ich9-ahci has six ports, and the sysbus-ahci
> created by Calxeda Highbank has just one.
> 
> Fix it anyway, by changing %d to %x.
> 
> I couldn't find an Open Firmware spec covering this.  For what it's
> worth, OVMF agrees with SeaBIOS.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/ide/qdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/2] Fix latent(?) bootindex bugs

[Laszlo Ersek]

On 08/15/14 13:32, Markus Armbruster wrote:
> I put (?) behind latent, because I didn't actually prove the reason it
> can't bite USB I put forward in PATCH 1.
> 
> PATCH 1 should be picked up by the USB maintainer (Kraxel), and PATCH
> 2 by a block maintainer (Kevin or Stefan).  I'm posting this as a
> series regardless, because I want them reviewed together.
> 
> Related: [SeaBIOS] [PATCH v3] boot: Fix boot order for SCSI target,
> lun > 9
> http://www.seabios.org/pipermail/seabios/2014-August/008205.html
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1096560
> 
> Markus Armbruster (2):
>   usb: Fix bootindex for portnr > 9
>   ide: Fix bootindex for bus_id > 9
> 
>  hw/ide/qdev.c | 2 +-
>  hw/usb/bus.c  | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 

series
Reviewed-by: Laszlo Ersek <lersek@redhat.com>