Skip to content

Xtensa dev #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
erhankur wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Xtensa dev #2

erhankur wants to merge 7 commits into from

Conversation

@erhankur
Copy link
Owner

@erhankur erhankur commented May 27, 2022

Pull Request Template

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • This change requires a documentation update

User Impact

Please describe the impact of this change. This can be a list of positive and/or negative impacts.

Performance Impact

Please describe any relevant performance impact of this change. This can be positive or negative impact. How did you characterize/test the performance impact?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

  • Test A
  • Test B

Hardware Configuration:

  • Development Kit:
  • Module or Chip Used:
  • Debug Adapter:

Software Configuration:

  • OpenOCD version:
  • Branch:
  • ESP_IDF version:
  • Operating System:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

@erhankur
Copy link
Owner Author

erhankur commented May 29, 2022

Thanks @ianstcdns, last commit is fixed the issue.

Looks like algorithm has been broken. I will look at it.

If you want to test, you can run openocd from the esp application main directory with
-c "program_esp_bins build flasher_args.json verify reset" -d3

@erhankur
Copy link
Owner Author

erhankur commented May 31, 2022

@ianstcdns this command -c "program_esp_bins build flasher_args.json verify reset" -d3 fails but other algorithm commands are OK such as flash probe 0 flash read_bank 0 tmp.rd 0 1000

So, for the upstream we don't need to spend time to fix it. I will investigate it in parallel.

erhankur
erhankur commented May 31, 2022
View reviewed changes
src/target/xtensa/xtensa_regs.h
XT_REG_IDX_AR13,
XT_REG_IDX_AR14,
XT_REG_IDX_AR15,
XT_REG_IDX_AR16,
Copy link
Owner Author

@erhankur erhankur May 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianstcdns Can you please clarify, what is the problem with keeping all the definitions here, so that I can use them in the rtos_xxx_stackings?

@ianstcdns
Copy link
Collaborator

ianstcdns commented May 31, 2022 via email

@erhankur
Copy link
Owner Author

erhankur commented May 31, 2022

The other, perhaps easier, option is to define them in an esp-core-specific file such that they match the order in the .cfg file. Maybe this is a reasonable first-step to getting this part of the code functional?

Yes this is probably the way I will go in the first place.

And my 2nd question;

How did you generate the xtensa-core-esp32s2.cfg ? What was your source for the register list and how did you mask them properly ? I would like to check and do for the others.

@ianstcdns
Copy link
Collaborator

ianstcdns commented May 31, 2022 via email

@erhankur
Copy link
Owner Author

erhankur commented May 31, 2022 •
edited
Loading

If there are more clean-ups you’ve found that Antonio/Tomas want to see, I’m happy to make those too. Really appreciate it.

Probably yes, some more formatting, type checks, casting needs to be resolved. I'll switch to your own branch, make changes and create a PR there. Then you can directly create a patch for the upstream.

Can you please update it with my latest patchset 44 and formatting I made here?

@ianstcdns
Copy link
Collaborator

ianstcdns commented May 31, 2022 via email

@erhankur
Copy link
Owner Author

erhankur commented May 31, 2022

6940 will be merged at the weekend I guess. Finally, there is nothing to do.

Actually before create your patch, I would like to push the guys for ESP32 review. It has SMP support and with ESP32-S2 it will be a good base for the other upcoming patches such as semihosting, flash support, FreeRTOS support.. And I don't think review will take much time as 6940.
And the other reason, they spent a lot of time for the xtensa review in 6940. With your new patch, we will almost drop all previous code base and will ask them another round of reviews. I think a small xtensa break would be better.

Of course, In the meantime, we can get your xtensa patch ready.

What do you think?

@ianstcdns
Copy link
Collaborator

ianstcdns commented May 31, 2022 via email

@erhankur
Copy link
Owner Author

erhankur commented Jun 1, 2022

Comparing to line count, it is %60 less. 5154 to 1981.

During freeze, review will go ahead with the limited workforce but nothing will be merged. To be realistic, I am expecting around 1 year for all Espressif patches.

@ianstcdns
Xtensa: initial attempt at xtensa-core-esp32s3.cfg
9d578bb 
Stubbed out TIE/user registers with misc0 contents
@erhankur erhankur mentioned this pull request Jun 14, 2022
Closed
erhankur added a commit that referenced this pull request Sep 23, 2022
@erhankur
rtos/esp32_stacking: AddressSanitizer: heap-buffer-overflow issue
90907b8 
    #0 0x10c41485f in __asan_memcpy+0x1af (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x4285f)
    #1 0x10ab3dcfd in buf_cpy binarybuffer.c:60
    #2 0x10ab2774b in rtos_generic_stack_read rtos.c:664
    #3 0x10aac3ed9 in freertos_get_thread_registers_from_stack FreeRTOS.c:1148
    #4 0x10aab2ea2 in freertos_get_thread_reg_list FreeRTOS.c:1238
    #5 0x10aab32b1 in freertos_get_thread_reg FreeRTOS.c:1269
    #6 0x10aabe903 in freertos_get_tasks_details FreeRTOS.c:797
    espressif#7 0x10aab270f in freertos_update_threads FreeRTOS.c:1060
    espressif#8 0x10ab1d66e in rtos_thread_packet rtos.c:395
    espressif#9 0x10ab1bad2 in gdb_thread_packet rtos.c:191
    espressif#10 0x10aa4643d in gdb_input_inner gdb_server.c:3547
    espressif#11 0x10aa3d534 in gdb_input gdb_server.c:3743
    espressif#12 0x10aa8e565 in server_loop server.c:566
    espressif#13 0x1099f6e66 in openocd_thread openocd.c:380
    espressif#14 0x1099f685e in openocd_main openocd.c:419
    espressif#15 0x1099f52b1 in main main.c:40
    espressif#16 0x7fff6bf78cc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@erhankur @ianstcdns