contrib/plugins/Makefile | 1 + contrib/plugins/cache.c | 595 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 596 insertions(+) create mode 100644 contrib/plugins/cache.c
In this RFC patch series, I propose an initial cache modelling TCG plugin. As of now, it models separate L1 data cache and L1 instruction cache. It supports three eviction policies: LRU, random, and FIFO. Once a policy is chosen, it's used for both instruction and data caches. v1 -> v2: Unlocked dmtx on early return in vcpu_mem_access & removed a (probably?) bad InsnData free. This is probably still problematic since it does not free the ``idata`` allocated for the vcpu_mem_access callback even once, but if it's placed, it would double-free it. How do I mitigate this? I need to free the InsnData passed to vcpu_mem_access only once if we find out that it's an IO access since we do not need it anymore and it will early return every time. Mahmoud Mandour (3): plugins: Added a new cache modelling plugin plugins: cache: Enabled parameterization and added trace printing plugins: cache: Added FIFO and LRU eviction policies. contrib/plugins/Makefile | 1 + contrib/plugins/cache.c | 595 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 596 insertions(+) create mode 100644 contrib/plugins/cache.c -- 2.25.1
On Sun, May 30, 2021 at 8:37 AM Mahmoud Mandour <ma.mandourr@gmail.com> wrote: > In this RFC patch series, I propose an initial cache modelling TCG > plugin. As of now, it models separate L1 data cache and L1 instruction > cache. It supports three eviction policies: LRU, random, and FIFO. Once > a policy is chosen, it's used for both instruction and data caches. > > v1 -> v2: Unlocked dmtx on early return in vcpu_mem_access & removed a > (probably?) bad InsnData free. > This is probably still problematic since it does not free the > ``idata`` allocated for the vcpu_mem_access callback even > once, but if it's placed, it would double-free it. > How do I mitigate this? I need to free the InsnData passed to > vcpu_mem_access only once if we find out that it's an IO > access since we do not need it anymore and it will early > return every time. > > Mahmoud Mandour (3): > plugins: Added a new cache modelling plugin > plugins: cache: Enabled parameterization and added trace printing > plugins: cache: Added FIFO and LRU eviction policies. > > contrib/plugins/Makefile | 1 + > contrib/plugins/cache.c | 595 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 596 insertions(+) > create mode 100644 contrib/plugins/cache.c > > -- > 2.25.1 > >
Mahmoud Mandour <ma.mandourr@gmail.com> writes: > In this RFC patch series, I propose an initial cache modelling TCG > plugin. As of now, it models separate L1 data cache and L1 instruction > cache. It supports three eviction policies: LRU, random, and FIFO. Once > a policy is chosen, it's used for both instruction and data caches. > > v1 -> v2: Unlocked dmtx on early return in vcpu_mem_access & removed a > (probably?) bad InsnData free. > This is probably still problematic since it does not free the > ``idata`` allocated for the vcpu_mem_access callback even > once, but if it's placed, it would double-free it. > How do I mitigate this? I need to free the InsnData passed to > vcpu_mem_access only once if we find out that it's an IO > access since we do not need it anymore and it will early > return every time. OK I've done my review, sorry I reviewed 1/3 from the previous series. I've made some comments inline in those patches but I think this is an excellent start to the project. I think we can get the core plugin up-streamed fairly quickly and then spend some time examining better integration and possibly enhancing the plugin API. -- Alex Bennée
On Tue, Jun 1, 2021 at 3:32 PM Alex Bennée <alex.bennee@linaro.org> wrote: > > Mahmoud Mandour <ma.mandourr@gmail.com> writes: > > > In this RFC patch series, I propose an initial cache modelling TCG > > plugin. As of now, it models separate L1 data cache and L1 instruction > > cache. It supports three eviction policies: LRU, random, and FIFO. Once > > a policy is chosen, it's used for both instruction and data caches. > > > > v1 -> v2: Unlocked dmtx on early return in vcpu_mem_access & removed a > > (probably?) bad InsnData free. > > This is probably still problematic since it does not free the > > ``idata`` allocated for the vcpu_mem_access callback even > > once, but if it's placed, it would double-free it. > > How do I mitigate this? I need to free the InsnData passed to > > vcpu_mem_access only once if we find out that it's an IO > > access since we do not need it anymore and it will early > > return every time. > > OK I've done my review, sorry I reviewed 1/3 from the previous series. > I've made some comments inline in those patches but I think this is an > excellent start to the project. I think we can get the core plugin > up-streamed fairly quickly and then spend some time examining better > integration and possibly enhancing the plugin API. > > -- > Alex Bennée > Thanks so much for taking the time to thoroughly and also quickly review the series. The feedback is greatly appreciated and I've implemented most of it, I want your approval about the LRU discussion and I think that I will be able to repost a cleaner version. Thanks, Mahmoud
© 2016 - 2024 Red Hat, Inc.