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

arch-x86: Fix TLB Assertion Error on CFLUSH #1080

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Lukas-Zenick
Copy link

@Lukas-Zenick Lukas-Zenick commented Apr 28, 2024

Fixed the assertion statement in the cpu's translation.hh file so that it doesn't fail the assertion if the cache is clean.

I compile this c code to test

#include <stdio.h>

static inline void clflush(volatile void *p) {
    __asm__ volatile ("clflush (%0)" : : "r"(p) : "memory");
}

int main() {
    int data = 42;  // Example variable

    printf("Value before clflush: %d\n", data);

    clflush(&data);

    printf("Value after clflush: %d\n", data);

    return 0;
}

And run it with this script
./build/X86/gem5.opt configs/learning_gem5/part1/two_level.py ./test
In order to verify that it no longer fails the assertion check.

GitHub Issue: #862
Change-Id: I6004662e7c99f637ba0ddb07d205d1657708e99f

Change-Id: I6004662e7c99f637ba0ddb07d205d1657708e99f
@hnpl
Copy link
Contributor

hnpl commented Apr 29, 2024

I still think we need to have the assertion mode == state->mode without the ||. However, I'm okay with this if it means this change allows clflush to work.

@ivanaamit ivanaamit added the arch-x86 The X86 ISA label Apr 29, 2024
@@ -254,7 +254,7 @@ class DataTranslation : public BaseMMU::Translation
BaseMMU::Mode mode)
{
assert(state);
assert(mode == state->mode);
assert(mode == state->mode || req->isCacheClean());
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel a little uncomfortable with this change. However, if it is what makes the most sense (e.g., this can't be fixed by changing the original Mode, then I'm good with this change.

Thanks for the contribution!

@hnpl
Copy link
Contributor

hnpl commented May 24, 2024

Is there a plan to merge this to v24.0 release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-x86 The X86 ISA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants