| Message ID | 20240316015720.3661236-7-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | plugins: Rewrite plugin code generation | expand |
On 3/16/24 05:57, Richard Henderson wrote: > TCGHelperInfo includes the ABI for every function call. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/qemu/plugin.h | 1 + > plugins/core.c | 51 ++++++++++++++++++++++++++++++++++++++----- > 2 files changed, 46 insertions(+), 6 deletions(-) > > diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h > index 143262dca8..793c44f1f2 100644 > --- a/include/qemu/plugin.h > +++ b/include/qemu/plugin.h > @@ -92,6 +92,7 @@ struct qemu_plugin_dyn_cb { > union { > struct { > union qemu_plugin_cb_sig f; > + TCGHelperInfo *info; > } regular; > struct { > qemu_plugin_u64 entry; > diff --git a/plugins/core.c b/plugins/core.c > index 837c373690..b0a2e80874 100644 > --- a/plugins/core.c > +++ b/plugins/core.c > @@ -338,12 +338,26 @@ void plugin_register_dyn_cb__udata(GArray **arr, > enum qemu_plugin_cb_flags flags, > void *udata) > { > - struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); > + static TCGHelperInfo info[3] = { > + [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, > + [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN, > + [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN, > + /* > + * Match qemu_plugin_vcpu_udata_cb_t: > + * void (*)(uint32_t, void *) > + Any chance we could have a static assert ensuring this? I know it's possible in C11, but I don't know if glib offers something for this. */ > + [0 ... 2].typemask = (dh_typemask(void, 0) | > + dh_typemask(i32, 1) | > + dh_typemask(ptr, 2)) > + }; > > + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); > dyn_cb->userp = udata; > - /* Note flags are discarded as unused. */ > - dyn_cb->regular.f.vcpu_udata = cb; > dyn_cb->type = PLUGIN_CB_REGULAR; > + dyn_cb->regular.f.vcpu_udata = cb; > + > + assert((unsigned)flags < ARRAY_SIZE(info)); > + dyn_cb->regular.info = &info[flags]; > } > > void plugin_register_vcpu_mem_cb(GArray **arr, > @@ -352,14 +366,39 @@ void plugin_register_vcpu_mem_cb(GArray **arr, > enum qemu_plugin_mem_rw rw, > void *udata) > { > - struct qemu_plugin_dyn_cb *dyn_cb; > + /* > + * Expect that the underlying type for enum qemu_plugin_meminfo_t > + * is either int32_t or uint32_t, aka int or unsigned int. > + */ > + QEMU_BUILD_BUG_ON( > + !__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t) && > + !__builtin_types_compatible_p(qemu_plugin_meminfo_t, int32_t)); > > - dyn_cb = plugin_get_dyn_cb(arr); > + static TCGHelperInfo info[3] = { > + [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, > + [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN, > + [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN, > + /* > + * Match qemu_plugin_vcpu_mem_cb_t: > + * void (*)(uint32_t, qemu_plugin_meminfo_t, uint64_t, void *) > + */ > + [0 ... 2].typemask = > + (dh_typemask(void, 0) | > + dh_typemask(i32, 1) | > + (__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t) > + ? dh_typemask(i32, 2) : dh_typemask(s32, 2)) | > + dh_typemask(i64, 3) | > + dh_typemask(ptr, 4)) > + }; > + > + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); > dyn_cb->userp = udata; > - /* Note flags are discarded as unused. */ > dyn_cb->type = PLUGIN_CB_REGULAR; > dyn_cb->rw = rw; > dyn_cb->regular.f.vcpu_mem = cb; > + > + assert((unsigned)flags < ARRAY_SIZE(info)); > + dyn_cb->regular.info = &info[flags]; > } > > /* else, Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
On 3/19/24 03:12, Pierrick Bouvier wrote: > On 3/16/24 05:57, Richard Henderson wrote: >> TCGHelperInfo includes the ABI for every function call. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> include/qemu/plugin.h | 1 + >> plugins/core.c | 51 ++++++++++++++++++++++++++++++++++++++----- >> 2 files changed, 46 insertions(+), 6 deletions(-) >> >> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h >> index 143262dca8..793c44f1f2 100644 >> --- a/include/qemu/plugin.h >> +++ b/include/qemu/plugin.h >> @@ -92,6 +92,7 @@ struct qemu_plugin_dyn_cb { >> union { >> struct { >> union qemu_plugin_cb_sig f; >> + TCGHelperInfo *info; >> } regular; >> struct { >> qemu_plugin_u64 entry; >> diff --git a/plugins/core.c b/plugins/core.c >> index 837c373690..b0a2e80874 100644 >> --- a/plugins/core.c >> +++ b/plugins/core.c >> @@ -338,12 +338,26 @@ void plugin_register_dyn_cb__udata(GArray **arr, >> enum qemu_plugin_cb_flags flags, >> void *udata) >> { >> - struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); >> + static TCGHelperInfo info[3] = { >> + [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, >> + [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN, >> + [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN, >> + /* >> + * Match qemu_plugin_vcpu_udata_cb_t: >> + * void (*)(uint32_t, void *) >> + > > Any chance we could have a static assert ensuring this? > I know it's possible in C11, but I don't know if glib offers something for this. I don't see how. While you could ask questions about the pointer type qemu_plugin_vcpu_udata_cb_t, you can't ask questions about the function arguments. r~
On 3/19/24 23:51, Richard Henderson wrote: > On 3/19/24 03:12, Pierrick Bouvier wrote: >> On 3/16/24 05:57, Richard Henderson wrote: >>> TCGHelperInfo includes the ABI for every function call. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> include/qemu/plugin.h | 1 + >>> plugins/core.c | 51 ++++++++++++++++++++++++++++++++++++++----- >>> 2 files changed, 46 insertions(+), 6 deletions(-) >>> >>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h >>> index 143262dca8..793c44f1f2 100644 >>> --- a/include/qemu/plugin.h >>> +++ b/include/qemu/plugin.h >>> @@ -92,6 +92,7 @@ struct qemu_plugin_dyn_cb { >>> union { >>> struct { >>> union qemu_plugin_cb_sig f; >>> + TCGHelperInfo *info; >>> } regular; >>> struct { >>> qemu_plugin_u64 entry; >>> diff --git a/plugins/core.c b/plugins/core.c >>> index 837c373690..b0a2e80874 100644 >>> --- a/plugins/core.c >>> +++ b/plugins/core.c >>> @@ -338,12 +338,26 @@ void plugin_register_dyn_cb__udata(GArray **arr, >>> enum qemu_plugin_cb_flags flags, >>> void *udata) >>> { >>> - struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); >>> + static TCGHelperInfo info[3] = { >>> + [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, >>> + [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN, >>> + [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN, >>> + /* >>> + * Match qemu_plugin_vcpu_udata_cb_t: >>> + * void (*)(uint32_t, void *) >>> + >> >> Any chance we could have a static assert ensuring this? >> I know it's possible in C11, but I don't know if glib offers something for this. > > I don't see how. While you could ask questions about the pointer type > qemu_plugin_vcpu_udata_cb_t, you can't ask questions about the function arguments. > I was thinking about something similar to: static_assert(typeof(qemu_plugin_vcpu_udata_cb_t) == void (*)(uint32_t, void *)); But I don't think it's possible to express this in C standard before 11. > > r~
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h index 143262dca8..793c44f1f2 100644 --- a/include/qemu/plugin.h +++ b/include/qemu/plugin.h @@ -92,6 +92,7 @@ struct qemu_plugin_dyn_cb { union { struct { union qemu_plugin_cb_sig f; + TCGHelperInfo *info; } regular; struct { qemu_plugin_u64 entry; diff --git a/plugins/core.c b/plugins/core.c index 837c373690..b0a2e80874 100644 --- a/plugins/core.c +++ b/plugins/core.c @@ -338,12 +338,26 @@ void plugin_register_dyn_cb__udata(GArray **arr, enum qemu_plugin_cb_flags flags, void *udata) { - struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); + static TCGHelperInfo info[3] = { + [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, + [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN, + [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN, + /* + * Match qemu_plugin_vcpu_udata_cb_t: + * void (*)(uint32_t, void *) + */ + [0 ... 2].typemask = (dh_typemask(void, 0) | + dh_typemask(i32, 1) | + dh_typemask(ptr, 2)) + }; + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); dyn_cb->userp = udata; - /* Note flags are discarded as unused. */ - dyn_cb->regular.f.vcpu_udata = cb; dyn_cb->type = PLUGIN_CB_REGULAR; + dyn_cb->regular.f.vcpu_udata = cb; + + assert((unsigned)flags < ARRAY_SIZE(info)); + dyn_cb->regular.info = &info[flags]; } void plugin_register_vcpu_mem_cb(GArray **arr, @@ -352,14 +366,39 @@ void plugin_register_vcpu_mem_cb(GArray **arr, enum qemu_plugin_mem_rw rw, void *udata) { - struct qemu_plugin_dyn_cb *dyn_cb; + /* + * Expect that the underlying type for enum qemu_plugin_meminfo_t + * is either int32_t or uint32_t, aka int or unsigned int. + */ + QEMU_BUILD_BUG_ON( + !__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t) && + !__builtin_types_compatible_p(qemu_plugin_meminfo_t, int32_t)); - dyn_cb = plugin_get_dyn_cb(arr); + static TCGHelperInfo info[3] = { + [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, + [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN, + [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN, + /* + * Match qemu_plugin_vcpu_mem_cb_t: + * void (*)(uint32_t, qemu_plugin_meminfo_t, uint64_t, void *) + */ + [0 ... 2].typemask = + (dh_typemask(void, 0) | + dh_typemask(i32, 1) | + (__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t) + ? dh_typemask(i32, 2) : dh_typemask(s32, 2)) | + dh_typemask(i64, 3) | + dh_typemask(ptr, 4)) + }; + + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr); dyn_cb->userp = udata; - /* Note flags are discarded as unused. */ dyn_cb->type = PLUGIN_CB_REGULAR; dyn_cb->rw = rw; dyn_cb->regular.f.vcpu_mem = cb; + + assert((unsigned)flags < ARRAY_SIZE(info)); + dyn_cb->regular.info = &info[flags]; } /*
TCGHelperInfo includes the ABI for every function call. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/qemu/plugin.h | 1 + plugins/core.c | 51 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 6 deletions(-)