Quantcast

OpenBSD 6.1: BOOTIA32 3.32 issue

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
Hi all,
I tried to upgrade to OpenBSD 6.1 on an Asus X205TA (bay trail, 32 bit
efi, 64 bit os) but the bootloader do not correctly detect the internal
disk.

I also tried a fresh install, but things do not change.
Boot fails and when I do a "machine diskinfo" I got a lot of "?"
symbols (a video here https://www.youtube.com/watch?v=fsomNX-oFTQ )

How can I debug the issue?

Thanks,
Michele

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Tue, May 09, 2017 at 10:20:03AM +0200, Michele Curti wrote:

> Hi all,
> I tried to upgrade to OpenBSD 6.1 on an Asus X205TA (bay trail, 32 bit
> efi, 64 bit os) but the bootloader do not correctly detect the internal
> disk.
>
> I also tried a fresh install, but things do not change.
> Boot fails and when I do a "machine diskinfo" I got a lot of "?"
> symbols (a video here https://www.youtube.com/watch?v=fsomNX-oFTQ )
>
> How can I debug the issue?
>

Compiling bootia32.efi :p

With sys/arch/amd64/stand/efiboot/efiboot.c revision 1.15 it works,
revision 1.16 it fails.

I'll try to understand, thanks,
Michele

> Thanks,
> Michele
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Tue, May 09, 2017 at 09:36:02PM +0200, Michele Curti wrote:

> On Tue, May 09, 2017 at 10:20:03AM +0200, Michele Curti wrote:
> > Hi all, I tried to upgrade to OpenBSD 6.1 on an Asus X205TA (bay
> > trail, 32 bit efi, 64 bit os) but the bootloader do not correctly
> > detect the internal disk.
> >
> > I also tried a fresh install, but things do not change.  Boot fails
> > and when I do a "machine diskinfo" I got a lot of "?" symbols (a video
> > here https://www.youtube.com/watch?v=fsomNX-oFTQ )
> >
> > How can I debug the issue?
> >
>
> Compiling bootia32.efi :p
>
> With sys/arch/amd64/stand/efiboot/efiboot.c revision 1.15 it works,
> revision 1.16 it fails.
>
> I'll try to understand, thanks, Michele


With the following diff it works, bye!


Index: efiboot/efiboot.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.17
diff -u -p -r1.17 efiboot.c
--- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
+++ efiboot/efiboot.c 9 May 2017 19:44:30 -0000
@@ -92,7 +92,7 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TA
  if (DevicePathType(dp) == MEDIA_DEVICE_PATH &&
     DevicePathSubType(dp) == MEDIA_HARDDRIVE_DP) {
  bios_bootdev = 0x80;
- efi_bootdp = dp0;
+ efi_bootdp = dp;
  break;
  }
  }

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Stefan Sperling-5
On Tue, May 09, 2017 at 09:47:14PM +0200, Michele Curti wrote:

> On Tue, May 09, 2017 at 09:36:02PM +0200, Michele Curti wrote:
> > On Tue, May 09, 2017 at 10:20:03AM +0200, Michele Curti wrote:
> > > Hi all, I tried to upgrade to OpenBSD 6.1 on an Asus X205TA (bay
> > > trail, 32 bit efi, 64 bit os) but the bootloader do not correctly
> > > detect the internal disk.
> > >
> > > I also tried a fresh install, but things do not change.  Boot fails
> > > and when I do a "machine diskinfo" I got a lot of "?" symbols (a video
> > > here https://www.youtube.com/watch?v=fsomNX-oFTQ )
> > >
> > > How can I debug the issue?
> > >
> >
> > Compiling bootia32.efi :p
> >
> > With sys/arch/amd64/stand/efiboot/efiboot.c revision 1.15 it works,
> > revision 1.16 it fails.
> >
> > I'll try to understand, thanks, Michele
>
>
> With the following diff it works, bye!

Looks good to me. Is anyone handling this patch?

> Index: efiboot/efiboot.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 efiboot.c
> --- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
> +++ efiboot/efiboot.c 9 May 2017 19:44:30 -0000
> @@ -92,7 +92,7 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TA
>   if (DevicePathType(dp) == MEDIA_DEVICE_PATH &&
>      DevicePathSubType(dp) == MEDIA_HARDDRIVE_DP) {
>   bios_bootdev = 0x80;
> - efi_bootdp = dp0;
> + efi_bootdp = dp;
>   break;
>   }
>   }
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Patrick Wildt-3
On Wed, May 10, 2017 at 03:14:30PM +0200, Stefan Sperling wrote:

> On Tue, May 09, 2017 at 09:47:14PM +0200, Michele Curti wrote:
> > On Tue, May 09, 2017 at 09:36:02PM +0200, Michele Curti wrote:
> > > On Tue, May 09, 2017 at 10:20:03AM +0200, Michele Curti wrote:
> > > > Hi all, I tried to upgrade to OpenBSD 6.1 on an Asus X205TA (bay
> > > > trail, 32 bit efi, 64 bit os) but the bootloader do not correctly
> > > > detect the internal disk.
> > > >
> > > > I also tried a fresh install, but things do not change.  Boot fails
> > > > and when I do a "machine diskinfo" I got a lot of "?" symbols (a video
> > > > here https://www.youtube.com/watch?v=fsomNX-oFTQ )
> > > >
> > > > How can I debug the issue?
> > > >
> > >
> > > Compiling bootia32.efi :p
> > >
> > > With sys/arch/amd64/stand/efiboot/efiboot.c revision 1.15 it works,
> > > revision 1.16 it fails.
> > >
> > > I'll try to understand, thanks, Michele
> >
> >
> > With the following diff it works, bye!
>
> Looks good to me. Is anyone handling this patch?
>
> > Index: efiboot/efiboot.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> > retrieving revision 1.17
> > diff -u -p -r1.17 efiboot.c
> > --- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
> > +++ efiboot/efiboot.c 9 May 2017 19:44:30 -0000
> > @@ -92,7 +92,7 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TA
> >   if (DevicePathType(dp) == MEDIA_DEVICE_PATH &&
> >      DevicePathSubType(dp) == MEDIA_HARDDRIVE_DP) {
> >   bios_bootdev = 0x80;
> > - efi_bootdp = dp0;
> > + efi_bootdp = dp;
> >   break;
> >   }
> >   }
> >
>

I don't think this is the correct fix.  It might solve your issue, but I
don't think it's completely right.  So EFI has those so called device
paths.  A path is basically a list of nodes.  To compare two paths you
need to compare the whole path and not just a single node of it.  If you
store dp instead of dp0 you will basically only save a part of the path,
not the full path.

What you can do is print the full path of efi_bootdp like..

    for (dp = efi_bootdp; !IsDevicePathEnd(dp);
        dp = NextDevicePathNode(dp)) {
            printf("%x %x - ", DevicePathType(dp), DevicePathSubType(dp));
    }
    printf("\n");

And do the same for the DPs that are being put into the
efi_device_path_cmp function.  That will at least print the types, but
not the content of the nodes.  That's a start into figuring out why it
does not correctly compare the paths.

Maybe there's a bug in the compare code?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Wed, May 10, 2017 at 08:35:28PM +0200, Patrick Wildt wrote:

> On Wed, May 10, 2017 at 03:14:30PM +0200, Stefan Sperling wrote:
> > On Tue, May 09, 2017 at 09:47:14PM +0200, Michele Curti wrote:
> > >   bios_bootdev = 0x80;
> > > - efi_bootdp = dp0;
> > > + efi_bootdp = dp;
> > >   break;
> > >   }
> > >   }
> > >
> >
>
> I don't think this is the correct fix.  It might solve your issue, but I
> don't think it's completely right.  So EFI has those so called device
> paths.  A path is basically a list of nodes.  To compare two paths you
> need to compare the whole path and not just a single node of it.  If you
> store dp instead of dp0 you will basically only save a part of the path,
> not the full path.
>
> What you can do is print the full path of efi_bootdp like..
>
>     for (dp = efi_bootdp; !IsDevicePathEnd(dp);
>         dp = NextDevicePathNode(dp)) {
>             printf("%x %x - ", DevicePathType(dp), DevicePathSubType(dp));
>     }
>     printf("\n");
>

