[edk2-devel] PING [PATCH 1/1 v2] EmulatorPkg: fixes for NetBSD

tlaronde@polynum.com posted 1 patch 1 year, 3 months ago
Failed in applying to current master (apply log)
EmulatorPkg/Unix/Host/BlockIo.c | 16 +++++++++++++++-
EmulatorPkg/Unix/Host/Host.c    |  7 +++++--
EmulatorPkg/Unix/Host/Host.h    |  7 ++++++-
3 files changed, 26 insertions(+), 4 deletions(-)
[edk2-devel] PING [PATCH 1/1 v2] EmulatorPkg: fixes for NetBSD
Posted by tlaronde@polynum.com 1 year, 3 months ago
Hello,

Ping'ing for this patch sent on November 2022.

The patch is trivial. Can someone review it or eventually merge it if OK?

TIA,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                    http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97986): https://edk2.groups.io/g/devel/message/97986
Mute This Topic: https://groups.io/mt/96070144/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Signed-off-by: Thierry Laronde <tlaronde@polynum.com>

Fixes for compilation on NetBSD.

---
 EmulatorPkg/Unix/Host/BlockIo.c | 16 +++++++++++++++-
 EmulatorPkg/Unix/Host/Host.c    |  7 +++++--
 EmulatorPkg/Unix/Host/Host.h    |  7 ++++++-
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/EmulatorPkg/Unix/Host/BlockIo.c b/EmulatorPkg/Unix/Host/BlockIo.c
index cf2d6b4cda..57b4c8d3be 100644
--- a/EmulatorPkg/Unix/Host/BlockIo.c
+++ b/EmulatorPkg/Unix/Host/BlockIo.c
@@ -133,6 +133,20 @@ EmuBlockIoOpenDevice (
 
       ioctl (Private->fd, DKIOCGETMAXBLOCKCOUNTWRITE, &Private->Media->OptimalTransferLengthGranularity);
     }
