nasa / PSP

The Core Flight System (cFS) Platform Support Package (PSP)
Apache License 2.0
68 stars 56 forks source link

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

Closed skliper closed 3 years ago

skliper commented 4 years ago

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:

Additional context None

Reporter Info Jacob Hageman - NASA/GSFC

skliper commented 3 years ago

Note the suffix is here: https://github.com/nasa/PSP/blob/b383f7c657f97c997c8129a8853a5557eddef517/cmake/Modules/Platform/VxWorks-CFE.cmake#L12

skliper commented 3 years ago

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

skliper commented 3 years ago

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 commented 3 years ago

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

skliper commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.