[PATCH] tester: Add support for test-too-long

[chrisj]

From: Chris Johns 

- A test that loops generating output did not timeout. Monitor the
  the session time and set a maximum test period.
---
 tester/rt/config.py |  45 +
 tester/rt/exe.py| 172 
 tester/rt/gdb.py| 117 +-
 tester/rt/report.py |  28 +-
 tester/rt/test.py   |   6 +-
 tester/rt/tftp.py   |  33 --
 tester/rtems/testing/testing.mc |   3 +-
 tester/wscript  |   1 +
 8 files changed, 348 insertions(+), 57 deletions(-)
 create mode 100644 tester/rt/exe.py

diff --git a/tester/rt/config.py b/tester/rt/config.py
index ee639e9..bc9263a 100644
--- a/tester/rt/config.py
+++ b/tester/rt/config.py
@@ -1,6 +1,6 @@
 #
 # RTEMS Tools Project (http://www.rtems.org/)
-# Copyright 2013-2017 Chris Johns (chr...@rtems.org)
+# Copyright 2013-2020 Chris Johns (chr...@rtems.org)
 # All rights reserved.
 #
 # This file is part of the RTEMS Tools package in 'rtems-tools'.
@@ -49,6 +49,7 @@ from rtemstoolkit import path
 from rtemstoolkit import stacktraces
 
 import console
+import exe
 import gdb
 import tftp
 
@@ -78,6 +79,7 @@ class file(config.file):
 self.report = report
 self.name = name
 self.timedout = False
+self.test_too_long = False
 self.test_started = False
 self.kill_good = False
 self.kill_on_end = False
@@ -102,6 +104,12 @@ class file(config.file):
 self._unlock()
 self.capture('*** TIMEOUT TIMEOUT')
 
+def _test_too_long(self):
+self._lock()
+self.test_too_long = True
+self._unlock()
+self.capture('*** TEST TOO LONG')
+
 def _ok_kill(self):
 self._lock()
 self.kill_good = True
@@ -219,22 +227,20 @@ class file(config.file):
 else:
 raise error.general(self._name_line_msg('invalid console 
type'))
 
-def _dir_execute(self, data, total, index, exe, bsp_arch, bsp):
-self.process = execute.execute(output = self.capture)
-if self.console:
-self.console.open()
+def _dir_execute(self, data, total, index, rexe, bsp_arch, bsp):
+self.process = exe.exe(bsp_arch, bsp, trace = self.exe_trace('exe'))
 if not self.in_error:
-self.capture_console('run: %s' % (' '.join(data)))
+if self.console:
+self.console.open()
 if not self.opts.dry_run():
-ec, proc = self.process.open(data,
- timeout = 
(int(self.expand('%{timeout}')),
-self._timeout))
-self._lock()
-if not (self.kill_good or self.defined('exe_ignore_ret')) and 
ec > 0:
-self._error('execute failed: %s: exit-code:%d' % (' 
'.join(data), ec))
-elif self.timedout:
-self.process.kill()
-self._unlock()
+self.process.open(data,
+  ignore_exit_code = 
self.defined('exe_ignore_ret'),
+  output = self.capture,
+  console = self.capture_console,
+  timeout = (int(self.expand('%{timeout}')),
+ 
int(self.expand('%{max_test_period}')),
+ self._timeout,
+ self._test_too_long))
 if self.console:
 self.console.close()
 
@@ -255,7 +261,10 @@ class file(config.file):
   script = script,
   output = self.capture,
   gdb_console = self.capture_console,
-  timeout = int(self.expand('%{timeout}')))
+  timeout = (int(self.expand('%{timeout}')),
+ 
int(self.expand('%{max_test_period}')),
+ self._timeout,
+ self._test_too_long))
 if self.console:
 self.console.close()
 
@@ -278,7 +287,9 @@ class file(config.file):
   output_length = self._output_length,
   console = self.capture_console,
   timeout = (int(self.expand('%{timeout}')),
- self._timeout))
+ 
int(self.expand('%{max_test_period}')),
+ self._timeout,
+ self._test_too_long))
 if self.console:
 self.console.close()
 
diff --git a/tester/rt/exe.py b/tester/rt/exe.py
new file mode 100644

Re: dl06 fails to build for RISC-V (griscv bsp)

[Hesham Almatary]

On Mon, 7 Sep 2020 at 09:01, Jiri Gaisler  wrote:
>
>
> On 9/7/20 10:44 AM, Hesham Almatary wrote:
> > I have only made sure rap builds when I added libdl support for
> > RISC-V. But I haven't tested it on run-time, only ELF objects. It's
> > likely to have been failing in run-time all along. Also, there are a
> > few relocations still to be implemented for ELF like PCREL_LO12_X and
> > RELAX. I got around that by passing -mno-relax
>
> Very interesting! I applied -mno-relax when building dl06, and the error then 
> changed to:
>
> error: rap::object: Section index '0' not found: 
> /home/jiri/src/rtems/6/bin/../lib/gcc/riscv-rtems6/10.2.1/../../../../riscv-rtems6/lib/rv32imafd/ilp32d/libm.a:lib_a-e_atan2.o@709534
>
> Does this indicate that newlib must be build with -mno-relax? If so, maybe we 
> should disable the dlxx tests for RISC-V ..?
>
I don't think so. Only the to-be-loaded object needs -mno-relax as
libdl ignores the RELAX relocations, but GCC does handle it.
AFAIK, relaxation only affects performance by trying to be smart
emitting one instruction (with an immediate) instead of two
instructions that construct a full reg-size value (in a register). It
shouldn't affect the functional behaviour if we ignore it, as we will
be just taking the slowpath of handling two instructions instead of
trying to relax them to just one. LLVM/LLD, for example, hasn't
implemented it until a few months ago. More information here:
https://gcc.gnu.org/onlinedocs/gcc/RISC-V-Options.html
https://www.sifive.com/blog/all-aboard-part-3-linker-relaxation-in-riscv-toolchain

>
> >
> > On Sun, 6 Sep 2020 at 19:50, Jiri Gaisler  wrote:
> >> I re-applied your patch and dl06 builds again, but the dl06.exe program 
> >> fails. I have updated the ticket:
> >>
> >> https://devel.rtems.org/ticket/4069#no2
> >>
> >>
> >> On 9/6/20 10:12 AM, Hesham Almatary wrote:
> >>> That's the same as [1]. I've seen that error before and (thought I)
> >>> fixed it [2], but not sure what has changed since then.
> >>>
> >>> [1] https://lists.rtems.org/pipermail/devel/2020-August/061717.html
> >>> [2] 
> >>> https://github.com/RTEMS/rtems-tools/commit/e6e610d262940b7651157597b6b1406aa806b4d1
> >>>
> >>> On Sun, 6 Sep 2020 at 09:14, Chris Johns  wrote:
>  On 6/9/20 6:32 am, Jiri Gaisler wrote:
> > I have updated both RTEMS and RSB to git head, and dl06 now fails to 
> > build:
> >
> > riscv-rtems6-g++ -march=rv32imafd -mabi=ilp32d -O2 -g 
> > -ffunction-sections -fdata-sections -Wall  -Wl,--gc-sections  
> > -march=rv32imafd -mabi=ilp32d  -B./../../lib/libbsp/riscv/griscv 
> > -B/home/jiri/ibm/src/rtems/rtems/bsps/riscv/griscv/start -specs 
> > bsp_specs -qrtems -L./../../cpukit 
> > -L/home/jiri/ibm/src/rtems/rtems/bsps/riscv/shared/start 
> > -Wl,--wrap=printf -Wl,--wrap=puts -Wl,--wrap=putchar -o dl05.exe 
> > dl05/dl05-init.o dl05/dl05-dl-load.o dl05/dl05-dl-cpp.o dl05-dl05-tar.o 
> > ../../lib/libbsp/riscv/griscv/librtemsbsp.a ../../cpukit/librtemscpu.a 
> > ../../cpukit/librtemstest.a dl05-sym.o
>  This is dl05 and I do not think it is related to the issue.
> 
> > rtems-ld -r /home/jiri/src/rtems/riscvmp/riscv-rtems6/c/griscv \
> >   -C riscv-rtems6-gcc -c "-march=rv32imafd -mabi=ilp32d" \
> >   -O rap -b dl06.pre -e rtems_main -s \
> >   -o dl06.rap dl06-o1.o dl06-o2.o -lm
> > error: rap::object: Section index '0' not found: dl06-o1.o
> > Makefile:8528: recipe for target 'dl06.rap' failed
> > make[5]: *** [dl06.rap] Error 10
>  This looks like something in rtems-ld in the rtems-tools.git repo.
> 
> > It seems like dl06-o1.o not built but a link is attempted - is dl06 
> > supposed to be enabled or disabled for RISC-V ?
>  Enabled but it seems something has changed to cause the test to not 
>  link. I
>  wonder if a tool upgrade is the reason. I have raised ..
> 
>  https://devel.rtems.org/ticket/4069
> 
>  Chris
>  ___
>  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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: BSP Test Results

[Gedare Bloom]

On Sun, Sep 6, 2020 at 8:55 PM Chris Johns  wrote:
>
> Hello,
>
> I would like to discuss BSP Test results early in the release cycle in the 
> hope
> we avoid the last minute issues we encountered with RTEMS 5 and the "expected"
> failure state ticket.
>
> I would like to update this section ...
>
> https://docs.rtems.org/branches/master/user/testing/tests.html#expected-test-states
>
> to state there is to be a ticket for each `expected-fail` test state. I 
> believe
> this was the outcome of the discussion that took place. Please correct me if
> this is not correct.
>
> The purpose of the `expected-fail` is to aid the accounting of the test 
> results
> to let us know if there are any regressions. We need to account for tests that
> fail so we can track if a recent commit results in a new failure, i.e. a
> regression. To do this we need to capture the state in a way `rtems-test` can
> indicate a regression.
>
> I think the `indeterminate` state may need further explanation as it will help
> in the cases a simulator passes a test but the test fails on some hardware. I 
> am
> currently seeing this with spcache01 on the PC BSP.
>
> With the level of continuous building and testing we are currently doing being
> able to easily determine a regression will become important. Check out the
> example below.
>
> I would like to avoid us sitting with failures that do not have tickets and 
> are
> not accounted for. I know there is a lump of work to account for the failures
> and after that is done I think the effort needed to maintain the failure 
> states
> will drop.
>
> As a result I have been pondering how I can encourage this work be done. I am
> considering updating the tier-1 status to requiring there be 0 unaccounted for
> failures. That is the `rtems-test`'s Failure count is 0 for a hardware test 
> run.
>
> Chris
>
> An example using Joel's recent test run (thanks Joel :)). The sparc/leon2
> results show no regressions:
>
> Summary
> ===
>
> Passed:580
> Failed:  0
> User Input:  6
> Expected Fail:   1
Is there a ticket for this one? out of curiosity.

> Indeterminate:   0
> Benchmark:   3
> Timeout: 0
> Invalid: 0
> Wrong Version:   0
> Wrong Build: 0
> Wrong Tools: 0
> --
> Total: 590
>
> [ https://lists.rtems.org/pipermail/build/2020-September/018089.html ]
>
> while the sparc/erc32 has a single failure:
>
> Summary
> ===
>
> Passed:579
> Failed:  1
> User Input:  6
> Expected Fail:   1
> Indeterminate:   0
> Benchmark:   3
> Timeout: 0
> Invalid: 0
> Wrong Version:   0
> Wrong Build: 0
> Wrong Tools: 0
> --
> Total: 590
>
> Failures:
>  spintrcritical08.exe
>
> [ https://lists.rtems.org/pipermail/build/2020-September/018088.html ]
>
> ___
> 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: [PATCH v2] rtems: Add rtems_task_create_from_config()

[Joel Sherrill]

  This has gotten to be a long-ish side discussion. I havecomments on the
patch and will reply to the original email for that.

On Thu, Sep 3, 2020 at 10:17 PM Chris Johns  wrote:

> On 3/9/20 10:08 pm, Sebastian Huber wrote:
> >
> > On 03/09/2020 04:08, Chris Johns wrote:
> >> On 3/9/20 2:09 am, Sebastian Huber wrote:
> >>> In contrast to rtems_task_create() this function creates a task with a
> >>> user-provided task storage area.  The new create function uses a
> >>> configuration structure instead of individual parameters.
> >>>
> >>> Add RTEMS_TASK_STORAGE_ALIGNMENT to define the recommended alignment of
> >>> a task storage area.
> >>>
> >>> Add RTEMS_TASK_STORAGE_SIZE() to calculate the recommended size of a
> >>> task storage area based on the task attributes and the size dedicated
> to
> >>> the task stack and thread-local storage.  This macro may allow future
> >>> extensions without breaking the API.
> >> Have all the issues in the other thread been resolved? It would be good
> to have
> >> Joel ack this patch before it is merged.
> > I tried to address the issues raised by Joel.
>
> He will need to answer.
>

I will also comment on the patch. But the first thing I noticed was how
does the user
know the amount of memory required for keys? That was in a Doxygen comment
about the macro which is a big improvement but still makes the user guess
something we should compute.

This reminds me of not giving an option to account for message buffers. Then
we added a confdefs macro to specify the total amount of memory. It took
another
release before we realized that we didn't give the user a way to calculate
the
memory required per queue based on number of messages and size of the
buffer. This made them guess to account for the overhead for each of those.
Let's try to avoid making users guess the size of memory that we would
normally
compute.

>
> >> In considering the support for static allocation in general I am
> wondering if
> >> the RTEMS Classic API Guide is poorly named? This is a new API with new
> features
> >> and it is not "classic". The Key Concepts section of the that manual
> says...
> >>
> >>   "RTEMS provides directives which can be used to dynamically
> >>create, delete, and manipulate a set of predefined object types."
> >>
> >> This change makes this statement questionable.
> >
> > The task is still dynamically created, however, with a user-provided task
> > storage area. It is similar to the POSIX threads which also have to
> option to
> > use a user-provided task storage area. When you don't use unlimited
> objects,
> > then the memory for task control blocks is only statically allocated.
> The tasks
> > are dynamically maintained.
>
> Thanks. This makes sense and almost what could be used in the
> documentation :)
>

We just need to make sure we name the various situations clearly without
names
like new/old, improved, etc.

I see a handful of RTOS memory organizations a systems designer has to pick
and we have to name and explain them.

(1) Workspace vs C Program Heap
- usually associated with hard limits on objects. Unlimited is of
marginal
  use with the split memory model.

(2) Unified Workspace/Heap
   - usually associated with unlimited objects

On top of that, you have

(a) stack and FP allocated from Workspace
(b)  BSP/application provided stack allocator
(c) Application provided memory per task

That makes six combinations. I would tend to say that if you choose to
use Unified Workspace, then 2a makes sense. 2b also makes sense if
you had some stack protection scheme. But why would someone mix in
2c. Should work but not likely to be a common setup unless we do it
in support code.

For (1), I can see situations where you would use 1a, 1b, and 1c.

Overall, I see a-c as useful with 1+2 and a-c should work with 1 or 2
but we have to explain the choices.

I think this means we need to name a-c so we use consistent
terms across the community.


>
> > The self-contained synchronization objects of this chapter
> >
> >
> https://docs.rtems.org/branches/master/c-user/self_contained_objects.html
> >
> > are a different kind of thing.
>
> Yes and this makes the word Classic look wrong.
>

Classic is probably technically only the core APIs that derived from
RTEID/ORKID
or soon thereafter. Perhaps only the managers that existed before any POSIX
was
added which would be strictly less than v4.x of RTEMS.

But now, Classic is just RTEMS specific APIs that start with rtems_ even
though that
leads to duplicate functionality between Classic proper and the self
contained objects.

I think this is just a matter of naming chapters correctly and ensuring
that the
first 2 sentences of each self-contained object type's description makes it
clear
the distinction.


>
> >> I am sure there are other places
> >> that will need to be updated. What documentation and examples are
> planned to
> >> explain the different allocation models RTEMS now supports? I think we
> have 3
> >> AP

Re: [PATCH v2] rtems: Add rtems_task_create_from_config()

[Joel Sherrill]

On Wed, Sep 2, 2020 at 11:09 AM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> In contrast to rtems_task_create() this function creates a task with a
> user-provided task storage area.  The new create function uses a
> configuration structure instead of individual parameters.
>
> Add RTEMS_TASK_STORAGE_ALIGNMENT to define the recommended alignment of
> a task storage area.
>
> Add RTEMS_TASK_STORAGE_SIZE() to calculate the recommended size of a
> task storage area based on the task attributes and the size dedicated to
> the task stack and thread-local storage.  This macro may allow future
> extensions without breaking the API.
>
> Update #3959.
> ---
>
> v2:
>
> Rename function from rtems_task_build() to
> rtems_task_create_from_config().  Add RTEMS_TASK_STORAGE_ALIGNMENT and
> RTEMS_TASK_STORAGE_SIZE().  Improve documentation.
>
>  cpukit/Makefile.am  |   1 +
>  cpukit/include/rtems/rtems/tasks.h  | 124 +++
>  cpukit/include/rtems/rtems/tasksimpl.h  |  11 +
>  cpukit/rtems/src/taskcreate.c   | 278 +---
>  cpukit/rtems/src/taskcreatefromconfig.c | 274 +++
>  testsuites/sptests/sp01/init.c  |  24 +-
>  testsuites/sptests/sp01/sp01.doc|   1 +
>  testsuites/sptests/sp01/system.h|   2 +-
>  8 files changed, 479 insertions(+), 236 deletions(-)
>  create mode 100644 cpukit/rtems/src/taskcreatefromconfig.c
>
> diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
> index e5009e53c9..caa6a9efe6 100644
> --- a/cpukit/Makefile.am
> +++ b/cpukit/Makefile.am
> @@ -787,6 +787,7 @@ librtemscpu_a_SOURCES += rtems/src/statustoerrno.c
>  librtemscpu_a_SOURCES += rtems/src/systemeventreceive.c
>  librtemscpu_a_SOURCES += rtems/src/systemeventsend.c
>  librtemscpu_a_SOURCES += rtems/src/taskcreate.c
> +librtemscpu_a_SOURCES += rtems/src/taskcreatefromconfig.c
>  librtemscpu_a_SOURCES += rtems/src/taskdelete.c
>  librtemscpu_a_SOURCES += rtems/src/taskexit.c
>  librtemscpu_a_SOURCES += rtems/src/taskgetaffinity.c
> diff --git a/cpukit/include/rtems/rtems/tasks.h
> b/cpukit/include/rtems/rtems/tasks.h
> index 12c323e60e..a183dcafed 100644
> --- a/cpukit/include/rtems/rtems/tasks.h
> +++ b/cpukit/include/rtems/rtems/tasks.h
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #ifdef __cplusplus
> @@ -164,6 +165,129 @@ rtems_status_code rtems_task_create(
>rtems_id*id
>  );
>
> +/**
> + * @brief Returns the recommended task storage area size for the
> specified size
> + *   and task attributes.
> + *
> + * @param _size is the size dedicated to the task stack and thread-local
> + *   storage.
>

How does the user get the TLS size?

Need advice on that. Seems hard to get at compile time since it is a link
time aggregation.


> + *
> + * @param _attributes is the attribute set of the task using the storage
> area.
> + *
> + * @return The recommended task storage area size is returned calculated
> from
> + *   the input parameters.
> + *
> + * @see rtems_task_config
> + */
> +#define RTEMS_TASK_STORAGE_SIZE( _size, _attributes ) \
> +  ( ( _size ) + \
> +( ( ( _attributes ) & RTEMS_FLOATING_POINT ) != 0 ? CONTEXT_FP_SIZE :
> 0 ) )
>

If the architecture requires all threads to be FP, I don't think this will
work.


> +
> +/**
> + * @brief This variable attribute defines the recommended alignment of a
> task
> + *   storage area.
> + *
> + * @see rtems_task_config
> + */
> +#define RTEMS_TASK_STORAGE_ALIGNMENT RTEMS_ALIGNED( CPU_STACK_ALIGNMENT )
>
> Good. I assume the stack comes off the lower address range. Will
the TLS and FP areas have sufficient alignment? Is that accounted for?

I can't speak to generic TLS alignment but seems like cache alignment
is what the linker script would aim for.

FP context may have to be double aligned on many architectures.


> +/**
> + * @brief This structure defines the configuration of a task created by
> + *   rtems_task_create_from_config().
> + */
> +typedef struct {
> +  /**
> +   * @brief This member defines the name of the task.
> +   */
> +  rtems_name name;
> +
> +  /**
> +   * @brief This member defines initial priority of the task.
> +   */
> +  rtems_task_priority initial_priority;
> +
> +  /**
> +   * @brief This member shall point to the task storage area begin.
> +   *
> +   * The task storage area will contain the task stack, the thread-local
> +   * storage, and, on some architectures, the floating-point context.
>

What does this mean? I think most architectures treat the areas as
separate (FP not in integer context) and some require all tasks to be
FP. This seems inaccurate.


> +   *
> +   * There are no alignment requirements for the task storage area.  To
> avoid
> +   * memory waste, use the ::RTEMS_TASK_STORAGE_ALIGNMENT variable
> attribute to
> +   * enforce the recommended alignment of the task storage area.
> +   */
> +  void *storage_area;
> +
> +  /**
> +   * @brief This member defines size of t

Re: BSP Test Results

[Joel Sherrill]

On Mon, Sep 7, 2020 at 12:44 AM Chris Johns  wrote:

> On 7/9/20 2:16 pm, Joel Sherrill wrote:
> >
> >
> > On Sun, Sep 6, 2020, 9:55 PM Chris Johns  > > wrote:
> >
> > Hello,
> >
> > I would like to discuss BSP Test results early in the release cycle
> in the hope
> > we avoid the last minute issues we encountered with RTEMS 5 and the
> "expected"
> > failure state ticket.
> >
> > I would like to update this section ...
> >
> >
> https://docs.rtems.org/branches/master/user/testing/tests.html#expected-test-states
> >
> > to state there is to be a ticket for each `expected-fail` test
> state. I believe
> > this was the outcome of the discussion that took place. Please
> correct me if
> > this is not correct.
> >
> > The purpose of the `expected-fail` is to aid the accounting of the
> test results
> > to let us know if there are any regressions. We need to account for
> tests that
> > fail so we can track if a recent commit results in a new failure,
> i.e. a
> > regression. To do this we need to capture the state in a way
> `rtems-test` can
> > indicate a regression.
> >
> > I think the `indeterminate` state may need further explanation as it
> will help
> > in the cases a simulator passes a test but the test fails on some
> hardware. I am
> > currently seeing this with spcache01 on the PC BSP.
> >
> >
> > I don't mind this as long as we have a rigid format to ensure these are
> easy to
> > find.
>
> Do you mean find the location in the tcfg files?
>

No. I just mean sometimes there is a comment directly above the l
exclude line, sometimes above a group, etc. I would want something
like

exclude: dl06  # #1234 dynamic loading broken on RISC-V

Maybe even not as a comment but a ticket number so it could be
checked that every exclude had a ticket number in the form of #[N]
(four or five digits).

>
> > Perhaps info at the main be of the line marking this state. In the past,
> > we have tended to our comments before.
>
> If you mean the location in the tcfg files being printed out by the test,
> have
> no idea how to do that. What about grep?
>

grep works if on the same line. Hence the above suggestion. If we want to
add excludes, let's document which test and ticket in a way that
theoretically
we could check if the ticket is open or closed.

No way to automate that it applies to this BSP's exclude I think.


> > With the level of continuous building and testing we are currently
> doing being
> > able to easily determine a regression will become important. Check
> out the
> > example below.
> >
> > I would like to avoid us sitting with failures that do not have
> tickets and are
> > not accounted for. I know there is a lump of work to account for the
> failures
> > and after that is done I think the effort needed to maintain the
> failure states
> > will drop.
> >
> > As a result I have been pondering how I can encourage this work be
> done. I am
> > considering updating the tier-1 status to requiring there be 0
> unaccounted for
> > failures. That is the `rtems-test`'s Failure count is 0 for a
> hardware test run.
> >
> > Chris
> >
> > An example using Joel's recent test run (thanks Joel :)).
> >
> >
> > No problem. I do want to point out that you can't tell that the tests
> ran on sis
> > rather than Tsim or some specific hardware. For leon3, you can add qemu
> to the
> > list of potential "platforms". I've mentioned this before.
>
> Maybe the `rtems-test` command line option name is misleading. It is
> `--rtems-bsp=` but it is more like `--bsp-target=`. Some BSPs have more
> than one
> way to be run, for example psim and psim-run. I see in one of the tests I
> linked
> too the command line has `--rtems-bsp=leon2-sis`.
>
> > While typing this, I had the idea that maybe adding a note argument that
> gets
> > added to the report might be an easy solution. Whoever runs the test
> could
> > freeform annotate the board model, simulator and version, etc. This
> would at
> > least allow the logs to show which platform when we need to look for an
> issue.
>
> If you think there should be a list of annotations a test run needs then
> please
> create a ticket with a list.
>

This would be enough to address my "HW or which simulator" question.

>
> > Also I am not sure but hopefully the test reports do accurately reflect
> host OS.
>
> There is a "Host" section at the top of the results log? It is just `uname
> -a`.
>

I think that's sufficient as long as it can distinguish Linux
distributions.

--joel

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

Re: [PATCH v2] rtems: Add rtems_task_create_from_config()

[Chris Johns]

On 9/9/20 8:43 am, Joel Sherrill wrote:
> On Wed, Sep 2, 2020 at 11:09 AM Sebastian Huber
>  >
> wrote:
> +/**
> + * @brief Returns the recommended task storage area size for the 
> specified size
> + *   and task attributes.
> + *
> + * @param _size is the size dedicated to the task stack and thread-local
> + *   storage.
> 
> How does the user get the TLS size? 
> 
> Need advice on that. Seems hard to get at compile time since it is a link
> time aggregation. 

I had not noticed this. Thank you for raising it.

Does the API call check this new size value against the size actually needed?

I thought the TLS size was set by the linker based on the declared TLS variables
in the code and that value is feed into the calculation of the memory when
dynamically allocating the task's space. I am now wondering if this is accounted
for if the allocation is from the workspace (sizeof(TLS) * tasks)? A linker
controlled size is fine for the statically linked only class of application
however it is broken for applications that dynamically load code with TLS
variables. There is a growing number of important applications dynamically
loading code so we need to consider it.

I would like to see us resolve how we manage TLS allocation for this case and
dynamic loading before I am OK with this change. I do not think it is fair to
have dynamic loading fit in or work around a change in this area.

FreeBSD and Linux dynamically allocate the TLS blocks and Linux resizes the
blocks when dynamically loaded code contains TLS variables. That approach is not
as easy on RTEMS for a range of reasons that are not important here.

A single allocation for all the data a task needs is attractive. It allows this
API to work and it saves the heap block overheads when using an allocator. It
however means the TLS size needs to be set to the maximum for an application
including all dynamically loaded code.

Maybe the ability to set the size in confdefs.h at the system level would be
sufficient. Defining it at the task level is misleading because it implies tasks
can have different TLS sizes and they cannot. A system level TLS size of 0 could
be used to have the linker size be the size used, this would be compatible to
what we currently have. A system level size of 0 for this new API call would be
a fatal error the there are TLS variables.

TLS management in RTEMS is similar to the SDATA area the PowerPC has. The only
difference is the TLS size can be varied and the SDATA size is fixed in
hardware. If a system level TLS size is supported and that value has a suitable
interface libdl can support dynamically loaded TLS variables the same way SDATA
is handled on the PowerPC. Libdl uses a bit-allocator to track 32bit blocks of
SDATA. The statically linked usage is known and initialised and libdl assumes
ownership of the remaining space. The TLS block can be handled the same way. The
RTL lock handles the runtime set up without effecting scheduling and the same
can happen for TLS data.

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

Re: BSP Test Results

[Chris Johns]

On 9/9/20 8:50 am, Joel Sherrill wrote:
> On Mon, Sep 7, 2020 at 12:44 AM Chris Johns  > wrote:
> On 7/9/20 2:16 pm, Joel Sherrill wrote:
> > On Sun, Sep 6, 2020, 9:55 PM Chris Johns  
> > >> wrote:
> >
> >     Hello,
> >
> >     I would like to discuss BSP Test results early in the release cycle 
> in
> the hope
> >     we avoid the last minute issues we encountered with RTEMS 5 and the
> "expected"
> >     failure state ticket.
> >
> >     I would like to update this section ...
> >
> >   
>  
> https://docs.rtems.org/branches/master/user/testing/tests.html#expected-test-states
> >
> >     to state there is to be a ticket for each `expected-fail` test 
> state.
> I believe
> >     this was the outcome of the discussion that took place. Please 
> correct
> me if
> >     this is not correct.
> >
> >     The purpose of the `expected-fail` is to aid the accounting of the
> test results
> >     to let us know if there are any regressions. We need to account for
> tests that
> >     fail so we can track if a recent commit results in a new failure, 
> i.e. a
> >     regression. To do this we need to capture the state in a way
> `rtems-test` can
> >     indicate a regression.
> >
> >     I think the `indeterminate` state may need further explanation as it
> will help
> >     in the cases a simulator passes a test but the test fails on some
> hardware. I am
> >     currently seeing this with spcache01 on the PC BSP.
> >
> >
> > I don't mind this as long as we have a rigid format to ensure these are
> easy to
> > find.
> 
> Do you mean find the location in the tcfg files?
> 
> No. I just mean sometimes there is a comment directly above the l
> exclude line, sometimes above a group, etc. I would want something
> like
> 
> exclude: dl06  # #1234 dynamic loading broken on RISC-V

I assume you mean `expected-fail`? Excluded tests are just that excluded, i.e.
not enough memory, so a ticket will not change that.

> Maybe even not as a comment but a ticket number so it could be
> checked that every exclude had a ticket number in the form of #[N]
> (four or five digits).

Oh of course. The format for expected-fail could be extended to include a ticket
number so it _has_ to be there? Comments tend to be fragile. A ticket number is
something I think can be implemented without too many issues.

Should the indeterminate state also have a ticket assigned?

I am fine to add this support but it does not mean I will be adding all the
tickets and the expected-fail states. It is a project wide task we all need to
help with. I just hope it happens rather than being left like last time. :)

> > Perhaps info at the main be of the line marking this state. In the past,
> > we have tended to our comments before.
> 
> If you mean the location in the tcfg files being printed out by the test, 
> have
> no idea how to do that. What about grep?
> 
> 
> grep works if on the same line. Hence the above suggestion. If we want to
> add excludes, let's document which test and ticket in a way that theoretically
> we could check if the ticket is open or closed.  
> 
> No way to automate that it applies to this BSP's exclude I think.

Let me have a look to see what can be done now I understand it is a ticket
number you would like to have in the test.

> >     With the level of continuous building and testing we are currently
> doing being
> >     able to easily determine a regression will become important. Check 
> out the
> >     example below.
> >
> >     I would like to avoid us sitting with failures that do not have
> tickets and are
> >     not accounted for. I know there is a lump of work to account for the
> failures
> >     and after that is done I think the effort needed to maintain the
> failure states
> >     will drop.
> >
> >     As a result I have been pondering how I can encourage this work be
> done. I am
> >     considering updating the tier-1 status to requiring there be 0
> unaccounted for
> >     failures. That is the `rtems-test`'s Failure count is 0 for a 
> hardware
> test run.
> >
> >     Chris
> >
> >     An example using Joel's recent test run (thanks Joel :)).
> >
> >
> > No problem. I do want to point out that you can't tell that the tests 
> ran
> on sis
> > rather than Tsim or some specific hardware. For leon3, you can add qemu 
> to the
> > list of potential "platforms". I've mentioned this before.
> 
> Maybe the `rtems-test` command line option name is misleading. It is
> `--rtems-bsp=` but it is more like `--bsp-target=`. Some BSPs have more 
> than one
> way to be run, for example psim and psim-run. I 

Re: BSP Test Results

[Chris Johns]

On 9/9/20 3:13 am, Gedare Bloom wrote:
> On Sun, Sep 6, 2020 at 8:55 PM Chris Johns  wrote:
>> An example using Joel's recent test run (thanks Joel :)). The sparc/leon2
>> results show no regressions:
>>
>> Summary
>> ===
>>
>> Passed:580
>> Failed:  0
>> User Input:  6
>> Expected Fail:   1
> Is there a ticket for this one? out of curiosity.

I do not think so. This and any others will need to be cleaned up once I
implement the change to support ticket numbers. I will do that as part of the
change seems I added the expected-fail states.

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

Re: pc386 BSP documentation.

[Chris Johns]

On 8/9/20 12:51 pm, Chris Johns wrote:
> On 8/9/20 5:57 am, Karel Gardas wrote:
>>
>> attached is a patch adding some pc386 BSP documentation.
>>
>> Review, comments, suggestions and/or commit of it are/is highly appreciated.
>>
> 
> This is a really good and welcome contribution so thank you.
> 
> I will commit it with a few minor changes in a day or so after others have had
> time to review it.

Pushed. Thanks

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

Re: [PATCH v2] rtems: Add rtems_task_create_from_config()

[Sebastian Huber]

On 09/09/2020 03:37, Chris Johns wrote:

On 9/9/20 8:43 am, Joel Sherrill wrote:

On Wed, Sep 2, 2020 at 11:09 AM Sebastian Huber
mailto:sebastian.hu...@embedded-brains.de>>
wrote:
 +/**
 + * @brief Returns the recommended task storage area size for the 
specified size
 + *   and task attributes.
 + *
 + * @param _size is the size dedicated to the task stack and thread-local
 + *   storage.

How does the user get the TLS size?

Need advice on that. Seems hard to get at compile time since it is a link
time aggregation.


I had not noticed this. Thank you for raising it.


The TLS size is only known after linking. The symbol _TLS_Size is 
defined by the linker script. The rtems_task_create_from_config() 
directives is for users who want to have full control over their memory 
allocation. These users have to know what they are doing. It is not for 
everyone.


Knowing the task size is also not an easy thing. It depends an what you 
call and what the compiler did.




Does the API call check this new size value against the size actually needed?


I think we should add a sanity check. We could for example check that 
after allocating the FP context and the TLS area, the size for the stack 
is greater than or equal to rtems_minimum_stack_size.




I thought the TLS size was set by the linker based on the declared TLS variables
in the code and that value is feed into the calculation of the memory when
dynamically allocating the task's space. I am now wondering if this is accounted
for if the allocation is from the workspace (sizeof(TLS) * tasks)? 


Yes, it is, see rtems_configuration_get_stack_space_size().


A linker
controlled size is fine for the statically linked only class of application
however it is broken for applications that dynamically load code with TLS
variables. There is a growing number of important applications dynamically
loading code so we need to consider it.

I would like to see us resolve how we manage TLS allocation for this case and
dynamic loading before I am OK with this change. I do not think it is fair to
have dynamic loading fit in or work around a change in this area.

FreeBSD and Linux dynamically allocate the TLS blocks and Linux resizes the
blocks when dynamically loaded code contains TLS variables. That approach is not
as easy on RTEMS for a range of reasons that are not important here.

A single allocation for all the data a task needs is attractive. It allows this
API to work and it saves the heap block overheads when using an allocator. It
however means the TLS size needs to be set to the maximum for an application
including all dynamically loaded code.

Maybe the ability to set the size in confdefs.h at the system level would be
sufficient. Defining it at the task level is misleading because it implies tasks
can have different TLS sizes and they cannot. A system level TLS size of 0 could
be used to have the linker size be the size used, this would be compatible to
what we currently have. 


I think this TLS issue with dynamic linking is independent of the 
rtems_task_create_from_config().


It would be easy to add a

CONFIGURE_MAXIMUM_THREAD_LOCAL_STORAGE_SIZE

application configuration option. If the TLS size defined by the linker 
is greater, then we would have a fatal system initialization error.  An 
extra space can be used by the dynamic linker.



A system level size of 0 for this new API call would be
a fatal error the there are TLS variables.


No, it don't think this should be an error. I think it would be enough 
to check that after allocating the FP context and the TLS area, the size 
for the stack is greater than or equal to rtems_minimum_stack_size.




TLS management in RTEMS is similar to the SDATA area the PowerPC has. The only
difference is the TLS size can be varied and the SDATA size is fixed in
hardware. If a system level TLS size is supported and that value has a suitable
interface libdl can support dynamically loaded TLS variables the same way SDATA
is handled on the PowerPC. Libdl uses a bit-allocator to track 32bit blocks of
SDATA. The statically linked usage is known and initialised and libdl assumes
ownership of the remaining space. The TLS block can be handled the same way. The
RTL lock handles the runtime set up without effecting scheduling and the same
can happen for TLS data.


Yes, this sounds good.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v2] rtems: Add rtems_task_create_from_config()

[Chris Johns]

On 9/9/20 3:14 pm, Sebastian Huber wrote:
> On 09/09/2020 03:37, Chris Johns wrote:
>> On 9/9/20 8:43 am, Joel Sherrill wrote:
>>> On Wed, Sep 2, 2020 at 11:09 AM Sebastian Huber
>>> >> >
>>> wrote:
>>>  +/**
>>>  + * @brief Returns the recommended task storage area size for the
>>> specified size
>>>  + *   and task attributes.
>>>  + *
>>>  + * @param _size is the size dedicated to the task stack and 
>>> thread-local
>>>  + *   storage.
>>>
>>> How does the user get the TLS size?
>>>
>>> Need advice on that. Seems hard to get at compile time since it is a link
>>> time aggregation.
>>
>> I had not noticed this. Thank you for raising it.
> 
> The TLS size is only known after linking. The symbol _TLS_Size is defined by 
> the
> linker script. 

Yes and this approach breaks dynamic loading but I cannot recall it or change 
it.

> The rtems_task_create_from_config() directives is for users who
> want to have full control over their memory allocation. These users have to 
> know
> what they are doing. It is not for everyone.

The API will exist without those consideration. I think that statement carries
no technical weight.

> Knowing the task size is also not an easy thing. It depends an what you call 
> and
> what the compiler did.

I am with Joel on this, the accounting needs to be accurate.

>> Does the API call check this new size value against the size actually needed?
> 
> I think we should add a sanity check. We could for example check that after
> allocating the FP context and the TLS area, the size for the stack is greater
> than or equal to rtems_minimum_stack_size.

Sure.

>> I thought the TLS size was set by the linker based on the declared TLS 
>> variables
>> in the code and that value is feed into the calculation of the memory when
>> dynamically allocating the task's space. I am now wondering if this is 
>> accounted
>> for if the allocation is from the workspace (sizeof(TLS) * tasks)? 
> 
> Yes, it is, see rtems_configuration_get_stack_space_size().

That looks like a runtime call? At the moment is the TLS data area allocated
from a workspace? If so is that space part of the work space size calculation?

>> A linker
>> controlled size is fine for the statically linked only class of application
>> however it is broken for applications that dynamically load code with TLS
>> variables. There is a growing number of important applications dynamically
>> loading code so we need to consider it.
>>
>> I would like to see us resolve how we manage TLS allocation for this case and
>> dynamic loading before I am OK with this change. I do not think it is fair to
>> have dynamic loading fit in or work around a change in this area.
>>
>> FreeBSD and Linux dynamically allocate the TLS blocks and Linux resizes the
>> blocks when dynamically loaded code contains TLS variables. That approach is 
>> not
>> as easy on RTEMS for a range of reasons that are not important here.
>>
>> A single allocation for all the data a task needs is attractive. It allows 
>> this
>> API to work and it saves the heap block overheads when using an allocator. It
>> however means the TLS size needs to be set to the maximum for an application
>> including all dynamically loaded code.
>>
>> Maybe the ability to set the size in confdefs.h at the system level would be
>> sufficient. Defining it at the task level is misleading because it implies 
>> tasks
>> can have different TLS sizes and they cannot. A system level TLS size of 0 
>> could
>> be used to have the linker size be the size used, this would be compatible to
>> what we currently have. 
> 
> I think this TLS issue with dynamic linking is independent of the
> rtems_task_create_from_config().

As the person who would have to add libdl support for TLS this change as it
stands makes a bunch of extra work to make this API OK and I would prefer not to
have to do that.

> It would be easy to add a
> 
> CONFIGURE_MAXIMUM_THREAD_LOCAL_STORAGE_SIZE
> 
> application configuration option. If the TLS size defined by the linker is
> greater, then we would have a fatal system initialization error.  An extra 
> space
> can be used by the dynamic linker.

Can CONFIGURE_MAXIMUM_THREAD_LOCAL_STORAGE_SIZE define the size of the TLS
section and if the linker overflows it we get an error? Not providing enough
space for dynamically loaded code is a runtime error returned by the dlopen call
so that is not an issue here.

>> A system level size of 0 for this new API call would be
>> a fatal error the there are TLS variables.
> 
> No, it don't think this should be an error. I think it would be enough to 
> check
> that after allocating the FP context and the TLS area, the size for the stack 
> is
> greater than or equal to rtems_minimum_stack_size.

If I understand Joel's question the user does not know the TLS size however it
is on the @param doco line so is the space accounted for in the statically
allocated task space or not? If it

Re: [PATCH v2] rtems: Add rtems_task_create_from_config()

[Sebastian Huber]

On 09/09/2020 07:56, Chris Johns wrote:

On 9/9/20 3:14 pm, Sebastian Huber wrote:

On 09/09/2020 03:37, Chris Johns wrote:

On 9/9/20 8:43 am, Joel Sherrill wrote:

On Wed, Sep 2, 2020 at 11:09 AM Sebastian Huber
mailto:sebastian.hu...@embedded-brains.de>>
wrote:
  +/**
  + * @brief Returns the recommended task storage area size for the
specified size
  + *   and task attributes.
  + *
  + * @param _size is the size dedicated to the task stack and thread-local
  + *   storage.

How does the user get the TLS size?

Need advice on that. Seems hard to get at compile time since it is a link
time aggregation.


I had not noticed this. Thank you for raising it.


The TLS size is only known after linking. The symbol _TLS_Size is defined by the
linker script.


Yes and this approach breaks dynamic loading but I cannot recall it or change 
it.


This approach is defined by the ABI.




The rtems_task_create_from_config() directives is for users who
want to have full control over their memory allocation. These users have to know
what they are doing. It is not for everyone.


The API will exist without those consideration. I think that statement carries
no technical weight.


Knowing the task size is also not an easy thing. It depends an what you call and
what the compiler did.


I am with Joel on this, the accounting needs to be accurate.


The TLS size is known a link-time and the size of the user-provided task 
storage area must be known at compile-time. These are the constraints we 
have. From my point of view all what we can do is a sanity check at 
run-time. Do you have a better approach?





Does the API call check this new size value against the size actually needed?


I think we should add a sanity check. We could for example check that after
allocating the FP context and the TLS area, the size for the stack is greater
than or equal to rtems_minimum_stack_size.


Sure.


I thought the TLS size was set by the linker based on the declared TLS variables
in the code and that value is feed into the calculation of the memory when
dynamically allocating the task's space. I am now wondering if this is accounted
for if the allocation is from the workspace (sizeof(TLS) * tasks)?


Yes, it is, see rtems_configuration_get_stack_space_size().


That looks like a runtime call? At the moment is the TLS data area allocated
from a workspace? If so is that space part of the work space size calculation?


Yes, see _Workspace_Handler_initialization().  The estimate from 
confdefs.h and the actual TLS size determine the workspace size.





A linker
controlled size is fine for the statically linked only class of application
however it is broken for applications that dynamically load code with TLS
variables. There is a growing number of important applications dynamically
loading code so we need to consider it.

I would like to see us resolve how we manage TLS allocation for this case and
dynamic loading before I am OK with this change. I do not think it is fair to
have dynamic loading fit in or work around a change in this area.

FreeBSD and Linux dynamically allocate the TLS blocks and Linux resizes the
blocks when dynamically loaded code contains TLS variables. That approach is not
as easy on RTEMS for a range of reasons that are not important here.

A single allocation for all the data a task needs is attractive. It allows this
API to work and it saves the heap block overheads when using an allocator. It
however means the TLS size needs to be set to the maximum for an application
including all dynamically loaded code.

Maybe the ability to set the size in confdefs.h at the system level would be
sufficient. Defining it at the task level is misleading because it implies tasks
can have different TLS sizes and they cannot. A system level TLS size of 0 could
be used to have the linker size be the size used, this would be compatible to
what we currently have.


I think this TLS issue with dynamic linking is independent of the
rtems_task_create_from_config().


As the person who would have to add libdl support for TLS this change as it
stands makes a bunch of extra work to make this API OK and I would prefer not to
have to do that.


What kind of extra work?




It would be easy to add a

CONFIGURE_MAXIMUM_THREAD_LOCAL_STORAGE_SIZE

application configuration option. If the TLS size defined by the linker is
greater, then we would have a fatal system initialization error.  An extra space
can be used by the dynamic linker.


Can CONFIGURE_MAXIMUM_THREAD_LOCAL_STORAGE_SIZE define the size of the TLS
section and if the linker overflows it we get an error?


Maybe we could make it a link-time error, but this would require some 
linker script magic. We would have to define the size as a symbol (we 
already do this for CONFIGURE_INTERRUPT_STACK_SIZE). I don't know you 
you can force a linker error if for example the value of symbol A > 
value of symbol B.



Not providing enough
space for dynamically loaded code is

RE: [PATCH v1 0/6] [libbsd] Fix e1000 driver for i386 in master and 5

[Jan.Sommer]

Hi Chris,

> -Original Message-
> From: Chris Johns 
> Sent: Friday, August 14, 2020 11:43 PM
> To: Sommer, Jan ; devel@rtems.org
> Subject: Re: [PATCH v1 0/6] [libbsd] Fix e1000 driver for i386 in master and 5
> 
> On 15/8/20 5:57 am, Jan Sommer wrote:
> > Hello,
> >
> > I finally got around to port the e1000 driver fixes which are already
> > present in the 5-freebsd-12 branch of rtems-libbsd also for the master
> > (and 5) branch.
> 
> Thanks. I will push these soon.
> 

As mentioned over at the users list, could you please push these changes?
They should enable compilation for i386 again and we could establish Ethernet 
connections with Intel network devices.
The patches should apply to libbsd master and 5 branch, as they are both based 
on FreeBSD master.

> I am not sure if the 5 branch in the rtems-libbsd.git repo is right and if it
> should be made from the 5-freebsd-12 branch? I feel it is currently confusing
> but either way it has issues.
> 

Maybe, we can continue to discuss that outside of this patch set? Renaming a 
branch later should be no trouble.
An option could be to rename 5 -> 5-freebsd-master, then the mapping between 
rtems and FreeBSD would always be explicit.

Cheers,

   Jan

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