gem5 icon indicating copy to clipboard operation
gem5 copied to clipboard

arch-x86: Fix TLB Assertion Error on CFLUSH

Open Lukas-Zenick opened this issue 10 months ago • 1 comments

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

Lukas-Zenick avatar Apr 28 '24 18:04 Lukas-Zenick

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.

hnpl avatar Apr 29 '24 01:04 hnpl

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

hnpl avatar May 24 '24 02:05 hnpl

Hi, I got to see this PR randomly as it is tagged as x86 but it actually affects every ISA as it touches the generic src/cpu/translation.hh code.

I believe the current solution, while it fixes the problem for X86 is a hack for all other ISAs and I don't see the reason why this couldn't be fixed properly in the x86 code. As far as I understood from a quick look at the issue, the problem arises as CFLUSH is treated as a read for permission checking:

// CLFLUSHOPT/WB/FLUSH is treated as read for protection checks but modelled as write assert(mode == state->mode || req->isCacheClean());

Well it shouldn't be hard to amend the permission checking logic in the TLB to treat the CacheClean as a corner case for BaseMMU::Write requests. I believe we do something similar in the ArmMMU code. I propose to revert this PR until things are done properly

giactra avatar Jun 03 '24 17:06 giactra

@giactra, we apologize for the oversight. I have created the revert PR.

ivanaamit avatar Jun 03 '24 21:06 ivanaamit

@giactra, we apologize for the oversight. I have created the revert PR.

No worries, thanks for handling this so quickly!

giactra avatar Jun 04 '24 10:06 giactra