[PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Rebecca Cran]

Some systems such as FreeBSD identify the platform as 'arm64'
and not 'aarch64' as Linux does.

Signed-off-by: Rebecca Cran <rebecca@bluestop.org>
---
 BaseTools/Source/C/GNUmakefile               | 5 +++--
 BaseTools/Source/C/Makefiles/header.makefile | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
index 1d048c4cc6..37bcce519c 100644
--- a/BaseTools/Source/C/GNUmakefile
+++ b/BaseTools/Source/C/GNUmakefile
@@ -21,8 +21,9 @@ ifndef HOST_ARCH
   endif
   ifneq (,$(findstring aarch64,$(uname_m)))
     HOST_ARCH=AARCH64
-  endif
-  ifneq (,$(findstring arm,$(uname_m)))
+  else ifneq (,$(findstring arm64,$(uname_m)))
+    HOST_ARCH=AARCH64
+  else ifneq (,$(findstring arm,$(uname_m)))
     HOST_ARCH=ARM
   endif
   ifndef HOST_ARCH
diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 90fb3453ad..d76b8283dd 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -23,8 +23,9 @@ ifndef HOST_ARCH
   endif
   ifneq (,$(findstring aarch64,$(uname_m)))
     HOST_ARCH=AARCH64
-  endif
-  ifneq (,$(findstring arm,$(uname_m)))
+  else ifneq (,$(findstring arm64,$(uname_m)))
+    HOST_ARCH=AARCH64
+  else ifneq (,$(findstring arm,$(uname_m)))
     HOST_ARCH=ARM
   endif
   ifndef HOST_ARCH
-- 
2.20.1

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Liming Gao]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Rebecca Cran via Groups.Io
> Sent: Thursday, April 11, 2019 11:17 AM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> Cc: Rebecca Cran <rebecca@bluestop.org>
> Subject: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64
> 
> Some systems such as FreeBSD identify the platform as 'arm64'
> and not 'aarch64' as Linux does.
> 
> Signed-off-by: Rebecca Cran <rebecca@bluestop.org>
> ---
>  BaseTools/Source/C/GNUmakefile               | 5 +++--
>  BaseTools/Source/C/Makefiles/header.makefile | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
> index 1d048c4cc6..37bcce519c 100644
> --- a/BaseTools/Source/C/GNUmakefile
> +++ b/BaseTools/Source/C/GNUmakefile
> @@ -21,8 +21,9 @@ ifndef HOST_ARCH
>    endif
>    ifneq (,$(findstring aarch64,$(uname_m)))
>      HOST_ARCH=AARCH64
> -  endif
> -  ifneq (,$(findstring arm,$(uname_m)))
> +  else ifneq (,$(findstring arm64,$(uname_m)))
> +    HOST_ARCH=AARCH64
> +  else ifneq (,$(findstring arm,$(uname_m)))
>      HOST_ARCH=ARM
>    endif
>    ifndef HOST_ARCH
> diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
> index 90fb3453ad..d76b8283dd 100644
> --- a/BaseTools/Source/C/Makefiles/header.makefile
> +++ b/BaseTools/Source/C/Makefiles/header.makefile
> @@ -23,8 +23,9 @@ ifndef HOST_ARCH
>    endif
>    ifneq (,$(findstring aarch64,$(uname_m)))
>      HOST_ARCH=AARCH64
> -  endif
> -  ifneq (,$(findstring arm,$(uname_m)))
> +  else ifneq (,$(findstring arm64,$(uname_m)))
> +    HOST_ARCH=AARCH64
> +  else ifneq (,$(findstring arm,$(uname_m)))
>      HOST_ARCH=ARM
>    endif
>    ifndef HOST_ARCH
> --
> 2.20.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#38845): https://edk2.groups.io/g/devel/message/38845
> Mute This Topic: https://groups.io/mt/31026994/1759384
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com]
> -=-=-=-=-=-=

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Philippe Mathieu-Daudé]

On 4/11/19 5:16 AM, Rebecca Cran via Groups.Io wrote:
> Some systems such as FreeBSD identify the platform as 'arm64'
> and not 'aarch64' as Linux does.

Per https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297
this seems to be a FreeBSD bug.

> Signed-off-by: Rebecca Cran <rebecca@bluestop.org>
> ---
>  BaseTools/Source/C/GNUmakefile               | 5 +++--
>  BaseTools/Source/C/Makefiles/header.makefile | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
> index 1d048c4cc6..37bcce519c 100644
> --- a/BaseTools/Source/C/GNUmakefile
> +++ b/BaseTools/Source/C/GNUmakefile
> @@ -21,8 +21,9 @@ ifndef HOST_ARCH
>    endif
>    ifneq (,$(findstring aarch64,$(uname_m)))
>      HOST_ARCH=AARCH64
> -  endif
> -  ifneq (,$(findstring arm,$(uname_m)))
> +  else ifneq (,$(findstring arm64,$(uname_m)))
> +    HOST_ARCH=AARCH64
> +  else ifneq (,$(findstring arm,$(uname_m)))
>      HOST_ARCH=ARM
>    endif
>    ifndef HOST_ARCH
> diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
> index 90fb3453ad..d76b8283dd 100644
> --- a/BaseTools/Source/C/Makefiles/header.makefile
> +++ b/BaseTools/Source/C/Makefiles/header.makefile
> @@ -23,8 +23,9 @@ ifndef HOST_ARCH
>    endif
>    ifneq (,$(findstring aarch64,$(uname_m)))
>      HOST_ARCH=AARCH64
> -  endif
> -  ifneq (,$(findstring arm,$(uname_m)))
> +  else ifneq (,$(findstring arm64,$(uname_m)))
> +    HOST_ARCH=AARCH64
> +  else ifneq (,$(findstring arm,$(uname_m)))
>      HOST_ARCH=ARM
>    endif
>    ifndef HOST_ARCH
> 

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Laszlo Ersek]