4e 6f - 5f 2d - 22 4e - 4e 55 - 3a 48 - 1e ce - and many others ........

I got the same values starting the for loop with dp = dp0 or dp = NULL

So dp0 was not intialized by the EFI_CALL() above?

        if (status == EFI_SUCCESS)
                status = EFI_CALL(BS->HandleProtocol, imgp->DeviceHandle,
                    &devp_guid, (void **)&dp0);
        if (status == EFI_SUCCESS) {


I'm going to study a bit about EFI.. :p

Thanks,
Michele

> And do the same for the DPs that are being put into the
> efi_device_path_cmp function.  That will at least print the types, but
> not the content of the nodes.  That's a start into figuring out why it
> does not correctly compare the paths.
>
> Maybe there's a bug in the compare code?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

YASUOKA Masahiko-3
In reply to this post by Michele Curti
Hi,

On Tue, 9 May 2017 10:20:03 +0200
Michele Curti <[hidden email]> wrote:
> I also tried a fresh install, but things do not change.
> Boot fails and when I do a "machine diskinfo" I got a lot of "?"
> symbols (a video here https://www.youtube.com/watch?v=fsomNX-oFTQ )

Hanging on "machine diskinfo" seems to be a different problem.
The diff is already committed.  Can you test this?

(I'll look into another problem later)

Index: sys/arch/amd64/stand/efiboot/efidev.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efidev.c,v
retrieving revision 1.24
diff -u -p -r1.24 efidev.c
--- sys/arch/amd64/stand/efiboot/efidev.c 24 Dec 2016 08:41:13 -0000 1.24
+++ sys/arch/amd64/stand/efiboot/efidev.c 11 May 2017 01:31:13 -0000
@@ -789,7 +789,7 @@ efi_dump_diskinfo(void)
  printf("hd%d\t%u\t%u\t%u%s\t0x%x\t0x%x\t%s\n",
     (bdi->bios_number & 0x7f),
     ed->blkio->Media->BlockSize,
-    ed->blkio->Media->IoAlign, siz, sizu,
+    ed->blkio->Media->IoAlign, (int)siz, sizu,
     bdi->flags, bdi->checksum,
     (ed->blkio->Media->RemovableMedia)? "Removable" : "");
  }

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Thu, May 11, 2017 at 10:42:04AM +0900, YASUOKA Masahiko wrote:

> Hi,
>
> On Tue, 9 May 2017 10:20:03 +0200
> Michele Curti <[hidden email]> wrote:
> > I also tried a fresh install, but things do not change.
> > Boot fails and when I do a "machine diskinfo" I got a lot of "?"
> > symbols (a video here https://www.youtube.com/watch?v=fsomNX-oFTQ )
>
> Hanging on "machine diskinfo" seems to be a different problem.
> The diff is already committed.  Can you test this?

Yes, no more hangs, thank you!

boot> machine diskinfo
Disk BlkSiz IoAlign Size Flags Checksum
hd0 512 1 4MB 0x0 0x0
hd1 512 1 4MB 0x0 0x0
hd2 512 1 29GB 0x2 0xad4a42c3

Michele

>
> (I'll look into another problem later)
>
> Index: sys/arch/amd64/stand/efiboot/efidev.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efidev.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 efidev.c
> --- sys/arch/amd64/stand/efiboot/efidev.c 24 Dec 2016 08:41:13 -0000 1.24
> +++ sys/arch/amd64/stand/efiboot/efidev.c 11 May 2017 01:31:13 -0000
> @@ -789,7 +789,7 @@ efi_dump_diskinfo(void)
>   printf("hd%d\t%u\t%u\t%u%s\t0x%x\t0x%x\t%s\n",
>      (bdi->bios_number & 0x7f),
>      ed->blkio->Media->BlockSize,
> -    ed->blkio->Media->IoAlign, siz, sizu,
> +    ed->blkio->Media->IoAlign, (int)siz, sizu,
>      bdi->flags, bdi->checksum,
>      (ed->blkio->Media->RemovableMedia)? "Removable" : "");
>   }
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
In reply to this post by Patrick Wildt-3
On Wed, May 10, 2017 at 08:35:28PM +0200, Patrick Wildt wrote:

> On Wed, May 10, 2017 at 03:14:30PM +0200, Stefan Sperling wrote:
> > On Tue, May 09, 2017 at 09:47:14PM +0200, Michele Curti wrote:
> > > On Tue, May 09, 2017 at 09:36:02PM +0200, Michele Curti wrote:
> > > > On Tue, May 09, 2017 at 10:20:03AM +0200, Michele Curti wrote:
> > > > > Hi all, I tried to upgrade to OpenBSD 6.1 on an Asus X205TA (bay
> > > > > trail, 32 bit efi, 64 bit os) but the bootloader do not correctly
> > > > > detect the internal disk.
> > > > >
> > > > > I also tried a fresh install, but things do not change.  Boot fails
> > > > > and when I do a "machine diskinfo" I got a lot of "?" symbols (a video
> > > > > here https://www.youtube.com/watch?v=fsomNX-oFTQ )
> > > > >

Thanks to yasuoka fix I just noted that using dp0 instead of dp
changes the diskinfo disks order

# setting efi_bootdp = dp;

Disk BlkSiz IoAlign Size Flags Checksum
hd0 512 1 29GB 0x2 0xad4a42c3
hd1 512 1 4MB 0x0 0x0
hd2 512 1 4MB 0x0 0x0

# setting efi_bootdp = dp0;

Disk BlkSiz IoAlign Size Flags Checksum
hd0 512 1 4MB 0x0 0x0
hd1 512 1 4MB 0x0 0x0
hd2 512 1 29GB 0x2 0xad4a42c3

So I can use the stock bootloader without changes but I must do a

boot> set device hd2a

Do not know how much useful is this info...

Michele

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

YASUOKA Masahiko-3
Hi,

Thank you for your tests,

On Thu, 11 May 2017 07:40:42 +0200
Michele Curti <[hidden email]> wrote:

> On Wed, May 10, 2017 at 08:35:28PM +0200, Patrick Wildt wrote:
>> On Wed, May 10, 2017 at 03:14:30PM +0200, Stefan Sperling wrote:
>> > On Tue, May 09, 2017 at 09:47:14PM +0200, Michele Curti wrote:
>> > > On Tue, May 09, 2017 at 09:36:02PM +0200, Michele Curti wrote:
>> > > > On Tue, May 09, 2017 at 10:20:03AM +0200, Michele Curti wrote:
>> > > > > Hi all, I tried to upgrade to OpenBSD 6.1 on an Asus X205TA (bay
>> > > > > trail, 32 bit efi, 64 bit os) but the bootloader do not correctly
>> > > > > detect the internal disk.
>> > > > >
>> > > > > I also tried a fresh install, but things do not change.  Boot fails
>> > > > > and when I do a "machine diskinfo" I got a lot of "?" symbols (a video
>> > > > > here https://www.youtube.com/watch?v=fsomNX-oFTQ )
>> > > > >
>
> Thanks to yasuoka fix I just noted that using dp0 instead of dp
> changes the diskinfo disks order

Yes,

> # setting efi_bootdp = dp;
>
> Disk BlkSiz IoAlign Size Flags Checksum
> hd0 512 1 29GB 0x2 0xad4a42c3
> hd1 512 1 4MB 0x0 0x0
> hd2 512 1 4MB 0x0 0x0
>
> # setting efi_bootdp = dp0;
>
> Disk BlkSiz IoAlign Size Flags Checksum
> hd0 512 1 4MB 0x0 0x0
> hd1 512 1 4MB 0x0 0x0
> hd2 512 1 29GB 0x2 0xad4a42c3
>
> So I can use the stock bootloader without changes but I must do a
>
> boot> set device hd2a

I'm not clear whether we still have a problem.

Does the boot loader select the booted disk as "hd0" correctly?

If you booted from 29GB disk, "machine diskinfo" must be:

  Disk BlkSiz IoAlign Size Flags Checksum
  hd0 512 1 29GB 0x2 0xad4a42c3
  hd1 512 1 4MB 0x0 0x0
  hd2 512 1 4MB 0x0 0x0

--yasuoka

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Thu, May 11, 2017 at 05:48:34PM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> Thank you for your tests,
>
> On Thu, 11 May 2017 07:40:42 +0200
>
...

> > So I can use the stock bootloader without changes but I must do a
> >
> > boot> set device hd2a
>
> I'm not clear whether we still have a problem.
>
> Does the boot loader select the booted disk as "hd0" correctly?
>
> If you booted from 29GB disk, "machine diskinfo" must be:
>
>   Disk BlkSiz IoAlign Size Flags Checksum
>   hd0 512 1 29GB 0x2 0xad4a42c3
>   hd1 512 1 4MB 0x0 0x0
>   hd2 512 1 4MB 0x0 0x0
>

There is still a problem because I booted from the 29GB disk, the
bootloader selects hd0, but the machine diskinfo order is:
        Disk BlkSiz IoAlign Size Flags Checksum
        hd0 512 1 4MB 0x0 0x0
        hd1 512 1 4MB 0x0 0x0
        hd2 512 1 29GB 0x2 0xad4a42c3

I don't know if it is related but a problem is probably
(efiboot.c, line 87) here:
        status = EFI_CALL(BS->HandleProtocol, imgp->DeviceHandle,
            &devp_guid, (void **)&dp0);

where status is EFI_SUCCESS but dp0 remains NULL, so the following
for loop loops through invalid device paths until a causal
IsDevicePathEnd() true.

I not sure of what I'm saying because I have not investigated yet,
I have to read some EFI documentation first to understand what's
happening.


Thank you for the help,
Michele
 
> --yasuoka

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
In reply to this post by Patrick Wildt-3
On Wed, May 10, 2017 at 08:35:28PM +0200, Patrick Wildt wrote:

>
> I don't think this is the correct fix.  It might solve your issue, but I
> don't think it's completely right.  So EFI has those so called device
> paths.  A path is basically a list of nodes.  To compare two paths you
> need to compare the whole path and not just a single node of it.  If you
> store dp instead of dp0 you will basically only save a part of the path,
> not the full path.
>
> What you can do is print the full path of efi_bootdp like..
>
>     for (dp = efi_bootdp; !IsDevicePathEnd(dp);
>         dp = NextDevicePathNode(dp)) {
>             printf("%x %x - ", DevicePathType(dp), DevicePathSubType(dp));
>     }
>     printf("\n");
>
> And do the same for the DPs that are being put into the
> efi_device_path_cmp function.  That will at least print the types, but
> not the content of the nodes.  That's a start into figuring out why it
> does not correctly compare the paths.
>
> Maybe there's a bug in the compare code?

Sorry, only now I understood what you said... Got

2 1 - 1 1 - 1 5 - 4 1 -

(2 1) (2 1) da db cmp da db cmp diff bootdev 1
(2 1) (2 1) da db cmp da db cmp diff bootdev 1
(2 1) (2 1) da db cmp da db cmp diff bootdev 1

with the following diff


Index: efiboot/efiboot.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.17
diff -u -p -r1.17 efiboot.c
--- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
+++ efiboot/efiboot.c 11 May 2017 21:39:14 -0000
@@ -89,6 +89,8 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TA
  if (status == EFI_SUCCESS) {
  for (dp = dp0; !IsDevicePathEnd(dp);
     dp = NextDevicePathNode(dp)) {
+ printf("%x %x - ", DevicePathType(dp),
+       DevicePathSubType(dp));
  if (DevicePathType(dp) == MEDIA_DEVICE_PATH &&
     DevicePathSubType(dp) == MEDIA_HARDDRIVE_DP) {
  bios_bootdev = 0x80;
@@ -96,6 +98,7 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TA
  break;
  }
  }
+ printf("\n");
  }
 
 #ifdef __amd64__
@@ -199,10 +202,18 @@ efi_diskprobe(void)
     (void **)&dp);
  if (EFI_ERROR(status))
  goto next;
+ printf("(%x %x) (%x %x) ",
+ DevicePathType(efi_bootdp),
+ DevicePathSubType(efi_bootdp),
+ DevicePathType(dp),
+ DevicePathSubType(dp)
+ );
  if (!efi_device_path_cmp(efi_bootdp, dp, HARDWARE_DEVICE_PATH)&&
     !efi_device_path_cmp(efi_bootdp, dp, ACPI_DEVICE_PATH) &&
     !efi_device_path_cmp(efi_bootdp, dp, MESSAGING_DEVICE_PATH))
  bootdev = 1;
+
+ printf("bootdev %d\n", bootdev);
 next:
  if (bootdev)
  TAILQ_INSERT_HEAD(&efi_disklist, di, list);
@@ -222,12 +233,14 @@ efi_device_path_cmp(EFI_DEVICE_PATH *dpa
  for (dp = dpa; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
  if (DevicePathType(dp) == dptype) {
  dpt_a = dp;
+ printf("da ");
  break;
  }
  }
  for (dp = dpb; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
  if (DevicePathType(dp) == dptype) {
  dpt_b = dp;
+ printf("db ");
  break;
  }
  }
@@ -236,8 +249,11 @@ efi_device_path_cmp(EFI_DEVICE_PATH *dpa
  cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
  if (cmp)
  return (cmp);
+ printf("cmp ");
  return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
  }
+
+ printf("diff ");
 
  return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);
 }

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

YASUOKA Masahiko-3
On Thu, 11 May 2017 23:45:17 +0200
Michele Curti <[hidden email]> wrote:

> On Wed, May 10, 2017 at 08:35:28PM +0200, Patrick Wildt wrote:
>>
>> I don't think this is the correct fix.  It might solve your issue, but I
>> don't think it's completely right.  So EFI has those so called device
>> paths.  A path is basically a list of nodes.  To compare two paths you
>> need to compare the whole path and not just a single node of it.  If you
>> store dp instead of dp0 you will basically only save a part of the path,
>> not the full path.
>>
>> What you can do is print the full path of efi_bootdp like..
>>
>>     for (dp = efi_bootdp; !IsDevicePathEnd(dp);
>>         dp = NextDevicePathNode(dp)) {
>>             printf("%x %x - ", DevicePathType(dp), DevicePathSubType(dp));
>>     }
>>     printf("\n");
>>
>> And do the same for the DPs that are being put into the
>> efi_device_path_cmp function.  That will at least print the types, but
>> not the content of the nodes.  That's a start into figuring out why it
>> does not correctly compare the paths.
>>
>> Maybe there's a bug in the compare code?
>
> Sorry, only now I understood what you said... Got
>
> 2 1 - 1 1 - 1 5 - 4 1 -
>
> (2 1) (2 1) da db cmp da db cmp diff bootdev 1
> (2 1) (2 1) da db cmp da db cmp diff bootdev 1
> (2 1) (2 1) da db cmp da db cmp diff bootdev 1
>
> with the following diff

All disks are identical in HARDWARE_DEVICE_PATH and ACPI_DEVICE_PATH
and they all don't have MESSAGING_DEVICE_PATH.  So the boot loader
mistakenly treats all of them as the bootdisk..

I'd like to know whether there is a way to distigish those disks.  Can
you try the diff following?

diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c b/sys/arch/amd64/stand/efiboot/efiboot.c
index efa371f2ecd..891b6c75cc9 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -208,6 +208,14 @@ next:
  TAILQ_INSERT_HEAD(&efi_disklist, di, list);
  else
  TAILQ_INSERT_TAIL(&efi_disklist, di, list);
+
+ printf("%d\n", i);
+ for (; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
+ int ii;
+ for (ii = 0; ii < DevicePathNodeLength(dp); ii++)
+ printf("%x ", *((u_char *)dp + ii));
+ printf("\n");
+ }
  }
 
  free(handles, sz);

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Fri, May 12, 2017 at 11:05:13AM +0900, YASUOKA Masahiko wrote:

> On Thu, 11 May 2017 23:45:17 +0200
> Michele Curti <[hidden email]> wrote:
>
> All disks are identical in HARDWARE_DEVICE_PATH and ACPI_DEVICE_PATH
> and they all don't have MESSAGING_DEVICE_PATH.  So the boot loader
> mistakenly treats all of them as the bootdisk..
>
> I'd like to know whether there is a way to distigish those disks.  Can
> you try the diff following?
>
> diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c b/sys/arch/amd64/stand/efiboot/efiboot.c
> index efa371f2ecd..891b6c75cc9 100644
> --- a/sys/arch/amd64/stand/efiboot/efiboot.c
> +++ b/sys/arch/amd64/stand/efiboot/efiboot.c
> @@ -208,6 +208,14 @@ next:
>   TAILQ_INSERT_HEAD(&efi_disklist, di, list);
>   else
>   TAILQ_INSERT_TAIL(&efi_disklist, di, list);
> +
> + printf("%d\n", i);
> + for (; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
> + int ii;
> + for (ii = 0; ii < DevicePathNodeLength(dp); ii++)
> + printf("%x ", *((u_char *)dp + ii));
> + printf("\n");
> + }
>   }
>  
>   free(handles, sz);
>

0
2 1 c 0 d0 41 3 a 0 0 0 0
1 1 6 0 0 17
1 5 8 0 0 0 0 0
1
2 1 c 0 d0 41 3 a 0 0 0 0
1 1 6 0 0 17
1 5 8 0 1 0 0 0
2
2 1 c 0 d0 41 3 a 0 0 0 0
1 1 6 0 0 17
1 5 8 0 2 0 0 0


The efi_device_path_cmp() compares only a path node.
I tried the following diff just to see if I undestood something,
hd0 is now set corrctly to the 29GB disk.


Index: efiboot/efiboot.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.17
diff -u -p -r1.17 efiboot.c
--- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
+++ efiboot/efiboot.c 12 May 2017 05:23:21 -0000
@@ -233,10 +233,21 @@ efi_device_path_cmp(EFI_DEVICE_PATH *dpa
  }
 
  if (dpt_a && dpt_b) {
- cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
- if (cmp)
- return (cmp);
- return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
+ for (;!IsDevicePathEnd(dpt_a);) {
+ cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
+ if (cmp)
+ return (cmp);
+ cmp = memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a));
+ if (cmp)
+ return (cmp);
+ dpt_a = NextDevicePathNode(dpt_a);
+ dpt_b = NextDevicePathNode(dpt_b);
+ cmp = IsDevicePathEnd(dpt_a) - IsDevicePathEnd(dpt_b);
+ if (cmp)
+ return (cmp);
+ }
+
+ return 0;
  }
 
  return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Fri, May 12, 2017 at 07:27:45AM +0200, Michele Curti wrote:

>
> The efi_device_path_cmp() compares only a path node.
> I tried the following diff just to see if I undestood something,
> hd0 is now set corrctly to the 29GB disk.
>
>
> Index: efiboot/efiboot.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 efiboot.c
> --- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
> +++ efiboot/efiboot.c 12 May 2017 05:23:21 -0000
> @@ -233,10 +233,21 @@ efi_device_path_cmp(EFI_DEVICE_PATH *dpa
>   }
>  
>   if (dpt_a && dpt_b) {
> - cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
> - if (cmp)
> - return (cmp);
> - return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
> + for (;!IsDevicePathEnd(dpt_a);) {
> + cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
> + if (cmp)
> + return (cmp);
> + cmp = memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a));
> + if (cmp)
> + return (cmp);
> + dpt_a = NextDevicePathNode(dpt_a);
> + dpt_b = NextDevicePathNode(dpt_b);
> + cmp = IsDevicePathEnd(dpt_a) - IsDevicePathEnd(dpt_b);
> + if (cmp)
> + return (cmp);
> + }
> +
> + return 0;
>   }
>  
>   return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);

And something like this?


Index: efiboot/efiboot.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.17
diff -u -p -r1.17 efiboot.c
--- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
+++ efiboot/efiboot.c 12 May 2017 07:02:10 -0000
@@ -52,7 +52,7 @@ static EFI_GUID blkio_guid = BLOCK_IO_
 static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL;
 u_long efi_loadaddr;
 
