Skip to content
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.

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

mcp750-vxworks PSP hardcodes core as "cfe-core.o" #111

Closed
skliper opened this issue Oct 21, 2019 · 9 comments · Fixed by #207 or #209
Closed

mcp750-vxworks PSP hardcodes core as "cfe-core.o" #111

skliper opened this issue Oct 21, 2019 · 9 comments · Fixed by #207 or #209
Assignees
Labels
bug Something isn't working Priority: Mission Feature or bug related to stakeholder needs
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Oct 21, 2019

Requested feature
CFE_PSP_GetCFETextSegmentInfo fails if CFE_MODULE_NAME doesn't match what was run, currently hardcoded to "cfe-core.o". Could make lookup more general so it wouldn't require hardcoded name.

To Reproduce
Steps to reproduce the behavior:

  1. Build for vxworks, use core name other than cfe-core.o (cfe-core.exe)
  2. Execute
  3. Checksum of text segment will report as 0xFFFF due to failed moduleFindByName on hardcoded cfe-core.o

Expected behavior
Checksum should work

Code snips
See fsw/mcp750-vxworks/src/cfe_psp_memory.c lines related to CFE_MODULE_NAME

System observed on:

  • Hardware: MCP750
  • OS: VxWorks6.9
  • Versions 6.7 bundle

Additional context
None

Reporter Info
Jacob Hageman - NASA/GSFC

@skliper skliper added bug Something isn't working enhancement New feature or request and removed bug Something isn't working labels Oct 21, 2019
@skliper skliper added this to the 1.6.0 milestone Oct 6, 2020
@skliper skliper added the Priority: Mission Feature or bug related to stakeholder needs label Oct 6, 2020
@skliper
Copy link
Contributor Author

skliper commented Oct 6, 2020

Note the suffix is here:

set(CMAKE_EXECUTABLE_SUFFIX ".exe")

@skliper
Copy link
Contributor Author

skliper commented Oct 6, 2020

ping @ejtimmon, @wmoleski - turns out we opened this issue about a year ago...

@skliper skliper added bug Something isn't working and removed enhancement New feature or request labels Oct 6, 2020
@skliper
Copy link
Contributor Author

skliper commented Oct 6, 2020

Switching to a bug, since this breaks on any multiple build system, since the target name is different. @jphickey how about removing the hardcoded value, extract it from the target properties and passing in as a -D to the PSP build?

@skliper
Copy link
Contributor Author

skliper commented Oct 6, 2020

or add to target_config.c, and get it from there?

@skliper
Copy link
Contributor Author

skliper commented Oct 7, 2020

Probably most resilient solution would be for the startCfeCore to pass the the moduleID in (no dependence on actual name). Second to that, could search the ID list (moduleIdListGet) for a partial name match ("core-" or "core-" + CPUNAME), and least resilient would be using the original build name as described above. The 2nd and 3rd option rely on part or all of the name not getting changed, which is an unfortunate "hidden" restriction on the user.

@jphickey
Copy link
Contributor

jphickey commented Oct 7, 2020

As far as I know, the cfeSupport.c file is somehow built and linked directly with the kernel. We don't currently build this as part of the CFE build - it is provided for example purposes only. One would copy this into their kernel build and modify to suit their particular BSP. Also the "main" routine / entry point to OSAL BSP is just a void function, for simplicity - no args.

My suggestion is to do both:

  1. Implement a new routine in cfeSupport.c akin to GetWrsKernelTextStart() and GetWrsKernelTextEnd() that returns the module ID of the cfe core module, if it was loaded via startCfeCore. Otherwise this can return NULL. Let's call it GetCfeCoreModuleID()
  2. Update the GLOBAL_PSP_CONFIGDATA configuration structure to include the expected name of the executable/library from the CMake script that built it.
  3. Then at runtime, use OS_SymbolLookup to determine if GetCfeCoreModuleID() is defined. If it is, call it, and store the return.
  4. If GetCfeCoreModuleID() either returned NULL or the function wasn't defined/provided at all, then use the name from GLOBAL_PSP_CONFIGDATA and pass it to moduleFindByName() to get the module ID.
  5. If either 3 (preferred) or 4 (fallback) yielded a valid module ID - use it to fulfill the CFE_PSP_GetCFETextSegmentInfo() information for computing the CRC.

With this approach one can get the full/complete/ideal solution if they rebuild their kernel and start CFE via startCfeCore(), but still get something that works if debugging and loading via the shell, or if they have not rebuilt the kernel to include this extra function.

@jphickey
Copy link
Contributor

jphickey commented Oct 7, 2020

I can implement the above if you want, but I won't be able to rebuild the VxWorks kernel AFAIK, as this requires use of the wind river IDE. I can do items 2-5 above though, if you agree with that approach. Someone else with access to the proper tools will have to rebuild the kernel.

@jphickey
Copy link
Contributor

jphickey commented Oct 7, 2020

As a side note - it is probably worth also considering that the way the CFE does the CRC calculation here is somewhat broken at the fundamental level. On a system with actual memory protection and a sufficiently capable MMU, usually the text pages are marked "execute only" for security reasons. On this type of system access of the code/text pages as data, as CFE does to compute the CRC, will trigger a segmentation fault.

It may be worth considering a completely alternative API where the CRC is computed when the module is loaded, via any available means on the platform, and a PSP API to retrieve that pre-calculated CRC.

@skliper
Copy link
Contributor Author

skliper commented Oct 7, 2020

We've got the capability to rebuild the kernel... but lets just do 2-5 for now. If the error reporting could be improved (when a match can't be found), and a functional level test could be added (via the test app capability) that would also help us avoid future breakage.

jphickey added a commit to jphickey/PSP that referenced this issue Oct 14, 2020
Improve the module ID lookup when getting the CFE core text segment info.

- Ideally get the ID directly from what was loaded by startCfeCore
- As a fallback use the actual CFE core name from the configdata

Do not use a hardcoded name.
astrogeco added a commit that referenced this issue Oct 20, 2020
Fix #111, do not assume a specific core name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Priority: Mission Feature or bug related to stakeholder needs
Projects
None yet
2 participants