On 04/11/19 16:23, Philippe Mathieu-Daudé wrote:
> On 4/11/19 5:16 AM, Rebecca Cran via Groups.Io wrote:
>> Some systems such as FreeBSD identify the platform as 'arm64'
>> and not 'aarch64' as Linux does.
> 
> Per https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297
> this seems to be a FreeBSD bug.

You are correct, but that issue was filed on 2017-06-26. I don't think
we should hold our breaths, as long as the workaround is simple. And, it
does look simple. (We've worked around worse.)

I do suggest an addition to the patch, however: Rebecca, please add the
link discovered by Phil near the "arm64" matches, in a comment.
Something like:

# work around <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297>

Thanks,
Laszlo

> 
>> Signed-off-by: Rebecca Cran <rebecca@bluestop.org>
>> ---
>>  BaseTools/Source/C/GNUmakefile               | 5 +++--
>>  BaseTools/Source/C/Makefiles/header.makefile | 5 +++--
>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
>> index 1d048c4cc6..37bcce519c 100644
>> --- a/BaseTools/Source/C/GNUmakefile
>> +++ b/BaseTools/Source/C/GNUmakefile
>> @@ -21,8 +21,9 @@ ifndef HOST_ARCH
>>    endif
>>    ifneq (,$(findstring aarch64,$(uname_m)))
>>      HOST_ARCH=AARCH64
>> -  endif
>> -  ifneq (,$(findstring arm,$(uname_m)))
>> +  else ifneq (,$(findstring arm64,$(uname_m)))
>> +    HOST_ARCH=AARCH64
>> +  else ifneq (,$(findstring arm,$(uname_m)))
>>      HOST_ARCH=ARM
>>    endif
>>    ifndef HOST_ARCH
>> diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
>> index 90fb3453ad..d76b8283dd 100644
>> --- a/BaseTools/Source/C/Makefiles/header.makefile
>> +++ b/BaseTools/Source/C/Makefiles/header.makefile
>> @@ -23,8 +23,9 @@ ifndef HOST_ARCH
>>    endif
>>    ifneq (,$(findstring aarch64,$(uname_m)))
>>      HOST_ARCH=AARCH64
>> -  endif
>> -  ifneq (,$(findstring arm,$(uname_m)))
>> +  else ifneq (,$(findstring arm64,$(uname_m)))
>> +    HOST_ARCH=AARCH64
>> +  else ifneq (,$(findstring arm,$(uname_m)))
>>      HOST_ARCH=ARM
>>    endif
>>    ifndef HOST_ARCH
>>
> 
> 
> 

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Rebecca Cran]

On 2019-04-11 11:52, Laszlo Ersek wrote:
> On 04/11/19 16:23, Philippe Mathieu-Daudé wrote:
>> On 4/11/19 5:16 AM, Rebecca Cran via Groups.Io wrote:
>>> Some systems such as FreeBSD identify the platform as 'arm64'
>>> and not 'aarch64' as Linux does.
>> Per https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297
>> this seems to be a FreeBSD bug.
> You are correct, but that issue was filed on 2017-06-26. I don't think
> we should hold our breaths, as long as the workaround is simple. And, it
> does look simple. (We've worked around worse.)
>
> I do suggest an addition to the patch, however: Rebecca, please add the
> link discovered by Phil near the "arm64" matches, in a comment.
> Something like:
>
> # work around <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297>



Thanks, I'll do that. I don't think there's anything to fix, because 
"uname -m" returns "arm64" and "uname -p" "aarch64". The EDK2 Makefiles 
use "uname -m" which I don't think anybody has complained about, but 
which return different values on Linux and FreeBSD.


-- 

Rebecca Cran

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Philippe Mathieu-Daudé]

