shell: add shell backend for audio DSP using shared memory window

[kv2019i]

Shell backend for Intel ADSPs that uses Zephyr winstream as memory streaming protocol. Another two patches to add support to cavstool/acetool tools to use the shell from host.

Some notes on design rationale:

• debug slot number is hardcoded on purpose so that writing a client that can wait for DSP to boot up (or resume from suspend) is possible/easier
• con of above is that currently "mtrace" log backend needs to be disabled when shell backend is enabled -- should be pragmatic as one can route the logs to the shell output
• cavstool.py pty code based on example from @nklayman 's intel_adsp: cavs: add gdb support #49077
• cavstool.py and acetool.py code is still duplicated, this PR adds duplicated changes. we have a lot of CI/developer usage that assume a single file can be deployed, so I think it's better to keep these separate (and standalone) until we can unify to a single python script that can handle all targets (IOW carving out justt the shell code to a separate common py file, doesn't make a lot of sense)

[kv2019i]

V2:

• quick fixes for the cavstool.py/acetool.py to address pylint warnings and the unintended "cmds" leftover

[andyross]

The Zephyr code looks great to me, though I'm not a shell expert and will leave the review of that API to others, except to say that I certainly don't see anything wrong.

But the python... heh. I complained the first time when acetool split from cavstool, and this is why. :)

We really can't be committing the same new change in two cut/paste variants. Options:

• Unify the two scripts, maybe with two different loader implementations. They can share all the non-hardware code, and a decent amount of "library" stuff too (the Regs class, sysfs/PCI/hugepage handling, logging, etc...). Definitely my favorite.
• Put the new code in a library used by both. This will require some care in deployment, because you can't just copy a script to a target anymore. I know multi-file python tres can be packaged as zip files, that might be an option. @marc-hb no doubt has input here.

Also one note about the python bytewise access being a reliability bug. Probably not a blocker, but something we should remember.

[andyross]

Style: would be smaller and easier to read if you just cast that ctx pointer in the argument to sys_winstream_write(), or provide it in a local predicate function like "shell_memwin()". There are a lot of bytes involved in typing out that local variable declaration and cast, and it gets repeated a bunch.

[kv2019i]

I left this in place as same verbose style is used in all shell backends..

[andyross]

Do note that this is actually a historical reliability problem with the python implementation. The algorithm is lockless and relies on the values changing atomically[1]. If we're doing bytewise I/O the firmware can end up seeing corrupt values, which will turn up as runtime errors (ideally a stream reset unless winstream itself has bugs).

For the specific case of a python script doing debug or logging access... it's probably OK. (Though now that we're writing to the firmware there are new opportunities for hijinx). If we want to do this right we probably need to be doing ctypes access to the shared memory buffer and not the bytearray stuff.

[1] And in the right order. In the case of write() (because winstream doesn't promise stream reliability in the case of overrun) all that's required is that the "end" field be updated only after all the bytes have been copied in and the start/len fields updated.

[kv2019i]

Changed this to 32bit writes via mmap slice in V4. Not sure if this is guaranteed to work, but a 32bit mmap write should be fairly reliable. I did not remember to put "end" to last, so I'll take a look at that still.

[marc-hb]

I complained the first time when acetool split from cavstool, and this is why. :)

• intel_adsp: ace: add firmware loading tool #66293

Unify the two scripts, maybe with two different loader implementations.

+1. It's not that much code in total.

Put the new code in a library used by both. This will require some care in deployment, because you can't just copy a script to a target anymore. I know multi-file python tres can be packaged as zip files, that might be an option. @marc-hb no doubt has input here.

I don't know anything about Python packaging unfortunately. We could simply deploy 3 files (ace.py, cavs.py, common.py), that should hopefully be a very small CI change and it would get the deduplication at least started...

[kv2019i]

Thanks @andyross and @marc-hb for review feedback. Let me go ahead and do the tool deduplication first. Marking this as draft for now. Deduplication PR submitted as #72799 . I'll resubmit this after the deduplication one is merged.

[andyross]

I don't know anything about Python packaging unfortunately.

FWIW I dug around and turned up this tiny package, which seems to do exactly what's needed (basically it just packages a zip file with a shebang line that allows it to run as a standalone script). https://docs.python.org/3/library/zipapp.html

Not an issue anymore now that the scripts are merged, but I'm probably going to need to come back to it myself. The mt8195 integration is resubmitted, and is using a copy of the Regs class, and will need to do likewise with winstream (or an evolved version with correct atomicity and ordering control -- I'm better at ctypes now than I was then) once I get that wired up.

