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

risc-v/mpfs: ihc: don't wait for master if it's already up #163

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

eenurkka
Copy link

After rebooting this ihc client hart only, ihc will hang as it's waiting for the initial handshake that just won't be there. Detect this situation and let the rptun / ihc know the master is already up. This makes the RPMSG bus usable even after NuttX reboot, in case there's not much traffic. However, Linux is likely to mark the virtio queue broken if there's descent amount of traffic. This makes the bus no longer function.

Summary

Fix NuttX <-> Linux RPMSG after NuttX reboots

Impact

NuttX <-> Linux RPMSG bus after NuttX reboot

Testing

Saluki v2

@eenurkka eenurkka requested review from jpaali and jlaitine September 18, 2023 06:13
@eenurkka
Copy link
Author

Will send new version soon

@eenurkka eenurkka force-pushed the rpmsg-reboot branch 2 times, most recently from 919cafa to 62d48d6 Compare September 19, 2023 10:53

/* g_vq_idx may be temporarily illegal after a warm boot */

if ((g_vq_idx - VRING0_NOTIFYID) >= VRINGS)
Copy link

@jlaitine jlaitine Sep 19, 2023

Choose a reason for hiding this comment

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

Somehow I find this fishy.

After a warm re-boot, initially g_vq_idx is initialized to 0, which == VRING0_NOTIFYID . Thus after a warm reboot this just always passes?

Would it be cleaner to just initialize g_vq_idx to some invalid value (e.g. 0xffff) and compare directly to that? That is,

#define VRING_UNINITIALIZED 0xffff
static uint16_t g_vq_idx = VRING_UNINITIALIZED;

...

if (g_vq_idx == VRING_UNINITIALIZED)
  {
     return;
  } 

Copy link
Author

Choose a reason for hiding this comment

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

After a warm boot, g_vq_idx is sometimes (1/10 or so) garbaged, so it won't be 0xffff, maybe the illegal value comes from the msg. Maybe I need to dig in more where the garbaged semi-random value comes from.

But setting it to an illegal value, eg 0xffff sounds like a good idea, but cannot compare to that as g_vq_idx is a rather random value when it corrupts.

Copy link
Author

Choose a reason for hiding this comment

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

 .sbss.g_rptun_initialized
                0x00000000a060f30c        0x1 ../../NuttX/nuttx/arch/risc-v/src/libarch.a(mpfs_ihc.o)
 *fill*         0x00000000a060f30d        0x1
 .sbss.g_vq_idx
                0x00000000a060f30e        0x2 ../../NuttX/nuttx/arch/risc-v/src/libarch.a(mpfs_ihc.o)
 .sbss._ZGVN10ModuleBaseI3ADCE7_objectE
                0x00000000a060f310        0x8 ../../src/drivers/adc/board_adc/libdrivers__board_adc.a(ADC.cpp.obj)
                0x00000000a060f310                guard variable for ModuleBase<ADC>::_object
 .sbss._ZN7cm8jl655g_devE
                0x00000000a060f318        0x8 ../../src/drivers/distance_sensor/cm8jl65/libdrivers__cm8jl65.a(cm8jl65_m>                0x00000000a060f318                cm8jl65::g_dev

I'll check if the neighbors overflow...

Choose a reason for hiding this comment

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

I guess it can only come through mpfs_ihc_interrupt, is there some spurious irq at boot up?

After rebooting this ihc client hart only, ihc will hang as it's
waiting for the initial handshake that just won't be there. Detect
this situation and let the rptun / ihc know the master is already
up. This makes the RPMSG bus usable even after NuttX reboot, in
case there's not much traffic. However, Linux is likely to mark
the virtio queue broken if there's descent amount of traffic. This
makes the bus no longer function.

Also deny uninitialized early access for particular structs.

Signed-off-by: Eero Nurkkala <[email protected]>
@eenurkka eenurkka merged commit 1e8a8f3 into master Sep 26, 2023
@eenurkka eenurkka deleted the rpmsg-reboot branch November 13, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants