Skip to content

Conversation

lyakh
Copy link

@lyakh lyakh commented Oct 7, 2020

Fixes for cAVS 1.8 (presumably, and beyond). I'll re-test with the newest SOF-integration branch though.

Some interrupts can be enabled by the ROM, e.g. the timer interrupt.
When then in Zephyr the interrupt controller is enabled, before
individual interrupts are configured, interrupts can arrive and lead
to the spurious interrupt handler being invoked. Fix thid by
disabling all child interrupts when configuring cAVS interrupt
controllers.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
lyakh added 9 commits October 8, 2020 15:40
SOF needs CONFIG_SYS_HEAP_ALIGNED_ALLOC enabled and until SMP is
supported on Xtensa Intel ADSPs, CONFIG_MP_NUM_CPUS has to be set
to 1.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
Unify default configurations to support both SMP and UP:

1. make SMP default, although it's currently disabled in prj.conf
2. use CAVS timer by default in both UP and SMP configurations
3. make MP_NUM_CPUS, IPM and IPM_CAVS_IDC depend on SMP

Signed-off-by: Guennadi Liakhovetski <[email protected]>
On cAVS 1.5, 2.0 and 2.5 platforms the correct manifest address is
0xB0032000.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
The current unused memory calculation is broken because it doesn't
take into account the stack area, allocated at the top of HP SRAM.
Until this is fixed disable powering down unused RAM.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
Shim register location on cAVS 1.5 is different than on 1.8 and up,
fix it.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
A configuration with CONFIG_MP_NUM_CPUS > 1 and CONFIG_IPM_CAVS_IDC not
defined is valid if COMFIG_SMP is disabled.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
If CONFIG_IPM_CAVS_IDC isn't selected, the file-global idc pointer in
soc_mp.c is unused.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
1. don't use "inline" in .c, let the compiler decide
2. remove superfluous parentheses
3. simplify a function by directly returning the result of a boolean
operation

Signed-off-by: Guennadi Liakhovetski <[email protected]>
soc/xtensa/intel_adsp/common/include/cavs/cpu.h redefines
PLATFORM_CORE_COUNT and PLATFORM_MASTER_CORE_ID which are already
defined in SOF. Be careful to avoid conflicts. Ideally one of the
two files should be removed.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
Copy link
Author

lyakh commented Oct 8, 2020

@andyross sorry, I pushed a wrong branch and it automatically closed this PR, fixed now.

The logging base address, provided by the LOG_BACKEND_RB_MEM_BASE
Kconfig option has been copied from cAVS 1.5, but it's different on
versions 1.8, 2.0 and 2.5. This patch fixes logtool functionality
on those platforms.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
andyross pushed a commit that referenced this pull request Nov 9, 2020
The _ldiv5() is an optimized divide-by-5 function that is smaller and
faster than the generic libgcc implementation.

Yet it can be made even smaller and faster with this replacement
implementation based on a reciprocal multiplication plus some tricks.

For example, here's the assembly from the original code on ARM:

_ldiv5:
        ldr     r3, [r0]
        movw    ip, zephyrproject-rtos#52429
        ldr     r1, [r0, #4]
        movt    ip, 52428
        adds    r3, r3, #2
        push    {r4, r5, r6, r7, lr}
        mov     lr, #0
        adc     r1, r1, lr
        adds    r2, lr, lr
        umull   r7, r6, ip, r1
        lsr     r6, r6, #2
        adc     r7, r6, r6
        adds    r2, r2, r2
        adc     r7, r7, r7
        adds    r2, r2, lr
        adc     r7, r7, r6
        subs    r3, r3, r2
        sbc     r7, r1, r7
        lsr     r2, r3, #3
        orr     r2, r2, r7, lsl zephyrproject-rtos#29
        umull   r2, r1, ip, r2
        lsr     r2, r1, #2
        lsr     r7, r1, zephyrproject-rtos#31
        lsl     r1, r2, #3
        adds    r4, lr, r1
        adc     r5, r6, r7
        adds    r2, r1, r1
        adds    r2, r2, r2
        adds    r2, r2, r1
        subs    r2, r3, r2
        umull   r3, r2, ip, r2
        lsr     r2, r2, #2
        adds    r4, r4, r2
        adc     r5, r5, #0
        strd    r4, [r0]
        pop     {r4, r5, r6, r7, pc}

And here's the resulting assembly with this commit applied:

_ldiv5:
        push    {r4, r5, r6, r7}
        movw    r4, zephyrproject-rtos#13107
        ldr     r6, [r0]
        movt    r4, 13107
        ldr     r1, [r0, #4]
        mov     r3, #0
        umull   r6, r7, r6, r4
        add     r2, r4, r4, lsl #1
        umull   r4, r5, r1, r4
        adds    r1, r6, r2
        adc     r2, r7, r2
        adds    ip, r6, r4
        adc     r1, r7, r5
        adds    r2, ip, r2
        adc     r2, r1, r3
        adds    r2, r4, r2
        adc     r3, r5, r3
        strd    r2, [r0]
        pop     {r4, r5, r6, r7}
        bx      lr

So we're down to 20 instructions from 36 initially, with only 2 umull
instructions instead of 3, and slightly smaller stack footprint.

Signed-off-by: Nicolas Pitre <[email protected]>
@lyakh
Copy link
Author

lyakh commented Jan 8, 2021

moved to zephyrproject-rtos#31127

@lyakh lyakh closed this Jan 8, 2021
@lyakh lyakh deleted the cavs18 branch January 8, 2021 15:49
andyross pushed a commit that referenced this pull request Mar 31, 2021
The fatal log now contains
- Trap type in human readable representation
- Integer registers visible to the program when trap was taken
- Special register values such as PC and PSR
- Backtrace with PC and SP

If CONFIG_EXTRA_EXCEPTION_INFO is enabled, then all the above is
logged. If not, only the special registers are logged.

The format is inspired by the GRMON debug monitor and TSIM simulator.
A quick guide on how to use the values is in fatal.c.

It now looks like this:

E: tt = 0x02, illegal_instruction
E:
E:       INS        LOCALS     OUTS       GLOBALS
E:   0:  00000000   f3900fc0   40007c50   00000000
E:   1:  00000000   40004bf0   40008d30   40008c00
E:   2:  00000000   40004bf4   40008000   00000003
E:   3:  40009158   00000000   40009000   00000002
E:   4:  40008fa8   40003c00   40008fa8   00000008
E:   5:  40009000   f3400fc0   00000000   00000080
E:   6:  4000a1f8   40000050   4000a190   00000000
E:   7:  40002308   00000000   40001fb8   000000c1
E:
E: psr: f30000c7   wim: 00000008   tbr: 40000020   y: 00000000
E:  pc: 4000a1f4   npc: 4000a1f8
E:
E:       pc         sp
E:  #0   4000a1f4   4000a190
E:  #1   40002308   4000a1f8
E:  #2   40003b24   4000a258

Signed-off-by: Martin Åberg <[email protected]>
andyross pushed a commit that referenced this pull request Nov 14, 2024
With introduction of Raw modes, nRF70 driver now advertises get_c
onfig OP, but doesn't implement all types.

This causes problems two-fold with checksum calculations:
  1. The "config" isn't uninitialized, so, every call returns differnet
     values. So, for UDP header checksum would be done and
     pkt->chksumdone would be set. But for IPv4 header checksum might be
     skipped.
  2. Even if we initialize to zero, then network stack gets all zeros
     and calculates checksum by itself rendering offload moot.

There is another problem in #1, as there is only single flag for pkt for
all checksum, nRF70 driver sees this and tells UMAC to skip checksum for
the entire packet. The design isn't coherent, and should be converted to
communicate per-type checksum status (some are filled by network stack
and some HW).

But as nRF70 support all checksum offloads, advertise all types for both
RX and TX.

Upstream PR #: 80882

Signed-off-by: Chaitanya Tata <[email protected]>
andyross added a commit that referenced this pull request Aug 10, 2025
…CH=y

I'm at a loss here.  The feature this test case wants to see (on
ARMv7M, not ARMv6M) is the ability to take a irq_lock() inside a
system call and then see that future system calls from the same thread
continue to hold the lock.

That's not documented AFAICT.  It's also just a terrible idea because either:

1. The obvious denial of service implications if user code is allowed
   to run in an unpreemptible mode, or:

2. The broken locking promise if this is implemented to release the
   lock and reacquire it in an attempt to avoid #1.

(FWIW: my read of the code is that #1 is the current implementation.
But hilariously the test isn't able to tell the difference!)

And in any case it's not how any of our other platforms work (or can
work, in some cases), making this a non-portable system call
API/feature at best.

Leave it in place for now out of conservatism, but disable with the
new arch_switch() code, whose behavior matches that of other Zephyr
userspaces.

Signed-off-by: Andy Ross <[email protected]>
andyross added a commit that referenced this pull request Aug 10, 2025
…CH=y

I'm at a loss here.  The feature this test case wants to see (on
ARMv7M, not ARMv6M) is the ability to take a irq_lock() inside a
system call and then see that future system calls from the same thread
continue to hold the lock.

