[PATCH v2] boot: Fix boot order for SCSI target, lun > 9

[Markus Armbruster]

We identify devices by their Open Firmware device paths.  The path
component for the logical unit on a bus is incorrect:
bootprio_find_pci_device() formats target (a.k.a. SCSI ID) and lun in
decimal, while QEMU uses hexadecimal.  Bootorder list entries with
target, lun > 9 aren't found (lucky case), or attributed to the wrong
logical unit (unlucky case).

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

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1096560

[*] Open Firmware Recommended Practice: SCSI-3 Parallel Interface,
Version 1, section 3.1 Physical Address Formats and Representations
http://www.openfirmware.org/1275/practice/spi/spi1_0.ps

Signed-off-by: Markus Armbruster 
---
v2:
- Fix the link to the spec (d'oh)
- While we're linking, link to RHBZ

src/boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/boot.c b/src/boot.c
index 5837ad0..6e41ccf 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -145,7 +145,7 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
     // Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
     char desc[256], *p;
     p = build_pci_path(desc, sizeof(desc), "*", pci);
-    snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
+    snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%x,%x", target, lun);
     return find_prio(desc);
 }

[Laszlo Ersek]

On 08/14/14 22:18, Markus Armbruster wrote:
> We identify devices by their Open Firmware device paths.  The path
> component for the logical unit on a bus is incorrect:
> bootprio_find_pci_device() formats target (a.k.a. SCSI ID) and lun in
> decimal, while QEMU uses hexadecimal.  Bootorder list entries with
> target, lun > 9 aren't found (lucky case), or attributed to the wrong
> logical unit (unlucky case).
>
> The relevant spec[*] agrees with QEMU (and OVMF, for that matter).
> Change %d to %x.
>
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1096560
>
> [*] Open Firmware Recommended Practice: SCSI-3 Parallel Interface,
> Version 1, section 3.1 Physical Address Formats and Representations
> http://www.openfirmware.org/1275/practice/spi/spi1_0.ps
> Signed-off-by: Markus Armbruster
>
> v2:
> Fix the link to the spec (d'oh)
> While we're linking, link to RHBZ
>
> src/boot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/boot.c b/src/boot.c
> index 5837ad0..6e41ccf 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -145,7 +145,7 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
>      // Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
>      char desc[256], *p;
>      p = build_pci_path(desc, sizeof(desc), "*", pci);
> snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
> snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%x,%x", target, lun);
> return find_prio(desc);
>
> }

Reviewed-by: Laszlo Ersek

[Kevin O'Connor]

On Thu, Aug 14, 2014 at 10:18:34PM +0200, Markus Armbruster wrote:
> We identify devices by their Open Firmware device paths.  The path
> component for the logical unit on a bus is incorrect:
> bootprio_find_pci_device() formats target (a.k.a. SCSI ID) and lun in
> decimal, while QEMU uses hexadecimal.  Bootorder list entries with
> target, lun > 9 aren't found (lucky case), or attributed to the wrong
> logical unit (unlucky case).
>
> The relevant spec[*] agrees with QEMU (and OVMF, for that matter).
> Change %d to %x.
>
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1096560
>
> [*] Open Firmware Recommended Practice: SCSI-3 Parallel Interface,
> Version 1, section 3.1 Physical Address Formats and Representations
> http://www.openfirmware.org/1275/practice/spi/spi1_0.ps

Thanks.  The patch looks okay to me.  But, does bootprio_find_usb()
also need to change?

I also wonder if the ":rom%d" stuff in bootprio_find_*_rom() should
also be made hex just for consistency (though it seems unlikely a
single rom would ever have more than 10 drives on it).

-Kevin

[PATCH v2] boot: Fix boot order for SCSI target, lun > 9

[Markus Armbruster]

"Kevin O'Connor"  writes:
> On Thu, Aug 14, 2014 at 10:18:34PM +0200, Markus Armbruster wrote:
> > We identify devices by their Open Firmware device paths.  The path
> > component for the logical unit on a bus is incorrect:
> > bootprio_find_pci_device() formats target (a.k.a. SCSI ID) and lun in
> > decimal, while QEMU uses hexadecimal.  Bootorder list entries with
> > target, lun > 9 aren't found (lucky case), or attributed to the wrong
> > logical unit (unlucky case).
> >
> > The relevant spec[*] agrees with QEMU (and OVMF, for that matter).
> > Change %d to %x.
> >
> > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1096560
> >
> > [*] Open Firmware Recommended Practice: SCSI-3 Parallel Interface,
> > Version 1, section 3.1 Physical Address Formats and Representations
> > http://www.openfirmware.org/1275/practice/spi/spi1_0.ps
>
> Thanks.  The patch looks okay to me.  But, does bootprio_find_usb()
> also need to change?

You're right, there's a LUN hiding in that path.  I'll send v3.

Aside: the USB port number is also inconsistent with QEMU. but in that
case, SeaBIOS is correct and QEMU is wrong.  I'm going to fix it.
> I also wonder if the ":rom%d" stuff in bootprio_find_*_rom() should
> also be made hex just for consistency (though it seems unlikely a
> single rom would ever have more than 10 drives on it).

I don't know.  QEMU never generates a ":NUMBER" suffix, and I haven't
found a specification for this device path.

[Kevin O'Connor]

On Fri, Aug 15, 2014 at 08:39:19AM +0200, Markus Armbruster wrote:
> > I also wonder if the ":rom%d" stuff in bootprio_find_*_rom() should
> > also be made hex just for consistency (though it seems unlikely a
> > single rom would ever have more than 10 drives on it).
>
> I don't know.  QEMU never generates a ":NUMBER" suffix, and I haven't
> found a specification for this device path.

It isn't part of a spec and I don't think QEMU ever used it.  Now that
I think about it, I believe I added it to make sure multiple BCV
declarations in a rom could be uniquely identified and wouldn't be
confused with each other during pattern matching.  So, a change to %x
would purely be for consistency.

I'll put up a separate patch for it.

-Kevin

[PATCH v2] boot: Fix boot order for SCSI target, lun > 9

[Markus Armbruster]

"Kevin O'Connor"  writes:
> On Fri, Aug 15, 2014 at 08:39:19AM +0200, Markus Armbruster wrote:
> > > I also wonder if the ":rom%d" stuff in bootprio_find_*_rom() should
> > > also be made hex just for consistency (though it seems unlikely a
> > > single rom would ever have more than 10 drives on it).
> >
> > I don't know.  QEMU never generates a ":NUMBER" suffix, and I haven't
> > found a specification for this device path.
>
> It isn't part of a spec and I don't think QEMU ever used it.  Now that
> I think about it, I believe I added it to make sure multiple BCV
> declarations in a rom could be uniquely identified and wouldn't be
> confused with each other during pattern matching.  So, a change to %x
> would purely be for consistency.
>
> I'll put up a separate patch for it.

Makes sense, as everything else is hexadecimal.