-static int efi_device_path_cmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, int);
+static int efi_device_path_cmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *);
 static void efi_heap_init(void);
 static void efi_memprobe_internal(void);
 static void efi_video_init(void);
@@ -199,9 +199,7 @@ efi_diskprobe(void)
     (void **)&dp);
  if (EFI_ERROR(status))
  goto next;
- if (!efi_device_path_cmp(efi_bootdp, dp, HARDWARE_DEVICE_PATH)&&
-    !efi_device_path_cmp(efi_bootdp, dp, ACPI_DEVICE_PATH) &&
-    !efi_device_path_cmp(efi_bootdp, dp, MESSAGING_DEVICE_PATH))
+ if (efi_device_path_cmp(efi_bootdp, dp) == 0)
  bootdev = 1;
 next:
  if (bootdev)
@@ -214,32 +212,29 @@ next:
 }
 
 static int
-efi_device_path_cmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int dptype)
+efi_device_path_cmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb)
 {
- int cmp;
- EFI_DEVICE_PATH *dp, *dpt_a = NULL, *dpt_b = NULL;
+ int cmp;
 
- for (dp = dpa; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
- if (DevicePathType(dp) == dptype) {
- dpt_a = dp;
- break;
- }
- }
- for (dp = dpb; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
- if (DevicePathType(dp) == dptype) {
- dpt_b = dp;
- break;
- }
- }
-
- if (dpt_a && dpt_b) {
- cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
- if (cmp)
- return (cmp);
- return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
- }
+ cmp = IsDevicePathEnd(dpa) - IsDevicePathEnd(dpb);
+ if (cmp)
+ return cmp;
+ if (IsDevicePathEnd(dpa))
+ return 0;
+ cmp = DevicePathType(dpa) - DevicePathType(dpb);
+ if (cmp)
+ return cmp;
+ cmp = DevicePathSubType(dpa) - DevicePathSubType(dpb);
+ if (cmp)
+ return cmp;
+ cmp = DevicePathNodeLength(dpa) - DevicePathNodeLength(dpb);
+ if (cmp)
+ return cmp;
+ cmp = memcmp(dpa, dpb, DevicePathNodeLength(dpa));
+ if (cmp)
+ return cmp;
 
- return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);
+ return efi_device_path_cmp(NextDevicePathNode(dpa), NextDevicePathNode(dpb));
 }
 
 /***********************************************************************

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

YASUOKA Masahiko-3

On Fri, 12 May 2017 09:33:04 +0200
Michele Curti <[hidden email]> wrote:

> On Fri, May 12, 2017 at 07:27:45AM +0200, Michele Curti wrote:
>>
>> The efi_device_path_cmp() compares only a path node.
>> I tried the following diff just to see if I undestood something,
>> hd0 is now set corrctly to the 29GB disk.
>>
>>
>> Index: efiboot/efiboot.c
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
>> retrieving revision 1.17
>> diff -u -p -r1.17 efiboot.c
>> --- efiboot/efiboot.c 3 Mar 2017 08:56:18 -0000 1.17
>> +++ efiboot/efiboot.c 12 May 2017 05:23:21 -0000
>> @@ -233,10 +233,21 @@ efi_device_path_cmp(EFI_DEVICE_PATH *dpa
>>   }
>>  
>>   if (dpt_a && dpt_b) {
>> - cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
>> - if (cmp)
>> - return (cmp);
>> - return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
>> + for (;!IsDevicePathEnd(dpt_a);) {
>> + cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
>> + if (cmp)
>> + return (cmp);
>> + cmp = memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a));
>> + if (cmp)
>> + return (cmp);
>> + dpt_a = NextDevicePathNode(dpt_a);
>> + dpt_b = NextDevicePathNode(dpt_b);
>> + cmp = IsDevicePathEnd(dpt_a) - IsDevicePathEnd(dpt_b);
>> + if (cmp)
>> + return (cmp);
>> + }
>> +
>> + return 0;
>>   }
>>  
>>   return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);
>
> And something like this?

Yes.  What we need to do is comparing the device path node before
MEDIA_DEVICE_PATH type.  So I rewrote it like

  media_idx = device_path_index(rootdp, MEDIA_DEVICE_PATH);
  for (...)
    if (device_path_ncmp(rootdp, dp, media_idx) == 0)
       bootdev = 1;

ok? comment?

diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c b/sys/arch/amd64/stand/efiboot/efiboot.c
index efa371f2ecd..b78ca2b350e 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -52,7 +52,9 @@ static EFI_GUID blkio_guid = BLOCK_IO_PROTOCOL;
 static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL;
 u_long efi_loadaddr;
 
-static int efi_device_path_cmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, int);
+static int efi_device_path_index(EFI_DEVICE_PATH *dp, int);
+static int efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *,
+    int);
 static void efi_heap_init(void);
 static void efi_memprobe_internal(void);
 static void efi_video_init(void);
@@ -159,7 +161,7 @@ struct disklist_lh efi_disklist;
 void
 efi_diskprobe(void)
 {
- int i, bootdev;
+ int i, media_idx, bootdev;
  UINTN sz;
  EFI_STATUS status;
  EFI_HANDLE *handles = NULL;
@@ -180,6 +182,8 @@ efi_diskprobe(void)
  if (handles == NULL || EFI_ERROR(status))
  panic("BS->LocateHandle() returns %d", status);
 
+ media_idx = efi_device_path_index(efi_bootdp, MEDIA_DEVICE_PATH);
+
  for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
  bootdev = 0;
  status = EFI_CALL(BS->HandleProtocol, handles[i], &blkio_guid,
@@ -199,9 +203,7 @@ efi_diskprobe(void)
     (void **)&dp);
  if (EFI_ERROR(status))
  goto next;
- if (!efi_device_path_cmp(efi_bootdp, dp, HARDWARE_DEVICE_PATH)&&
-    !efi_device_path_cmp(efi_bootdp, dp, ACPI_DEVICE_PATH) &&
-    !efi_device_path_cmp(efi_bootdp, dp, MESSAGING_DEVICE_PATH))
+ if (efi_device_path_ncmp(efi_bootdp, dp, media_idx) == 0)
  bootdev = 1;
 next:
  if (bootdev)
@@ -214,32 +216,39 @@ next:
 }
 
 static int
-efi_device_path_cmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int dptype)
+efi_device_path_index(EFI_DEVICE_PATH *dp, int dptype)
 {
- int cmp;
- EFI_DEVICE_PATH *dp, *dpt_a = NULL, *dpt_b = NULL;
+ int idx = 0;
 
- for (dp = dpa; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
- if (DevicePathType(dp) == dptype) {
- dpt_a = dp;
+ for (idx = 0; !IsDevicePathEnd(dp); idx++) {
+ if (DevicePathType(dp) == dptype)
  break;
- }
- }
- for (dp = dpb; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
- if (DevicePathType(dp) == dptype) {
- dpt_b = dp;
- break;
- }
+ dp = NextDevicePathNode(dp);
  }
 
- if (dpt_a && dpt_b) {
- cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
+ return (idx);
+}
+
+static int
+efi_device_path_ncmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int idx)
+{
+ int i, cmp;
+
+ for (i = 0; i < idx &&
+    !IsDevicePathEnd(dpa) && !IsDevicePathEnd(dpb); i++) {
+ cmp = DevicePathNodeLength(dpa) - DevicePathNodeLength(dpb);
  if (cmp)
  return (cmp);
- return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
+ cmp = memcmp(dpa, dpb, DevicePathNodeLength(dpa));
+ if (cmp)
+ return (cmp);
+ dpa = NextDevicePathNode(dpa);
+ dpb = NextDevicePathNode(dpb);
  }
+ if (i < idx)
+ return (-1);
 
- return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);
+ return (0);
 }
 
 /***********************************************************************

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Fri, May 12, 2017 at 06:01:35PM +0900, YASUOKA Masahiko wrote:

>
> > And something like this?
>
> Yes.  What we need to do is comparing the device path node before
> MEDIA_DEVICE_PATH type.  So I rewrote it like
>
>   media_idx = device_path_index(rootdp, MEDIA_DEVICE_PATH);
>   for (...)
>     if (device_path_ncmp(rootdp, dp, media_idx) == 0)
>        bootdev = 1;
>
> ok? comment?
>

The device order is correct, the laptop boots without problems,
but executing "machine diskinfo" prints a source file now..

https://www.youtube.com/watch?v=EUao9Fq5Bww

It seems gnu/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp


Michele

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

YASUOKA Masahiko-3
On Fri, 12 May 2017 16:15:52 +0200
Michele Curti <[hidden email]> wrote:

> On Fri, May 12, 2017 at 06:01:35PM +0900, YASUOKA Masahiko wrote:
>> > And something like this?
>>
>> Yes.  What we need to do is comparing the device path node before
>> MEDIA_DEVICE_PATH type.  So I rewrote it like
>>
>>   media_idx = device_path_index(rootdp, MEDIA_DEVICE_PATH);
>>   for (...)
>>     if (device_path_ncmp(rootdp, dp, media_idx) == 0)
>>        bootdev = 1;
>>
>> ok? comment?
>>
>
> The device order is correct, the laptop boots without problems,

Thank you for your test.

> but executing "machine diskinfo" prints a source file now..
>
> https://www.youtube.com/watch?v=EUao9Fq5Bww
>
> It seems gnu/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp

It seems the problem which had been fixed previous is happening again.
Does efidev.c have following $OpenBSD$?

  /*      $OpenBSD: efidev.c,v 1.25 2017/05/11 01:37:24 yasuoka Exp $     */


