[RFC PATCH v2 0/3] Cache modelling TCG plugin

Mahmoud Mandour posted 3 patches 2 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/next-importer-push tags/patchew/20210530063712.6832-1-ma.mandourr@gmail.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>
contrib/plugins/Makefile |   1 +
contrib/plugins/cache.c  | 595 +++++++++++++++++++++++++++++++++++++++
2 files changed, 596 insertions(+)
create mode 100644 contrib/plugins/cache.c
[RFC PATCH v2 0/3] Cache modelling TCG plugin
Posted by Mahmoud Mandour 2 years, 10 months ago
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


Re: [RFC PATCH v2 0/3] Cache modelling TCG plugin
Posted by Mahmoud Mandour 2 years, 10 months ago
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
>
>
Re: [RFC PATCH v2 0/3] Cache modelling TCG plugin
Posted by Alex Bennée 2 years, 10 months ago
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

Re: [RFC PATCH v2 0/3] Cache modelling TCG plugin
Posted by Mahmoud Mandour 2 years, 10 months ago
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