Mahmoud Mandour <ma.mando...@gmail.com> writes:
> Made both icache and dcache configurable through plugin arguments.
>
> Signed-off-by: Mahmoud Mandour <ma.mando...@gmail.com>
> ---
> contrib/plugins/cache.c | 44 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
> index 715e5443b0..d8e8c750b6 100644
> --- a/contrib/plugins/cache.c
> +++ b/contrib/plugins/cache.c
> @@ -104,8 +104,17 @@ static inline uint64_t extract_set(struct Cache *cache,
> uint64_t addr)
> return (addr & cache->set_mask) >> cache->blksize_shift;
> }
>
> +static bool bad_cache_params(int blksize, int assoc, int cachesize)
> +{
> + return (cachesize % blksize) != 0 || (cachesize % (blksize * assoc) !=
> 0);
> +}
> +
> static struct Cache *cache_init(int blksize, int assoc, int cachesize)
> {
> + if (bad_cache_params(blksize, assoc, cachesize)) {
> + return NULL;
> + }
> +
> struct Cache *cache;
> int i;
> uint64_t blk_mask;
> @@ -403,8 +412,30 @@ int qemu_plugin_install(qemu_plugin_id_t id, const
> qemu_info_t *info,
>
> for (i = 0; i < argc; i++) {
> char *opt = argv[i];
> - if (g_str_has_prefix(opt, "limit=")) {
> - limit = g_ascii_strtoull(opt + 6, NULL, 10);
> + if (g_str_has_prefix(opt, "I=")) {
> + gchar **toks = g_strsplit(opt + 2, " ", -1);
I don't think this works because a space will trigger the shell to split
the args - the only way I could get it to work was by quoting the whole
-plugin argument. I know the syntax of optional plugin args is ugly as
hell but this should probably use "," like hwprofile.
> + if (g_strv_length(toks) != 3) {
> + g_strfreev(toks);
> + fprintf(stderr, "option parsing failed: %s\n", opt);
> + return -1;
> + }
> + icachesize = g_ascii_strtoull(toks[0], NULL, 10);
> + iassoc = g_ascii_strtoull(toks[1], NULL, 10);
> + iblksize = g_ascii_strtoull(toks[2], NULL, 10);
> + g_strfreev(toks);
> + } else if (g_str_has_prefix(opt, "D=")) {
> + gchar **toks = g_strsplit(opt + 2, " ", -1);
> + if (g_strv_length(toks) != 3) {
> + g_strfreev(toks);
> + fprintf(stderr, "option parsing failed: %s\n", opt);
> + return -1;
> + }
> + dcachesize = g_ascii_strtoull(toks[0], NULL, 10);
> + dassoc = g_ascii_strtoull(toks[1], NULL, 10);
> + dblksize = g_ascii_strtoull(toks[2], NULL, 10);
> + g_strfreev(toks);
> + } else if (g_str_has_prefix(opt, "limit=")) {
> + limit = g_ascii_strtoll(opt + 6, NULL, 10);
> } else {
> fprintf(stderr, "option parsing failed: %s\n", opt);
> return -1;
> @@ -412,7 +443,16 @@ int qemu_plugin_install(qemu_plugin_id_t id, const
> qemu_info_t *info,
> }
>
> dcache = cache_init(dblksize, dassoc, dcachesize);
> + if (!dcache) {
> + fprintf(stderr, "dcache cannot be constructed from given
> parameters\n");
> + return -1;
> + }
> +
Can we give users more of a hint of what's wrong? I suspect you'll need
to factor out a validate_cache_param and return a string that describes
the failure mode. Otherwise it gets frustrating to the user.
> icache = cache_init(iblksize, iassoc, icachesize);
> + if (!icache) {
> + fprintf(stderr, "icache cannot be constructed from given
> parameters\n");
> + return -1;
> + }
ditto
>
> qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
--
Alex Bennée