Also let me update the diff.

diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c b/sys/arch/amd64/stand/efiboot/efiboot.c
index efa371f2ecd..19df6b1dc64 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -52,7 +52,9 @@ static EFI_GUID blkio_guid = BLOCK_IO_PROTOCOL;
 static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL;
 u_long efi_loadaddr;
 
-static int efi_device_path_cmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, int);
+static int efi_device_path_depth(EFI_DEVICE_PATH *dp, int);
+static int efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *,
+    int);
 static void efi_heap_init(void);
 static void efi_memprobe_internal(void);
 static void efi_video_init(void);
@@ -159,7 +161,7 @@ struct disklist_lh efi_disklist;
 void
 efi_diskprobe(void)
 {
- int i, bootdev;
+ int i, bootdev = 0, depth = -1;
  UINTN sz;
  EFI_STATUS status;
  EFI_HANDLE *handles = NULL;
@@ -180,8 +182,10 @@ efi_diskprobe(void)
  if (handles == NULL || EFI_ERROR(status))
  panic("BS->LocateHandle() returns %d", status);
 
+ if (efi_bootdp != NULL)
+ depth = efi_device_path_depth(efi_bootdp, MEDIA_DEVICE_PATH);
+
  for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
- bootdev = 0;
  status = EFI_CALL(BS->HandleProtocol, handles[i], &blkio_guid,
     (void **)&blkio);
  if (EFI_ERROR(status))
@@ -193,53 +197,57 @@ efi_diskprobe(void)
  di = alloc(sizeof(struct diskinfo));
  efid_init(di, blkio);
 
- if (efi_bootdp == NULL)
+ if (efi_bootdp == NULL || depth == -1 || bootdev != 0)
  goto next;
  status = EFI_CALL(BS->HandleProtocol, handles[i], &devp_guid,
     (void **)&dp);
  if (EFI_ERROR(status))
  goto next;
- if (!efi_device_path_cmp(efi_bootdp, dp, HARDWARE_DEVICE_PATH)&&
-    !efi_device_path_cmp(efi_bootdp, dp, ACPI_DEVICE_PATH) &&
-    !efi_device_path_cmp(efi_bootdp, dp, MESSAGING_DEVICE_PATH))
+ if (efi_device_path_ncmp(efi_bootdp, dp, depth) == 0) {
+ TAILQ_INSERT_HEAD(&efi_disklist, di, list);
  bootdev = 1;
+ continue;
+ }
 next:
- if (bootdev)
- TAILQ_INSERT_HEAD(&efi_disklist, di, list);
- else
- TAILQ_INSERT_TAIL(&efi_disklist, di, list);
+ TAILQ_INSERT_TAIL(&efi_disklist, di, list);
  }
 
  free(handles, sz);
 }
 
 static int
