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

Add pulp-offload-intf.c to stimulate and test CVA6-PULP cluster interference #255

Closed
wants to merge 5 commits into from

Conversation

luca-valente
Copy link
Collaborator

No description provided.

@luca-valente luca-valente removed the request for review from alex96295 February 28, 2024 10:16
Copy link
Contributor

@yvantor yvantor left a comment

Choose a reason for hiding this comment

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

@luca-valente I left a few comments.

carfield.mk Outdated Show resolved Hide resolved
@@ -61,9 +61,11 @@ CAR_ELFLOAD_BLOCKING_SAFED_SRC_C := $(CAR_SW_DIR)/tests/bare-metal/hostd/safed_o
CAR_ELFLOAD_BLOCKING_SAFED_PATH := $(basename $(CAR_ELFLOAD_BLOCKING_SAFED_SRC_C))
CAR_ELFLOAD_PULPD_SRC_C := $(CAR_SW_DIR)/tests/bare-metal/hostd/pulp-offload.c
CAR_ELFLOAD_PULPD_PATH := $(basename $(CAR_ELFLOAD_PULPD_SRC_C))
CAR_ELFLOAD_PULPD_INTF_SRC_C := $(CAR_SW_DIR)/tests/bare-metal/hostd/pulp-offload-intf.c
CAR_ELFLOAD_PULPD_INTF_PATH := $(basename $(CAR_ELFLOAD_PULPD_INTF_SRC_C))
Copy link
Contributor

@yvantor yvantor Mar 1, 2024

Choose a reason for hiding this comment

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

Do you think that doing:

CAR_ELFLOAD_PULPD_SRC_C += $(CAR_SW_DIR)/tests/bare-metal/hostd/pulp-offload-intf.c

Would work the same or might break? So we don't have to add dedicated variables for each new test we add (not sure it would work though).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto, we can append to the genric source variable CAR_ELFLOAD_PULPD_SRC_C

Copy link
Collaborator Author

@luca-valente luca-valente Mar 20, 2024

Choose a reason for hiding this comment

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

I don't think it would work. CAR_ELFLOAD_PULPD_SRC_C (i.e. $(CAR_SW_DIR)/tests/bare-metal/hostd/pulp-offload.c) is used as an argument of offload_tests_template. We would need to add a second for loop there to check all the source files and execute the command $(CHS_SW_CC) $(CAR_SW_INCLUDES) $(CHS_SW_CCFLAGS) -c $(3) -o $(4).$(basename $(notdir $(header))).car.o; 2 times, with $(3) being pulp-offload.c and pulp-offload-intf.c in the second iteration. Without adding the for loop, we would pass two files with two main to the cc compiler and it would break.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree. @alex96295 what do you think?

sw/sw.mk Show resolved Hide resolved
sw/sw.mk Show resolved Hide resolved
sw/tests/bare-metal/hostd/pulp-offload-intf.c Outdated Show resolved Hide resolved
@alex96295
Copy link
Collaborator

@luca-valente thanks for the addition of the test! I left few comments, mostly in line with what already commented by @yvantor

If you have time to go through the review, that would be great! Otherwise, ping us and we can have a look :)

@alex96295 alex96295 changed the title Add pulp-offload-intf.c so that CVA6 interferes with the PULP cluster running applications (and viceversa). Add pulp-offload-intf.c to stimulate and test CVA6-PULP cluster interference Mar 2, 2024
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.

3 participants