That's not documented AFAICT.  It's also just a terrible idea because either:

1. The obvious denial of service implications if user code is allowed
   to run in an unpreemptible mode, or:

2. The broken locking promise if this is implemented to release the
   lock and reacquire it in an attempt to avoid #1.

(FWIW: my read of the code is that #1 is the current implementation.
But hilariously the test isn't able to tell the difference!)

And in any case it's not how any of our other platforms work (or can
work, in some cases), making this a non-portable system call
API/feature at best.

Leave it in place for now out of conservatism, but disable with the
new arch_switch() code, whose behavior matches that of other Zephyr
userspaces.

Signed-off-by: Andy Ross <[email protected]>
andyross added a commit that referenced this pull request Aug 10, 2025
I'm at a loss here.  The feature this test case wants to see (on
ARMv7M, not ARMv6M) is the ability to take a irq_lock() inside a
system call and then see that future system calls from the same thread
continue to hold the lock.

That's not documented AFAICT.  It's also just a terrible idea because
either:

1. The obvious denial of service implications if user code is allowed
   to run in an unpreemptible mode, or:

2. The broken locking promise if this is implemented to release the
   lock and reacquire it in an attempt to avoid #1.

(FWIW: my read of the code is that #1 is the current implementation.
But hilariously the test isn't able to tell the difference!)

And in any case it's not how any of our other platforms work (or can
work, in some cases), making this a non-portable system call
API/feature at best.

Leave it in place for now out of conservatism, but disable with the
new arch_switch() code, whose behavior matches that of other Zephyr
userspaces.

Signed-off-by: Andy Ross <[email protected]>
andyross added a commit that referenced this pull request Aug 11, 2025
I'm at a loss here.  The feature this test case wants to see (on
ARMv7M, not ARMv6M) is the ability to take a irq_lock() inside a
system call and then see that future system calls from the same thread
continue to hold the lock.

That's not documented AFAICT.  It's also just a terrible idea because
either:

1. The obvious denial of service implications if user code is allowed
   to run in an unpreemptible mode, or:

2. The broken locking promise if this is implemented to release the
   lock and reacquire it in an attempt to avoid #1.

(FWIW: my read of the code is that #1 is the current implementation.
But hilariously the test isn't able to tell the difference!)

And in any case it's not how any of our other platforms work (or can
work, in some cases), making this a non-portable system call
API/feature at best.

Leave it in place for now out of conservatism, but disable with the
new arch_switch() code, whose behavior matches that of other Zephyr
userspaces.

Signed-off-by: Andy Ross <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant