[gem5-dev] Change in gem5/gem5[master]: mem: Add invalid context id check on LLSC checks
Tiago Mück has submitted this change and it was merged. ( https://gem5-review.googlesource.com/c/public/gem5/+/18792 ) Change subject: mem: Add invalid context id check on LLSC checks .. mem: Add invalid context id check on LLSC checks If the request's address is in the LLSC list, its context Id was being fetched unconditionally, which could cause the assert at Request::contextId() to fail. Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Signed-off-by: Tiago Muck Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/18792 Reviewed-by: Nikos Nikoleris Maintainer: Nikos Nikoleris Tested-by: kokoro --- M src/mem/abstract_mem.cc M src/mem/abstract_mem.hh 2 files changed, 8 insertions(+), 3 deletions(-) Approvals: Nikos Nikoleris: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc index a998530..6870ba3 100644 --- a/src/mem/abstract_mem.cc +++ b/src/mem/abstract_mem.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2012,2017-2018 ARM Limited + * Copyright (c) 2010-2012,2017-2019 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -284,7 +284,10 @@ DPRINTF(LLSC, "Erasing lock record: context %d addr %#x\n", i->contextId, paddr); ContextID owner_cid = i->contextId; -ContextID requester_cid = pkt->req->contextId(); +assert(owner_cid != InvalidContextID); +ContextID requester_cid = req->hasContextId() ? + req->contextId() : + InvalidContextID; if (owner_cid != requester_cid) { ThreadContext* ctx = system()->getThreadContext(owner_cid); TheISA::globalClearExclusive(ctx); diff --git a/src/mem/abstract_mem.hh b/src/mem/abstract_mem.hh index 18d8ee9..8b944b9 100644 --- a/src/mem/abstract_mem.hh +++ b/src/mem/abstract_mem.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012 ARM Limited + * Copyright (c) 2012, 2019 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -83,6 +83,8 @@ // check for matching execution context bool matchesContext(const RequestPtr &req) const { +assert(contextId != InvalidContextID); +assert(req->hasContextId()); return (contextId == req->contextId()); } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/18792 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: master Gerrit-Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Gerrit-Change-Number: 18792 Gerrit-PatchSet: 4 Gerrit-Owner: Tiago Mück Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Nikos Nikoleris Gerrit-Reviewer: Tiago Mück Gerrit-Reviewer: Xianwei Zhang Gerrit-Reviewer: kokoro Gerrit-CC: Srikant Bharadwaj Gerrit-MessageType: merged ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Change in gem5/gem5[master]: mem: Add invalid context id check on LLSC checks
Hello kokoro, Daniel Carvalho, Xianwei Zhang, Nikos Nikoleris, I'd like you to reexamine a change. Please visit https://gem5-review.googlesource.com/c/public/gem5/+/18792 to look at the new patch set (#3). Change subject: mem: Add invalid context id check on LLSC checks .. mem: Add invalid context id check on LLSC checks If the request's address is in the LLSC list, its context Id was being fetched unconditionally, which could cause the assert at Request::contextId() to fail. Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Signed-off-by: Tiago Muck --- M src/mem/abstract_mem.cc M src/mem/abstract_mem.hh 2 files changed, 8 insertions(+), 3 deletions(-) -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/18792 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: master Gerrit-Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Gerrit-Change-Number: 18792 Gerrit-PatchSet: 3 Gerrit-Owner: Tiago Mück Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Nikos Nikoleris Gerrit-Reviewer: Tiago Mück Gerrit-Reviewer: Xianwei Zhang Gerrit-Reviewer: kokoro Gerrit-CC: Srikant Bharadwaj Gerrit-MessageType: newpatchset ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Change in gem5/gem5[master]: mem: Add invalid context id check on LLSC checks
Hello Xianwei Zhang, I'd like you to reexamine a change. Please visit https://gem5-review.googlesource.com/c/public/gem5/+/18792 to look at the new patch set (#2). Change subject: mem: Add invalid context id check on LLSC checks .. mem: Add invalid context id check on LLSC checks If the request's address is in the LLSC list, its context Id was being fetched unconditionally, which could cause the assert at Request::contextId() to fail. Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Signed-off-by: Tiago Muck --- M src/mem/abstract_mem.cc M src/mem/abstract_mem.hh 2 files changed, 8 insertions(+), 3 deletions(-) -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/18792 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: master Gerrit-Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Gerrit-Change-Number: 18792 Gerrit-PatchSet: 2 Gerrit-Owner: Tiago Mück Gerrit-Reviewer: Tiago Mück Gerrit-Reviewer: Xianwei Zhang Gerrit-CC: Daniel Carvalho Gerrit-CC: Srikant Bharadwaj Gerrit-MessageType: newpatchset ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Change in gem5/gem5[master]: mem: Add invalid context id check on LLSC checks
Tiago Mück has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/18792 Change subject: mem: Add invalid context id check on LLSC checks .. mem: Add invalid context id check on LLSC checks If the request's address is in the LLSC list, its context Id was being fetched unconditionally, which could cause the assert at Request::contextId() to fail. Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Signed-off-by: Tiago Muck --- M src/mem/abstract_mem.cc 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc index a998530..ce40dca 100644 --- a/src/mem/abstract_mem.cc +++ b/src/mem/abstract_mem.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2012,2017-2018 ARM Limited + * Copyright (c) 2010-2012,2017-2019 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -258,6 +258,7 @@ if (isLLSC) { while (i != lockedAddrList.end()) { +assert(req->hasContextId()); if (i->addr == paddr && i->matchesContext(req)) { // it's a store conditional, and as far as the memory system can // tell, the requesting context's lock is still valid. @@ -284,7 +285,10 @@ DPRINTF(LLSC, "Erasing lock record: context %d addr %#x\n", i->contextId, paddr); ContextID owner_cid = i->contextId; -ContextID requester_cid = pkt->req->contextId(); +assert(owner_cid != InvalidContextID); +ContextID requester_cid = pkt->req->hasContextId() + ? pkt->req->contextId() + : InvalidContextID; if (owner_cid != requester_cid) { ThreadContext* ctx = system()->getThreadContext(owner_cid); TheISA::globalClearExclusive(ctx); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/18792 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: master Gerrit-Change-Id: Iae9791f81c8fe9a7fcd842cd8ab7db18f34f2808 Gerrit-Change-Number: 18792 Gerrit-PatchSet: 1 Gerrit-Owner: Tiago Mück Gerrit-MessageType: newchange ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev