Re: [PATCH] cpukit/rtems-fdt: Avoid use of malloc/errno

[Chris Johns]

Looks good and thanks

Chris

On 19/11/2022 2:47 am, Kinsey Moore wrote:
> Use of malloc implies errno which adds TLS dependencies and prevents use
> of this FDT wrapper library in BSP initialization code. This change
> makes use of rtems_malloc and rtems_calloc which avoid TLS dependencies.
> ---
>  cpukit/libmisc/rtems-fdt/rtems-fdt.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt.c 
> b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> index 1e382ca108..e5bab21664 100644
> --- a/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> +++ b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> @@ -25,9 +25,7 @@
>   * POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> -#include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -35,6 +33,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -175,13 +174,13 @@ rtems_fdt_init_index (rtems_fdt_handle* fdt, 
> rtems_fdt_blob* blob)
>/*
> * Create the index.
> */
> -  entries = calloc(num_entries, sizeof(rtems_fdt_index_entry));
> +  entries = rtems_calloc(num_entries, sizeof(rtems_fdt_index_entry));
>if (!entries)
>{
>  return -RTEMS_FDT_ERR_NO_MEMORY;
>}
>  
> -  names = calloc(1, total_name_memory);
> +  names = rtems_calloc(1, total_name_memory);
>if (!names)
>{
>  free(entries);
> @@ -505,7 +504,7 @@ rtems_fdt_load (const char* filename, rtems_fdt_handle* 
> handle)
>{
>  size_t offset;
>  
> -cdata = malloc(sb.st_size);
> +cdata = rtems_malloc(sb.st_size);
>  if (!cdata)
>  {
>close (bf);
> @@ -546,7 +545,7 @@ rtems_fdt_load (const char* filename, rtems_fdt_handle* 
> handle)
>  
>name_len = strlen (filename) + 1;
>  
> -  blob = malloc(sizeof (rtems_fdt_blob) + name_len + bsize);
> +  blob = rtems_malloc(sizeof (rtems_fdt_blob) + name_len + bsize);
>if (!blob)
>{
>  free(cdata);
> @@ -649,7 +648,7 @@ rtems_fdt_register (const void* dtb, rtems_fdt_handle* 
> handle)
>  return fe;
>}
>  
> -  blob = malloc(sizeof (rtems_fdt_blob));
> +  blob = rtems_malloc(sizeof (rtems_fdt_blob));
>if (!blob)
>{
>  return -RTEMS_FDT_ERR_NO_MEMORY;
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH] cpukit/rtems-fdt: Avoid use of malloc/errno

[Kinsey Moore]

Use of malloc implies errno which adds TLS dependencies and prevents use
of this FDT wrapper library in BSP initialization code. This change
makes use of rtems_malloc and rtems_calloc which avoid TLS dependencies.
---
 cpukit/libmisc/rtems-fdt/rtems-fdt.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt.c 
b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
index 1e382ca108..e5bab21664 100644
--- a/cpukit/libmisc/rtems-fdt/rtems-fdt.c
+++ b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
@@ -25,9 +25,7 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -35,6 +33,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -175,13 +174,13 @@ rtems_fdt_init_index (rtems_fdt_handle* fdt, 
rtems_fdt_blob* blob)
   /*
* Create the index.
*/
-  entries = calloc(num_entries, sizeof(rtems_fdt_index_entry));
+  entries = rtems_calloc(num_entries, sizeof(rtems_fdt_index_entry));
   if (!entries)
   {
 return -RTEMS_FDT_ERR_NO_MEMORY;
   }
 
-  names = calloc(1, total_name_memory);
+  names = rtems_calloc(1, total_name_memory);
   if (!names)
   {
 free(entries);
@@ -505,7 +504,7 @@ rtems_fdt_load (const char* filename, rtems_fdt_handle* 
handle)
   {
 size_t offset;
 
-cdata = malloc(sb.st_size);
+cdata = rtems_malloc(sb.st_size);
 if (!cdata)
 {
   close (bf);
@@ -546,7 +545,7 @@ rtems_fdt_load (const char* filename, rtems_fdt_handle* 
handle)
 
   name_len = strlen (filename) + 1;
 
-  blob = malloc(sizeof (rtems_fdt_blob) + name_len + bsize);
+  blob = rtems_malloc(sizeof (rtems_fdt_blob) + name_len + bsize);
   if (!blob)
   {
 free(cdata);
@@ -649,7 +648,7 @@ rtems_fdt_register (const void* dtb, rtems_fdt_handle* 
handle)
 return fe;
   }
 
-  blob = malloc(sizeof (rtems_fdt_blob));
+  blob = rtems_malloc(sizeof (rtems_fdt_blob));
   if (!blob)
   {
 return -RTEMS_FDT_ERR_NO_MEMORY;
-- 
2.30.2

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Use of rtems_fdt_* and sp01

[Joel Sherrill]

On Fri, Nov 18, 2022, 8:49 AM Kinsey Moore  wrote:

>
> On Fri, Nov 18, 2022 at 12:44 AM Sebastian Huber <
> sebastian.hu...@embedded-brains.de> wrote:
>
>> On 18/11/2022 06:32, Kinsey Moore wrote:
>> > On Thu, Nov 17, 2022 at 4:49 PM Chris Johns > > > wrote:
>> >
>> > On 18/11/2022 2:39 am, Kinsey Moore wrote:
>> >  > I recently added FDT support to the AArch64 ZynqMP BSPs to
>> > support an optional
>> >  > management console and managing ethernet parameters for LibBSD.
>> > Use of the
>> >  > rtems_fdt_* functions implies use of malloc which adds 4 bytes in
>> > the TLS space.
>> >  > The sp01 test uses rtems_task_construct and specifies a maximum
>> > TLS size of 0
>> >  > bytes which causes a failure which the non-zero TLS size is
>> > checked against the
>> >  > maximum TLS size of 0.
>> >
>> > Does this mean there exists a requirement a BSPs cannot internally
>> > use the heap?
>> >
>> >
>> > That appears to be the case, at least practically. It works fine, but
>> it
>> > causes a test failure...
>>
>> You can use the heap during system initialization. However, you should
>> avoid dependencies on errno, so instead of using malloc()/calloc() use
>> rtems_malloc()/rtems_calloc(). Independent of this, if the BSP
>> initialization can easily avoid the heap, then it should not use the heap.
>>
>> >
>> >
>> > Maybe the test needs to be adjusted?
>> >
>> >
>> > That's part of why I sent this to the list. I was unsure whether it was
>> > allowed or whether the test had bad assumptions baked into it.
>>
>> The tests check specific aspects of the thread-local storage support and
>> this works only if the BSP does not depend on TLS objects such as errno.
>>
>> This affects several other tests as well, so I guess my solution here is
> either to alter the rtems_fdt_ wrappers to avoid dependencies on TLS or
> proceed with the existing patch that uses the non-wrapped FDT functions.
>

I agree with Sebastian that fixing the bsp is right.

But also rtems_ fdt methods should be changed to use rtems_malloc. That
would leave those methods useful. They are broken now for many intended use
cases.


> Thanks!
>
> Kinsey
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Use of rtems_fdt_* and sp01

[Sebastian Huber]

On 18/11/2022 15:49, Kinsey Moore wrote:


On Fri, Nov 18, 2022 at 12:44 AM Sebastian Huber 
> wrote:


On 18/11/2022 06:32, Kinsey Moore wrote:
 > On Thu, Nov 17, 2022 at 4:49 PM Chris Johns mailto:chr...@rtems.org>
 > >> wrote:
 >
 >     On 18/11/2022 2:39 am, Kinsey Moore wrote:
 >      > I recently added FDT support to the AArch64 ZynqMP BSPs to
 >     support an optional
 >      > management console and managing ethernet parameters for
LibBSD.
 >     Use of the
 >      > rtems_fdt_* functions implies use of malloc which adds 4
bytes in
 >     the TLS space.
 >      > The sp01 test uses rtems_task_construct and specifies a
maximum
 >     TLS size of 0
 >      > bytes which causes a failure which the non-zero TLS size is
 >     checked against the
 >      > maximum TLS size of 0.
 >
 >     Does this mean there exists a requirement a BSPs cannot
internally
 >     use the heap?
 >
 >
 > That appears to be the case, at least practically. It works fine,
but it
 > causes a test failure...

You can use the heap during system initialization. However, you should
avoid dependencies on errno, so instead of using malloc()/calloc() use
rtems_malloc()/rtems_calloc(). Independent of this, if the BSP
initialization can easily avoid the heap, then it should not use the
heap.

 >
 >
 >     Maybe the test needs to be adjusted?
 >
 >
 > That's part of why I sent this to the list. I was unsure whether
it was
 > allowed or whether the test had bad assumptions baked into it.

The tests check specific aspects of the thread-local storage support
and
this works only if the BSP does not depend on TLS objects such as errno.

This affects several other tests as well, so I guess my solution here is 
either to alter the rtems_fdt_ wrappers to avoid dependencies on TLS or 
proceed with the existing patch that uses the non-wrapped FDT functions.


Your patch is pretty straight forward and gets rid of a heap usage 
during initialization. I would check it in anyway.


--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Use of rtems_fdt_* and sp01

[Kinsey Moore]

On Fri, Nov 18, 2022 at 12:44 AM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> On 18/11/2022 06:32, Kinsey Moore wrote:
> > On Thu, Nov 17, 2022 at 4:49 PM Chris Johns  > > wrote:
> >
> > On 18/11/2022 2:39 am, Kinsey Moore wrote:
> >  > I recently added FDT support to the AArch64 ZynqMP BSPs to
> > support an optional
> >  > management console and managing ethernet parameters for LibBSD.
> > Use of the
> >  > rtems_fdt_* functions implies use of malloc which adds 4 bytes in
> > the TLS space.
> >  > The sp01 test uses rtems_task_construct and specifies a maximum
> > TLS size of 0
> >  > bytes which causes a failure which the non-zero TLS size is
> > checked against the
> >  > maximum TLS size of 0.
> >
> > Does this mean there exists a requirement a BSPs cannot internally
> > use the heap?
> >
> >
> > That appears to be the case, at least practically. It works fine, but it
> > causes a test failure...
>
> You can use the heap during system initialization. However, you should
> avoid dependencies on errno, so instead of using malloc()/calloc() use
> rtems_malloc()/rtems_calloc(). Independent of this, if the BSP
> initialization can easily avoid the heap, then it should not use the heap.
>
> >
> >
> > Maybe the test needs to be adjusted?
> >
> >
> > That's part of why I sent this to the list. I was unsure whether it was
> > allowed or whether the test had bad assumptions baked into it.
>
> The tests check specific aspects of the thread-local storage support and
> this works only if the BSP does not depend on TLS objects such as errno.
>
> This affects several other tests as well, so I guess my solution here is
either to alter the rtems_fdt_ wrappers to avoid dependencies on TLS or
proceed with the existing patch that uses the non-wrapped FDT functions.

Thanks!

Kinsey
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Question about SMP tests after shutdown

[Sebastian Huber]

Hello Alan,

On 16/11/2022 17:55, Alan Cudmore wrote:

I  am running testsuite applications on a dual core RISC-V CPU. When I
run samples such as ticker.exe, the test ends without error messages.
On SMP tests, the tests seem to run correctly and end, but there is
always a fatal error from thread(s) on the other CPU.
(RTEMS_FATAL_SOURCE_SMP)
My guess is that the threads are being scheduled after the primary
test thread shut down the RTEMS executive, causing the error to be
caught. Is this expected behavior on a SMP system?
There are a couple of BSPs that implement a fatal error handler to
catch this error, and I would expect that on a real application, it
would be handled appropriately.


_Terminate() works like this:

void _Terminate(
  Internal_errors_Source the_source,
  Internal_errors_t  the_error
)
{
  _User_extensions_Fatal( the_source, the_error );
  _System_state_Set( SYSTEM_STATE_TERMINATED );
  _SMP_Request_shutdown();
  _CPU_Fatal_halt( the_source, the_error );
}

You get this behaviour, when there is no BSP fatal error extension which 
resets the system. The _User_extensions_Fatal() would not return in this 
case. If it returns, then _SMP_Request_shutdown() issues the shutdown 
request to the other processors.


--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel