xtensa: use misc register to implement up_this_task macro

[chirping78]

Summary

This is the follow-up work of #13726 (percpu reg store this_task) on xtensa.
To help accelerate this_task(), use misc0 register to store the current running task tcb pointer.
And use the least significant bit of misc0 to indicate whether in interrupt context.

This feature is controlled by a config item XTENSA_PERCPU_TCB_IN_MISC0, which is currently off by default.

Impact

sched

Testing

ostest passed on these config: esp32s3-devkit:smp, esp32s3-devkit:tickless, esp32s2-saola-1:ostest,
with XTENSA_PERCPU_TCB_IN_MISC0 on and off (totally 6 combinations).

This feature does not work with esp32-devkitc:smp plus XTENSA_PERCPU_TCB_IN_MISC0 on, ostest fails at signal handler test.
Have no idea why it does not work with esp32, so set XTENSA_PERCPU_TCB_IN_MISC0 to be off by default.
Maybe change it to default n if ARCH_CHIP_ESP32 is a good idea?

[nuttxpr]

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a decent summary and testing information, it is lacking in several key areas:

• Impact is incomplete: The PR only states "sched" which is insufficient. It needs to address all the impact points explicitly (YES/NO with explanations): user impact, build impact, hardware impact (specifically mentioning ESP32 issues), documentation impact, security impact, compatibility impact.
• Testing logs are missing: The PR claims ostest passed but doesn't provide the actual logs as requested by the template. "Passed" is not enough; the logs themselves need to be included.
• Clarity on ESP32 Issues: The PR mentions the feature doesn't work with esp32-devkitc:smp and suggests a conditional config. This needs more investigation and a clearer explanation of the problem and proposed solution. Simply saying "Have no idea why" is unacceptable. If the root cause is unknown, it should be explicitly stated that further investigation is needed.
• Missing "Anything else to consider?": This section should be addressed, even if the answer is "N/A".

In short, the PR needs to be more thorough and address all the points in the template explicitly. The missing logs and vague explanation of the ESP32 issue are particularly concerning.

[tmedicci]

I will let it testing on our internal CI. Please don't merge it yet.

[tmedicci]

I will let it testing on our internal CI. Please don't merge it yet.

I found no errors running it as-is on our internal CI (for all existing defconfigs), but existing defconfigs don't enable this feature, so I will change it locally and test it again. Please wait.

[tmedicci]

This option is valid for devices that enables SMP, right?

If so, it should depend on it.

[xiaoxiang781216]

single core can get the benefit too

[tmedicci]

Alright, I'll enable it for every defconfig and let the CI test...

[tmedicci]

I'm sorry for the late response.

In fact, our CI shows that it doesn't work for ESP32 when SMP is enabled.

As it's something that affects all xtensa devices (and, in theory, shouldn't be specific for a specific chip), I'd more comfortable if you could @chirping78 investigate it further for ESP32 too...

[chirping78]

ESP32 SMP ostest fails at sighand test. It looks like the CPU1 hangs for some reason I don't know.

How about for now change the config item XTENSA_PERCPU_TCB_IN_MISC0 to:

  default y
  default n if ARCH_CHIP_ESP32

[xiaoxiang781216]

We have tested this patch with several chips from the different vendor, only ESP32 fail some case from ostest. Since ESP32 is an old chip and the code base is maintained actively by Espressif staff, so it's better to do more investigation on your side.

[tmedicci]

Sorry, @xiaoxiang781216 , I didn't see this answer.

Just as a complement to my comment (#15821 (comment)), although it was tested with "with several chips from the different vendors", these chips are not supported by NuttX (officially, NuttX supports only ESP32, ESP32-S2 and ESP32-S3. Custom xtensa-based chips are possible, but they weren't integrated to the tree).

Furthermore, ESP32 longevity is 15 years from 2016 (EOL is expected to be in 2031) (https://www.espressif.com/en/products/longevity-commitment), so it isn't an "old" chip that is going to be deprecated soon. Its code base (which is common for all xtensa devices) is not actively maintained by Espressif (it was first developed by Greg), so leaving it behind because "it fails" without a proper explanation doesn't make sense to me.

[xiaoxiang781216]

Sorry, @xiaoxiang781216 , I didn't see this answer.

Just as a complement to my comment (#15821 (comment)), although it was tested with "with several chips from the different vendors", these chips are not supported by NuttX (officially, NuttX supports only ESP32, ESP32-S2 and ESP32-S3. Custom xtensa-based chips are possible, but they weren't integrated to the tree).

please see #15821 (comment):

ostest passed on these config: esp32s3-devkit:smp, esp32s3-devkit:tickless, esp32s2-saola-1:ostest,
with XTENSA_PERCPU_TCB_IN_MISC0 on and off (totally 6 combinations).

This feature does not work with esp32-devkitc:smp plus XTENSA_PERCPU_TCB_IN_MISC0 on, ostest fails at signal handler test.

@chirping78 already test the change with esp32, esp32s3 and esp32s2, only esp32 doesn't work.

Furthermore, ESP32 longevity is 15 years from 2016 (EOL is expected to be in 2031) (https://www.espressif.com/en/products/longevity-commitment), so it isn't an "old" chip that is going to be deprecated soon. Its code base (which is common for all xtensa devices) is not actively maintained by Espressif (it was first developed by Greg), so leaving it behind because "it fails" without a proper explanation doesn't make sense to me.

If the same code work with esp32s2 and esp32s3, but not esp32. The best explanation should come from Espressif staff, the volunteer doesn't have enough internal information and help to understand the difference around these chips.

The change is minor and weill commented, so could you give me a proper explanation? esp32 maintainter.

[tmedicci]

If the same code work with esp32s2 and esp32s3, but not esp32. The best explanation should come from Espressif staff, the volunteer doesn't have enough internal information and help to understand the difference around these chips.

The only known difference is that ESP32 is LX6 (and the others are LX7). There aren't any other differences that justify it and that's the point: there is a unique device with such characteristics on NuttX upstream and this device is ESP32 (LX6 + SMP). This PR - which affects all xtensa devices - isn't working for this device. Maybe it wasn't extensively tested under diverse conditions. This should be further investigated.

Espressif doesn't maintain common code base.

[chirping78]

The 1st patchset missed the change to arch/xtensa/src/esp32/esp32_cpustart.c.
I have updated it in latest patch, this will make ESP32 SMP can boot, but still ostest will fail at sighand test if enable the config.

And also change XTENSA_PERCPU_TCB_IN_MISC0 to:

	default n if ARCH_CHIP_ESP32
	default y

To investigate why ESP32 SMP does not work is not easy, it needs long time and comprehensive understanding of ESP32 chip.
So I suggest to commit with current change, which will benefit all other chips, and leave a bug to investigate later.

[tmedicci]

The 1st patchset missed the change to arch/xtensa/src/esp32/esp32_cpustart.c. I have updated it in latest patch, this will make ESP32 SMP can boot, but still ostest will fail at sighand test if enable the config.

And also change XTENSA_PERCPU_TCB_IN_MISC0 to:

	default n if ARCH_CHIP_ESP32
	default y

Yes, I saw it. I tested with it.

To investigate why ESP32 SMP does not work is not easy, it needs long time and comprehensive understanding of ESP32 chip. So I suggest to commit with current change, which will benefit all other chips, and leave a bug to investigate later.

I'm not comfortable with merging it leaving ESP32 behind... This is an architecture optimization that doesn't work for a chip. The fact that it doesn't work for a known case of ESP32 may be related to some flaw in the design (maybe it isn't failing on other use cases because we are not testing it enough).