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

jtagvpi: Don't use blocking IO, to avoid blocking the simulation #1350

Merged
merged 1 commit into from May 13, 2024

Conversation

rpls
Copy link
Contributor

@rpls rpls commented Mar 12, 2024

Closes #1349

Context, Motivation & Description

A first trivial fix for the blocking IO problem in the JTAG VPI implementation. Just adds a while loop with a sleep, which is similar to the behaviour of the JtagTCP implementation.

Impact on code generation

Shouldn't have any impact on code generation, just the simulation.

Checklist

  • Unit tests were added
  • API changes are or will be documented:

@Dolu1990
Copy link
Member

Seems good to me ^^

@rpls rpls marked this pull request as ready for review March 16, 2024 20:53
@rpls
Copy link
Contributor Author

rpls commented Mar 16, 2024

I raised the number of bytes to wait for a bit more.

@andreasWallner
Copy link
Collaborator

Why wait for the max size of a VPI command, not min size? (in the end the whole code is a bit sketchy since we might only have received half of a command until now, but that's not your fault)

@rpls
Copy link
Contributor Author

rpls commented Mar 17, 2024

Why wait for the max size of a VPI command, not min size?

Is there a minimum size? I was looking at that code and the corresponding JTAG VPI code I found around the net as well (OpenOCD, other projects that use VPI, etc.). Exactly because of the name MaxSize.... And as far as I can tell, each package consists of the full size. And the code following that also always loads the full size (readFully will always wait for enough data to fill the entire buffer array... which is sized at MaxSizeOfVpiCmd bytes).

@andreasWallner
Copy link
Collaborator

You are right, misremembered the code when the last review, sry

@Dolu1990
Copy link
Member

What if openocd send less than MaxSizeOfVpiCmd ? seems it is a dead lock.

          while(inputStream.available() < MaxSizeOfVpiCmd) {
            sleep(jtagClkPeriod * 200)
          }

Seems like the whole implementation (include before this patch) assume that once there is something in inputStream, there is enough for a whole packet. Meaning the following would be fine :

          while(inputStream.available() > 0) {
            sleep(jtagClkPeriod * 200)
          }

?

@andreasWallner
Copy link
Collaborator

@Dolu1990 "max size" is a bit of a misnomer it seems - from what I can tell (and what @rpls posted above) all VPI commands have the same length.
Since it's a TCP stream you can't rely on all bytes arriving at the same time, so there is a need to check for > MaxSizeOfVpiCmd

@Dolu1990
Copy link
Member

Ahhh ok ^^

Thanks !

@Dolu1990 Dolu1990 merged commit c150e90 into SpinalHDL:dev May 13, 2024
11 checks passed
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.

JTAG VPI Simulation gets blocked by IO
3 participants