[kv2019i]

v4:

• dependency PR merged, rebased this one and marked ready for review
• some further cleanup to python code based on @andyross feedback

[kv2019i]

Removing DNM as v4 is merge ready. The winstream implementation can be improved, but not sure if we want to do it this same PR.

@andyross , as alternative/improvement, I have a local version that uses Regs to access. If this seems like a good direction, I can update the winstream_read code as well to use this and get rid of the byte access to:

+    global bar4_mem
     (bar4_mem, bar4_mmap) = bar_map(pcidir, 4)
     dsp = Regs(bar4_mem)
     if adsp_is_ace():
@@ -708,10 +709,20 @@ def idx_mod(wlen, idx):
 def idx_sub(wlen, a, b):
     return idx_mod(wlen, a + (wlen - b))
 
+def winstream_reg_hdr(base):
+    hdr = Regs(bar4_mem + base)
+    hdr.WLEN  = 0x00
+    hdr.START = 0x04
+    hdr.END   = 0x08
+    hdr.SEQ   = 0x0c
+    hdr.freeze()
+    return hdr
+
 # Python implementation of the same algorithm in sys_winstream_write(),
 # see there for details.
 def winstream_write(base, msg):
     (wlen, start, end, seq) = win_hdr(base)
+    hdr = winstream_reg_hdr(base)
     if wlen > SHELL_MAX_VALID_SLOT_SIZE:
         log.debug("DSP powered off at winstream_write")
         return
@@ -726,9 +737,9 @@ def winstream_write(base, msg):
     if seq != 0:
         avail = (wlen - 1) - idx_sub(wlen, end, start)
         if lenmsg > avail:
-            start = idx_mod(wlen, start + (lenmsg - avail))
+            hdr.START = idx_mod(wlen, start + (lenmsg - avail))
     if lenmsg < lenmsg0:
-        start = end
+        hdr.START = end
         drop = lenmsg0 - lenmsg
         msg = msg[drop : lenmsg - drop]
     suffix = min(lenmsg, wlen - end)
@@ -737,15 +748,8 @@ def winstream_write(base, msg):
     if lenmsg > suffix:
         for c in range(0, lenmsg - suffix):
             bar4_mmap[base + 16 + c] = msg[suffix + c]
-    end = idx_mod(wlen, end + lenmsg)
-    seq += lenmsg0
-    # write back updated fields as 32bit writes
-    update_hdr = struct.pack("<III", start, end, seq)
-    dst = base + 4 # skip wlen
-    for c in range(0, 3):
-        src = c * 4
-        bar4_mmap[dst : dst + 4] = update_hdr[src : src + 4]
-        dst += 4
+    hdr.END = idx_mod(wlen, end + lenmsg)
+    hdr.SEQ += lenmsg0

[andyross]

Looks good to me. Regarding the Regs use in the winstream code: no complaints here, that looks perfect. Though ideally we'd want to pair that with some kind of lower level memcpy() style thing for the stream content too. IIRC there are/were performance edges with the python polling where a handful of the tests emit too fast and roll the (comparatively small) buffer over too fast for the script to keep up.

[kv2019i]

v5:

• fixed two conformance issues
• rebased to latest main

Should be ready to go ahead now.

[andyross]

Looks great here. We should still get a shell expert to +1 just to make sure the backend isn't breaking any rules; that's not my area.

[kv2019i]

@jakub-uC let me know if I can help with the process for a new shell backend. This is my first contribution to shell subsystem, so hopefully all mandatory things for a new backend are in place.

[jakub-uC]

Looks good, some of the hard-coded settings can be moved to the Kconfig.

[kv2019i]

v6:

• update to the backend commit (no updates to python/cavstool.py code) to address comments from @jakub-uC (thanks for review!)

[jakub-uC]

LGTM, please only implement the minor change in the Kconfig file.

[kv2019i]

v3 patchset:

• added ifdef around the adsp_memory_window specific Kconfig option to align with other backends (address review comment from @jakub-uC )
• no other changes

[andyross]

Still +1 from me on the DSP side

[jsarha]

Just a question and a suggestion. The code looks good to me.

[jsarha]

What is the point of (struct shell_memwindow *)-cast here? .ctx is void *, and I can not even easily the declaration or definition of struct shell_memwindow anywhere (must be some weird macro somewhere).

[jsarha]

Wouldn't it make sense to depend on !LOG_BACKEND_ADSP_MTRACE too?