On 4/11/19 7:52 PM, Laszlo Ersek wrote:
> On 04/11/19 16:23, Philippe Mathieu-Daudé wrote:
>> On 4/11/19 5:16 AM, Rebecca Cran via Groups.Io wrote:
>>> Some systems such as FreeBSD identify the platform as 'arm64'
>>> and not 'aarch64' as Linux does.
>>
>> Per https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297
>> this seems to be a FreeBSD bug.
> 
> You are correct, but that issue was filed on 2017-06-26. I don't think
> we should hold our breaths, as long as the workaround is simple. And, it
> does look simple. (We've worked around worse.)
> 
> I do suggest an addition to the patch, however: Rebecca, please add the
> link discovered by Phil near the "arm64" matches, in a comment.
> Something like:
> 
> # work around <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297>

Good idea.

> Thanks,
> Laszlo
> 
>>
>>> Signed-off-by: Rebecca Cran <rebecca@bluestop.org>
>>> ---
>>>  BaseTools/Source/C/GNUmakefile               | 5 +++--
>>>  BaseTools/Source/C/Makefiles/header.makefile | 5 +++--
>>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
>>> index 1d048c4cc6..37bcce519c 100644
>>> --- a/BaseTools/Source/C/GNUmakefile
>>> +++ b/BaseTools/Source/C/GNUmakefile
>>> @@ -21,8 +21,9 @@ ifndef HOST_ARCH
>>>    endif
>>>    ifneq (,$(findstring aarch64,$(uname_m)))
>>>      HOST_ARCH=AARCH64
>>> -  endif
>>> -  ifneq (,$(findstring arm,$(uname_m)))
>>> +  else ifneq (,$(findstring arm64,$(uname_m)))

Here you change 2 different if() blocks into a single if/elif/elif.
Although not mentioned in the patch description, it is cleaner than
before your patch.

With the work around link added:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

Thanks,

Phil.

>>> +    HOST_ARCH=AARCH64
>>> +  else ifneq (,$(findstring arm,$(uname_m)))
>>>      HOST_ARCH=ARM
>>>    endif
>>>    ifndef HOST_ARCH
>>> diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
>>> index 90fb3453ad..d76b8283dd 100644
>>> --- a/BaseTools/Source/C/Makefiles/header.makefile
>>> +++ b/BaseTools/Source/C/Makefiles/header.makefile
>>> @@ -23,8 +23,9 @@ ifndef HOST_ARCH
>>>    endif
>>>    ifneq (,$(findstring aarch64,$(uname_m)))
>>>      HOST_ARCH=AARCH64
>>> -  endif
>>> -  ifneq (,$(findstring arm,$(uname_m)))
>>> +  else ifneq (,$(findstring arm64,$(uname_m)))
>>> +    HOST_ARCH=AARCH64
>>> +  else ifneq (,$(findstring arm,$(uname_m)))
>>>      HOST_ARCH=ARM
>>>    endif
>>>    ifndef HOST_ARCH
>>>
>>
>> 
>>
> 

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[rebecca]

On 2019-04-11 13:55, Philippe Mathieu-Daudé wrote:
>
> With the work around link added:
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>



But it's _not_ working around that bug. That was about "uname -p", but 
the patch is about "uname -m".


-- 
Rebecca Cran

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Philippe Mathieu-Daudé]

On 4/11/19 9:58 PM, Rebecca Cran wrote:
> On 2019-04-11 13:55, Philippe Mathieu-Daudé wrote:
>>
>> With the work around link added:
>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> 
> 
> 
> But it's _not_ working around that bug. That was about "uname -p", but
> the patch is about "uname -m".

Ah you are right, the script simply parse the 'machine hardware name' to
extract the host architecture. We shouldn't rely on the 'processor type
name' which is non-portable. Simply checking for 'arm64' is enough.

So do not add the link, and keep my tag:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

Regards,

Phil.

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Laszlo Ersek]

On 04/11/19 22:11, Philippe Mathieu-Daudé wrote:
> On 4/11/19 9:58 PM, Rebecca Cran wrote:
>> On 2019-04-11 13:55, Philippe Mathieu-Daudé wrote:
>>>
>>> With the work around link added:
>>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>
>>
>>
>> But it's _not_ working around that bug. That was about "uname -p", but
>> the patch is about "uname -m".

That sounds convincing, thank you for the clarification. I'd still say
it's a workaround, just not for that specific FreeBSD BZ, because:

> Ah you are right, the script simply parse the 'machine hardware name' to
> extract the host architecture. We shouldn't rely on the 'processor type
> name' which is non-portable.

obviously the "machine hardware name" is non-portable either, when we
consider FreeBSD.

Either way I've obsessed enough about this.

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

Thanks
Laszlo

> Simply checking for 'arm64' is enough.
> 
> So do not add the link, and keep my tag:
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[rebecca]

On 2019-04-12 01:54, Laszlo Ersek wrote:
>
> obviously the "machine hardware name" is non-portable either, when we
> consider FreeBSD.
>
> Either way I've obsessed enough about this.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Could someone commit this, please?


-- 
Rebecca Cran

Re: [edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

[Laszlo Ersek]

On 05/02/19 23:15, rebecca@bluestop.org wrote:
> On 2019-04-12 01:54, Laszlo Ersek wrote:
>>
>> obviously the "machine hardware name" is non-portable either, when we
>> consider FreeBSD.
>>
>> Either way I've obsessed enough about this.
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> 
> Could someone commit this, please?
> 
> 

[lersek@redhat.com: fix up DKIM damage in the "From:" address]

Commit fbb0ec7ea4c0.

Thanks,
Laszlo