+ #elif _NETBSD_SOURCE
+    {
+      UINTN  BlockSize;
+      off_t  DiskSize;
+
+      if (ioctl (Private->fd,  DIOCGSECTORSIZE, &BlockSize) == 0) {
+        Private->Media->BlockSize = BlockSize;
+      }
+
+      if (ioctl (Private->fd, DIOCGMEDIASIZE, &DiskSize) == 0) {
+        Private->NumberOfBlocks   = DivU64x32 (DiskSize, (UINT32)BlockSize);
+        Private->Media->LastBlock = Private->NumberOfBlocks - 1;
+      }
+    }
  #else
     {
       size_t  BlockSize;
@@ -154,7 +168,7 @@ EmuBlockIoOpenDevice (
     Private->Media->LastBlock = Private->NumberOfBlocks - 1;
 
     if (fstatfs (Private->fd, &buf) == 0) {
- #if __APPLE__
+ #if __APPLE__ || _NETBSD_SOURCE
       Private->Media->OptimalTransferLengthGranularity = buf.f_iosize/buf.f_bsize;
  #else
       Private->Media->OptimalTransferLengthGranularity = buf.f_bsize/buf.f_bsize;
diff --git a/EmulatorPkg/Unix/Host/Host.c b/EmulatorPkg/Unix/Host/Host.c
index 38c01c84af..1700a4b60f 100644
--- a/EmulatorPkg/Unix/Host/Host.c
+++ b/EmulatorPkg/Unix/Host/Host.c
@@ -12,6 +12,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define MAP_ANONYMOUS  MAP_ANON
 #endif
 
+#define MAP_ANON_FD_ (-1)
+
 //
 // Globals
 //
@@ -187,7 +189,7 @@ main (
   //
   InitialStackMemorySize = STACK_SIZE;
   InitialStackMemory     = (UINTN)MapMemory (
-                                    0,
+                                    MAP_ANON_FD_,
                                     (UINT32)InitialStackMemorySize,
                                     PROT_READ | PROT_WRITE | PROT_EXEC,
                                     MAP_ANONYMOUS | MAP_PRIVATE
@@ -348,6 +350,7 @@ MapMemory (
   while ((!isAligned) && (base != 0)) {
     res = mmap ((void *)base, length, prot, flags, fd, 0);
     if (res == MAP_FAILED) {
+      perror("MapMemory");
       return NULL;
     }
 
@@ -640,7 +643,7 @@ SecUnixPeiAutoScan (
 
   *MemoryBase = 0;
   res         = MapMemory (
-                  0,
+                  MAP_ANON_FD_,
                   gSystemMemory[Index].Size,
                   PROT_READ | PROT_WRITE | PROT_EXEC,
                   MAP_PRIVATE | MAP_ANONYMOUS
diff --git a/EmulatorPkg/Unix/Host/Host.h b/EmulatorPkg/Unix/Host/Host.h
index 0c81cdfc01..0de925adaf 100644
--- a/EmulatorPkg/Unix/Host/Host.h
+++ b/EmulatorPkg/Unix/Host/Host.h
@@ -31,6 +31,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #if __CYGWIN__
   #include <sys/dirent.h>
+#elif _NETBSD_SOURCE
+  #include <dirent.h>
 #else
   #include <sys/dir.h>
 #endif
@@ -55,7 +57,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <net/if.h>
 #include <ifaddrs.h>
 
-#ifdef __APPLE__
+#if defined(__APPLE__)
   #include <net/if_dl.h>
   #include <net/bpf.h>
   #include <sys/param.h>
@@ -65,6 +67,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
   #ifndef _Bool
 #define _Bool  char  // for clang debug
   #endif
+#elif defined(_NETBSD_SOURCE)
+  #define statfs statvfs
+  #define fstatfs fstatvfs
 #else
   #include <termio.h>
   #include <sys/vfs.h>
-- 
2.34.1

-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                    http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96614): https://edk2.groups.io/g/devel/message/96614
Mute This Topic: https://groups.io/mt/95238792/7312392
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [tlaronde@polynum.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] PING [PATCH 1/1 v2] EmulatorPkg: fixes for NetBSD
Posted by Pedro Falcato 1 year, 3 months ago
On Thu, Jan 5, 2023 at 11:50 AM <tlaronde@polynum.com> wrote:

> Hello,
>
> Ping'ing for this patch sent on November 2022.
>
> The patch is trivial. Can someone review it or eventually merge it if OK?
>
> TIA,
> --
>         Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
>                      http://www.kergis.com/
>                     http://kertex.kergis.com/
> Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C
>
>
>
> ---------- Forwarded message ----------
> From: tlaronde@polynum.com
> To: devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>, Ray Ni <ray.ni@intel.com>, Pedro
> Falcato <pedro.falcato@gmail.com>
> Bcc:
> Date: Thu, 24 Nov 2022 15:46:59 +0100
> Subject: [edk2-devel] [PATCH 1/1 v2] EmulatorPkg: fixes for NetBSD
> Signed-off-by: Thierry Laronde <tlaronde@polynum.com>
>
> Fixes for compilation on NetBSD.
>
> ---
>  EmulatorPkg/Unix/Host/BlockIo.c | 16 +++++++++++++++-
>  EmulatorPkg/Unix/Host/Host.c    |  7 +++++--
>  EmulatorPkg/Unix/Host/Host.h    |  7 ++++++-
>  3 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/EmulatorPkg/Unix/Host/BlockIo.c
> b/EmulatorPkg/Unix/Host/BlockIo.c
> index cf2d6b4cda..57b4c8d3be 100644
> --- a/EmulatorPkg/Unix/Host/BlockIo.c
> +++ b/EmulatorPkg/Unix/Host/BlockIo.c
> @@ -133,6 +133,20 @@ EmuBlockIoOpenDevice (
>
>        ioctl (Private->fd, DKIOCGETMAXBLOCKCOUNTWRITE,
> &Private->Media->OptimalTransferLengthGranularity);
>      }
> + #elif _NETBSD_SOURCE
> +    {
> +      UINTN  BlockSize;
> +      off_t  DiskSize;
> +
> +      if (ioctl (Private->fd,  DIOCGSECTORSIZE, &BlockSize) == 0) {
> +        Private->Media->BlockSize = BlockSize;
> +      }
> +
> +      if (ioctl (Private->fd, DIOCGMEDIASIZE, &DiskSize) == 0) {
> +        Private->NumberOfBlocks   = DivU64x32 (DiskSize,
> (UINT32)BlockSize);
> +        Private->Media->LastBlock = Private->NumberOfBlocks - 1;
> +      }
> +    }
>   #else
>      {
>        size_t  BlockSize;
> @@ -154,7 +168,7 @@ EmuBlockIoOpenDevice (
>      Private->Media->LastBlock = Private->NumberOfBlocks - 1;
>
>      if (fstatfs (Private->fd, &buf) == 0) {
> - #if __APPLE__
> + #if __APPLE__ || _NETBSD_SOURCE
>
Isn't this the wrong test? Shouldn't you test for __NetBSD__ instead? AIUI
_NETBSD_SOURCE is just a feature test macro like _GNU_SOURCE, right?

>        Private->Media->OptimalTransferLengthGranularity =
> buf.f_iosize/buf.f_bsize;
>   #else
>        Private->Media->OptimalTransferLengthGranularity =
> buf.f_bsize/buf.f_bsize;
> diff --git a/EmulatorPkg/Unix/Host/Host.c b/EmulatorPkg/Unix/Host/Host.c
> index 38c01c84af..1700a4b60f 100644
> --- a/EmulatorPkg/Unix/Host/Host.c
> +++ b/EmulatorPkg/Unix/Host/Host.c
> @@ -12,6 +12,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define MAP_ANONYMOUS  MAP_ANON
>  #endif
>
> +#define MAP_ANON_FD_ (-1)
>
Again, I don't think you need a define for this. At most you could just
stick a -1 down there with a comment like:
// We're required to pass -1 for anonymous mappings on BSD-like systems

> +
>  //
>  // Globals
>  //
> @@ -187,7 +189,7 @@ main (
>    //
>    InitialStackMemorySize = STACK_SIZE;
>    InitialStackMemory     = (UINTN)MapMemory (
> -                                    0,
> +                                    MAP_ANON_FD_,
>                                      (UINT32)InitialStackMemorySize,
>                                      PROT_READ | PROT_WRITE | PROT_EXEC,
>                                      MAP_ANONYMOUS | MAP_PRIVATE
> @@ -348,6 +350,7 @@ MapMemory (
>    while ((!isAligned) && (base != 0)) {
>      res = mmap ((void *)base, length, prot, flags, fd, 0);
>      if (res == MAP_FAILED) {
> +      perror("MapMemory");
>        return NULL;
>      }
>
> @@ -640,7 +643,7 @@ SecUnixPeiAutoScan (
>
>    *MemoryBase = 0;
>    res         = MapMemory (
> -                  0,
> +                  MAP_ANON_FD_,
>                    gSystemMemory[Index].Size,
>                    PROT_READ | PROT_WRITE | PROT_EXEC,
>                    MAP_PRIVATE | MAP_ANONYMOUS
> diff --git a/EmulatorPkg/Unix/Host/Host.h b/EmulatorPkg/Unix/Host/Host.h
> index 0c81cdfc01..0de925adaf 100644
> --- a/EmulatorPkg/Unix/Host/Host.h
> +++ b/EmulatorPkg/Unix/Host/Host.h
> @@ -31,6 +31,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #if __CYGWIN__
>    #include <sys/dirent.h>
> +#elif _NETBSD_SOURCE
>
Same as above.

> +  #include <dirent.h>
>  #else
>    #include <sys/dir.h>
>  #endif
> @@ -55,7 +57,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <net/if.h>
>  #include <ifaddrs.h>
>
> -#ifdef __APPLE__
> +#if defined(__APPLE__)
>
   #include <net/if_dl.h>
>    #include <net/bpf.h>
>    #include <sys/param.h>
> @@ -65,6 +67,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>    #ifndef _Bool
>  #define _Bool  char  // for clang debug
>    #endif
> +#elif defined(_NETBSD_SOURCE)
> +  #define statfs statvfs
> +  #define fstatfs fstatvfs
>  #else
>    #include <termio.h>
>    #include <sys/vfs.h>
> --
> 2.34.1
>
> --
>         Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
>                      http://www.kergis.com/
>                     http://kertex.kergis.com/
> Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C
>

I can give you my Ack or RB after this, but FWIW I'm not a maintainer for
EmulatorPkg so you'll still need appropriate reviews from the package
mainainers.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98039): https://edk2.groups.io/g/devel/message/98039
Mute This Topic: https://groups.io/mt/96070144/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] PING [PATCH 1/1 v2] EmulatorPkg: fixes for NetBSD
Posted by tlaronde@polynum.com 1 year, 3 months ago
Le Thu, Jan 05, 2023 at 11:40:43PM +0000, Pedro Falcato a écrit :
> On Thu, Jan 5, 2023 at 11:50 AM <tlaronde@polynum.com> wrote:
> 
> > Hello,
> >
> > Ping'ing for this patch sent on November 2022.
> >
> > The patch is trivial. Can someone review it or eventually merge it if OK?
> >
> > TIA,
> > --
> >         Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
> >                      http://www.kergis.com/
> >                     http://kertex.kergis.com/
> > Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C
> >
> >
> >
> > ---------- Forwarded message ----------
> > From: tlaronde@polynum.com
> > To: devel@edk2.groups.io
> > Cc: Andrew Fish <afish@apple.com>, Ray Ni <ray.ni@intel.com>, Pedro
> > Falcato <pedro.falcato@gmail.com>
> > Bcc:
> > Date: Thu, 24 Nov 2022 15:46:59 +0100
> > Subject: [edk2-devel] [PATCH 1/1 v2] EmulatorPkg: fixes for NetBSD
> > Signed-off-by: Thierry Laronde <tlaronde@polynum.com>
> >
> > Fixes for compilation on NetBSD.
> >
> > ---
> >  EmulatorPkg/Unix/Host/BlockIo.c | 16 +++++++++++++++-
> >  EmulatorPkg/Unix/Host/Host.c    |  7 +++++--
> >  EmulatorPkg/Unix/Host/Host.h    |  7 ++++++-
> >  3 files changed, 26 insertions(+), 4 deletions(-)
> >
> > diff --git a/EmulatorPkg/Unix/Host/BlockIo.c
> > b/EmulatorPkg/Unix/Host/BlockIo.c
> > index cf2d6b4cda..57b4c8d3be 100644
> > --- a/EmulatorPkg/Unix/Host/BlockIo.c
> > +++ b/EmulatorPkg/Unix/Host/BlockIo.c
> > @@ -133,6 +133,20 @@ EmuBlockIoOpenDevice (
> >
> >        ioctl (Private->fd, DKIOCGETMAXBLOCKCOUNTWRITE,
> > &Private->Media->OptimalTransferLengthGranularity);
> >      }
> > + #elif _NETBSD_SOURCE
> > +    {
> > +      UINTN  BlockSize;
> > +      off_t  DiskSize;
> > +
> > +      if (ioctl (Private->fd,  DIOCGSECTORSIZE, &BlockSize) == 0) {
> > +        Private->Media->BlockSize = BlockSize;
> > +      }
> > +
> > +      if (ioctl (Private->fd, DIOCGMEDIASIZE, &DiskSize) == 0) {
> > +        Private->NumberOfBlocks   = DivU64x32 (DiskSize,
> > (UINT32)BlockSize);
> > +        Private->Media->LastBlock = Private->NumberOfBlocks - 1;
> > +      }
> > +    }
> >   #else
> >      {
> >        size_t  BlockSize;
> > @@ -154,7 +168,7 @@ EmuBlockIoOpenDevice (
> >      Private->Media->LastBlock = Private->NumberOfBlocks - 1;
> >
> >      if (fstatfs (Private->fd, &buf) == 0) {
> > - #if __APPLE__
> > + #if __APPLE__ || _NETBSD_SOURCE
> >
> Isn't this the wrong test? Shouldn't you test for __NetBSD__ instead? AIUI
> _NETBSD_SOURCE is just a feature test macro like _GNU_SOURCE, right?

You're right: I was focusing on "not POSIXly defined" and grapping to a
featuretest macro at the same level. __NetBSD__ will do.

> 
> >        Private->Media->OptimalTransferLengthGranularity =
> > buf.f_iosize/buf.f_bsize;
> >   #else
> >        Private->Media->OptimalTransferLengthGranularity =
> > buf.f_bsize/buf.f_bsize;
> > diff --git a/EmulatorPkg/Unix/Host/Host.c b/EmulatorPkg/Unix/Host/Host.c
> > index 38c01c84af..1700a4b60f 100644
> > --- a/EmulatorPkg/Unix/Host/Host.c
> > +++ b/EmulatorPkg/Unix/Host/Host.c
> > @@ -12,6 +12,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #define MAP_ANONYMOUS  MAP_ANON
> >  #endif
> >
> > +#define MAP_ANON_FD_ (-1)
> >
> Again, I don't think you need a define for this. At most you could just
> stick a -1 down there with a comment like:
> // We're required to pass -1 for anonymous mappings on BSD-like systems
> 

OK. I will suppress the macro and go to a comment.

> > +
> >  //
> >  // Globals
> >  //
> > @@ -187,7 +189,7 @@ main (
> >    //
> >    InitialStackMemorySize = STACK_SIZE;
> >    InitialStackMemory     = (UINTN)MapMemory (
> > -                                    0,
> > +                                    MAP_ANON_FD_,
> >                                      (UINT32)InitialStackMemorySize,
> >                                      PROT_READ | PROT_WRITE | PROT_EXEC,
> >                                      MAP_ANONYMOUS | MAP_PRIVATE
> > @@ -348,6 +350,7 @@ MapMemory (
> >    while ((!isAligned) && (base != 0)) {
> >      res = mmap ((void *)base, length, prot, flags, fd, 0);
> >      if (res == MAP_FAILED) {
> > +      perror("MapMemory");
> >        return NULL;
> >      }
> >
> > @@ -640,7 +643,7 @@ SecUnixPeiAutoScan (
> >
> >    *MemoryBase = 0;
> >    res         = MapMemory (
> > -                  0,
> > +                  MAP_ANON_FD_,
> >                    gSystemMemory[Index].Size,
> >                    PROT_READ | PROT_WRITE | PROT_EXEC,
> >                    MAP_PRIVATE | MAP_ANONYMOUS
> > diff --git a/EmulatorPkg/Unix/Host/Host.h b/EmulatorPkg/Unix/Host/Host.h
> > index 0c81cdfc01..0de925adaf 100644
> > --- a/EmulatorPkg/Unix/Host/Host.h
> > +++ b/EmulatorPkg/Unix/Host/Host.h
> > @@ -31,6 +31,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  #if __CYGWIN__
> >    #include <sys/dirent.h>
> > +#elif _NETBSD_SOURCE
> >
> Same as above.
> 
> > +  #include <dirent.h>
> >  #else
> >    #include <sys/dir.h>
> >  #endif
> > @@ -55,7 +57,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #include <net/if.h>
> >  #include <ifaddrs.h>
> >
> > -#ifdef __APPLE__
> > +#if defined(__APPLE__)
> >
>    #include <net/if_dl.h>
> >    #include <net/bpf.h>
> >    #include <sys/param.h>
> > @@ -65,6 +67,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >    #ifndef _Bool
> >  #define _Bool  char  // for clang debug
> >    #endif
> > +#elif defined(_NETBSD_SOURCE)
> > +  #define statfs statvfs
> > +  #define fstatfs fstatvfs
> >  #else
> >    #include <termio.h>
> >    #include <sys/vfs.h>
> > --
> > 2.34.1
> >
> 
> I can give you my Ack or RB after this, but FWIW I'm not a maintainer for
> EmulatorPkg so you'll still need appropriate reviews from the package
> mainainers.

OK. Thanks! I will prepare a v3 and let the maintainers decide.

Best,
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                    http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98133): https://edk2.groups.io/g/devel/message/98133
Mute This Topic: https://groups.io/mt/96070144/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-