-efi_device_path_cmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int dptype)
+efi_device_path_depth(EFI_DEVICE_PATH *dp, int dptype)
 {
- int cmp;
- EFI_DEVICE_PATH *dp, *dpt_a = NULL, *dpt_b = NULL;
+ int i;
 
- for (dp = dpa; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
- if (DevicePathType(dp) == dptype) {
- dpt_a = dp;
- break;
- }
- }
- for (dp = dpb; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
- if (DevicePathType(dp) == dptype) {
- dpt_b = dp;
- break;
- }
+ for (i = 0; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp), i++) {
+ if (DevicePathType(dp) == dptype)
+ return (i);
  }
 
- if (dpt_a && dpt_b) {
- cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
+ return (-1);
+}
+
+static int
+efi_device_path_ncmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int deptn)
+{
+ int i, cmp;
+
+ for (i = 0; i < deptn; i++) {
+ if (IsDevicePathEnd(dpa) || IsDevicePathEnd(dpb))
+ return ((IsDevicePathEnd(dpa) && IsDevicePathEnd(dpb))
+    ? 0 : (IsDevicePathEnd(dpa))? -1 : 1);
+ cmp = DevicePathNodeLength(dpa) - DevicePathNodeLength(dpb);
+ if (cmp)
+ return (cmp);
+ cmp = memcmp(dpa, dpb, DevicePathNodeLength(dpa));
  if (cmp)
  return (cmp);
- return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
+ dpa = NextDevicePathNode(dpa);
+ dpb = NextDevicePathNode(dpb);
  }
 
- return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);
+ return (0);
 }
 
 /***********************************************************************




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OpenBSD 6.1: BOOTIA32 3.32 issue

Michele Curti
On Sun, May 14, 2017 at 07:31:09PM +0900, YASUOKA Masahiko wrote:
> On Fri, 12 May 2017 16:15:52 +0200
>
> It seems the problem which had been fixed previous is happening again.
> Does efidev.c have following $OpenBSD$?
>
>   /*      $OpenBSD: efidev.c,v 1.25 2017/05/11 01:37:24 yasuoka Exp $     */
>

You are right, I wasn't following HEAD, so cvs update did not
retrieved the latest version.. Sorry for the trouble.

>
> Also let me update the diff.

Works for me :)

Thank you,
Michele

>
> diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c b/sys/arch/amd64/stand/efiboot/efiboot.c
> index efa371f2ecd..19df6b1dc64 100644
> --- a/sys/arch/amd64/stand/efiboot/efiboot.c
> +++ b/sys/arch/amd64/stand/efiboot/efiboot.c
> @@ -52,7 +52,9 @@ static EFI_GUID blkio_guid = BLOCK_IO_PROTOCOL;
>  static EFI_GUID devp_guid = DEVICE_PATH_PROTOCOL;
>  u_long efi_loadaddr;
>  
> -static int efi_device_path_cmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *, int);
> +static int efi_device_path_depth(EFI_DEVICE_PATH *dp, int);
> +static int efi_device_path_ncmp(EFI_DEVICE_PATH *, EFI_DEVICE_PATH *,
> +    int);
>  static void efi_heap_init(void);
>  static void efi_memprobe_internal(void);
>  static void efi_video_init(void);
> @@ -159,7 +161,7 @@ struct disklist_lh efi_disklist;
>  void
>  efi_diskprobe(void)
>  {
> - int i, bootdev;
> + int i, bootdev = 0, depth = -1;
>   UINTN sz;
>   EFI_STATUS status;
>   EFI_HANDLE *handles = NULL;
> @@ -180,8 +182,10 @@ efi_diskprobe(void)
>   if (handles == NULL || EFI_ERROR(status))
>   panic("BS->LocateHandle() returns %d", status);
>  
> + if (efi_bootdp != NULL)
> + depth = efi_device_path_depth(efi_bootdp, MEDIA_DEVICE_PATH);
> +
>   for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
> - bootdev = 0;
>   status = EFI_CALL(BS->HandleProtocol, handles[i], &blkio_guid,
>      (void **)&blkio);
>   if (EFI_ERROR(status))
> @@ -193,53 +197,57 @@ efi_diskprobe(void)
>   di = alloc(sizeof(struct diskinfo));
>   efid_init(di, blkio);
>  
> - if (efi_bootdp == NULL)
> + if (efi_bootdp == NULL || depth == -1 || bootdev != 0)
>   goto next;
>   status = EFI_CALL(BS->HandleProtocol, handles[i], &devp_guid,
>      (void **)&dp);
>   if (EFI_ERROR(status))
>   goto next;
> - if (!efi_device_path_cmp(efi_bootdp, dp, HARDWARE_DEVICE_PATH)&&
> -    !efi_device_path_cmp(efi_bootdp, dp, ACPI_DEVICE_PATH) &&
> -    !efi_device_path_cmp(efi_bootdp, dp, MESSAGING_DEVICE_PATH))
> + if (efi_device_path_ncmp(efi_bootdp, dp, depth) == 0) {
> + TAILQ_INSERT_HEAD(&efi_disklist, di, list);
>   bootdev = 1;
> + continue;
> + }
>  next:
> - if (bootdev)
> - TAILQ_INSERT_HEAD(&efi_disklist, di, list);
> - else
> - TAILQ_INSERT_TAIL(&efi_disklist, di, list);
> + TAILQ_INSERT_TAIL(&efi_disklist, di, list);
>   }
>  
>   free(handles, sz);
>  }
>  
>  static int
> -efi_device_path_cmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int dptype)
> +efi_device_path_depth(EFI_DEVICE_PATH *dp, int dptype)
>  {
> - int cmp;
> - EFI_DEVICE_PATH *dp, *dpt_a = NULL, *dpt_b = NULL;
> + int i;
>  
> - for (dp = dpa; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
> - if (DevicePathType(dp) == dptype) {
> - dpt_a = dp;
> - break;
> - }
> - }
> - for (dp = dpb; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) {
> - if (DevicePathType(dp) == dptype) {
> - dpt_b = dp;
> - break;
> - }
> + for (i = 0; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp), i++) {
> + if (DevicePathType(dp) == dptype)
> + return (i);
>   }
>  
> - if (dpt_a && dpt_b) {
> - cmp = DevicePathNodeLength(dpt_a) - DevicePathNodeLength(dpt_b);
> + return (-1);
> +}
> +
> +static int
> +efi_device_path_ncmp(EFI_DEVICE_PATH *dpa, EFI_DEVICE_PATH *dpb, int deptn)
> +{
> + int i, cmp;
> +
> + for (i = 0; i < deptn; i++) {
> + if (IsDevicePathEnd(dpa) || IsDevicePathEnd(dpb))
> + return ((IsDevicePathEnd(dpa) && IsDevicePathEnd(dpb))
> +    ? 0 : (IsDevicePathEnd(dpa))? -1 : 1);
> + cmp = DevicePathNodeLength(dpa) - DevicePathNodeLength(dpb);
> + if (cmp)
> + return (cmp);
> + cmp = memcmp(dpa, dpb, DevicePathNodeLength(dpa));
>   if (cmp)
>   return (cmp);
> - return (memcmp(dpt_a, dpt_b, DevicePathNodeLength(dpt_a)));
> + dpa = NextDevicePathNode(dpa);
> + dpb = NextDevicePathNode(dpb);
>   }
>  
> - return ((uintptr_t)dpt_a - (uintptr_t)dpt_b);
> + return (0);
>  }
>  
>  /***********************************************************************
>
>
>
>

Loading...