| Message ID | 20240502180847.287673-1-pierrick.bouvier@linaro.org |
|---|---|
| Headers | show |
| Series | TCG plugins new inline operations | expand |
Contrary to what the cover letter says, all patches have been reviewed in the series since v4. On 5/2/24 11:08, Pierrick Bouvier wrote: > This series implement two new operations for plugins: > - Store inline allows to write a specific value to a scoreboard. > - Conditional callback executes a callback only when a given condition is true. > The condition is evaluated inline. > > It's possible to mix various inline operations (add, store) with conditional > callbacks, allowing efficient "trap" based counters. > > It builds on top of new scoreboard API, introduced in the previous series. > > NOTE: Two patches still need review > > v2 > -- > > - fixed issue with udata not being passed to conditional callback > - added specific test for this in tests/plugin/inline.c (udata was NULL before). > > v3 > -- > > - rebased on top of "plugins: Rewrite plugin code generation": > 20240316015720.3661236-1-richard.henderson@linaro.org > - single pass code generation > - small cleanups for code generation > > v4 > -- > > - remove op field from qemu_plugin_inline_cb > - use tcg_constant_i64 to load immediate value to store > > v5 > -- > > - rebase on top of master now that Richard's series was merged > > Pierrick Bouvier (9): > plugins: prepare introduction of new inline ops > plugins: extract generate ptr for qemu_plugin_u64 > plugins: add new inline op STORE_U64 > tests/plugin/inline: add test for STORE_U64 inline op > plugins: conditional callbacks > tests/plugin/inline: add test for conditional callback > plugins: distinct types for callbacks > plugins: extract cpu_index generate > plugins: remove op from qemu_plugin_inline_cb > > include/qemu/plugin.h | 42 +++++++---- > include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++- > plugins/plugin.h | 12 +++- > accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++-------- > plugins/api.c | 39 ++++++++++ > plugins/core.c | 109 ++++++++++++++++++++-------- > tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++-- > plugins/qemu-plugins.symbols | 2 + > 8 files changed, 466 insertions(+), 84 deletions(-) >
Hi Pierrick, On 5/2/24 3:08 PM, Pierrick Bouvier wrote: > This series implement two new operations for plugins: > - Store inline allows to write a specific value to a scoreboard. > - Conditional callback executes a callback only when a given condition is true. > The condition is evaluated inline. > > It's possible to mix various inline operations (add, store) with conditional > callbacks, allowing efficient "trap" based counters. > > It builds on top of new scoreboard API, introduced in the previous series. > > NOTE: Two patches still need review > > v2 > -- > > - fixed issue with udata not being passed to conditional callback > - added specific test for this in tests/plugin/inline.c (udata was NULL before). > > v3 > -- > > - rebased on top of "plugins: Rewrite plugin code generation": > 20240316015720.3661236-1-richard.henderson@linaro.org > - single pass code generation > - small cleanups for code generation > > v4 > -- > > - remove op field from qemu_plugin_inline_cb > - use tcg_constant_i64 to load immediate value to store > > v5 > -- > > - rebase on top of master now that Richard's series was merged > > Pierrick Bouvier (9): > plugins: prepare introduction of new inline ops > plugins: extract generate ptr for qemu_plugin_u64 > plugins: add new inline op STORE_U64 > tests/plugin/inline: add test for STORE_U64 inline op > plugins: conditional callbacks > tests/plugin/inline: add test for conditional callback > plugins: distinct types for callbacks > plugins: extract cpu_index generate > plugins: remove op from qemu_plugin_inline_cb > > include/qemu/plugin.h | 42 +++++++---- > include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++- > plugins/plugin.h | 12 +++- > accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++-------- > plugins/api.c | 39 ++++++++++ > plugins/core.c | 109 ++++++++++++++++++++-------- > tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++-- > plugins/qemu-plugins.symbols | 2 + > 8 files changed, 466 insertions(+), 84 deletions(-) The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing. Is this intentional? Cheers, Gustavo
Hi Gustavo, On 5/2/24 12:16, Gustavo Romero wrote: > Hi Pierrick, > > On 5/2/24 3:08 PM, Pierrick Bouvier wrote: >> This series implement two new operations for plugins: >> - Store inline allows to write a specific value to a scoreboard. >> - Conditional callback executes a callback only when a given condition is true. >> The condition is evaluated inline. >> >> It's possible to mix various inline operations (add, store) with conditional >> callbacks, allowing efficient "trap" based counters. >> >> It builds on top of new scoreboard API, introduced in the previous series. >> >> NOTE: Two patches still need review >> >> v2 >> -- >> >> - fixed issue with udata not being passed to conditional callback >> - added specific test for this in tests/plugin/inline.c (udata was NULL before). >> >> v3 >> -- >> >> - rebased on top of "plugins: Rewrite plugin code generation": >> 20240316015720.3661236-1-richard.henderson@linaro.org >> - single pass code generation >> - small cleanups for code generation >> >> v4 >> -- >> >> - remove op field from qemu_plugin_inline_cb >> - use tcg_constant_i64 to load immediate value to store >> >> v5 >> -- >> >> - rebase on top of master now that Richard's series was merged >> >> Pierrick Bouvier (9): >> plugins: prepare introduction of new inline ops >> plugins: extract generate ptr for qemu_plugin_u64 >> plugins: add new inline op STORE_U64 >> tests/plugin/inline: add test for STORE_U64 inline op >> plugins: conditional callbacks >> tests/plugin/inline: add test for conditional callback >> plugins: distinct types for callbacks >> plugins: extract cpu_index generate >> plugins: remove op from qemu_plugin_inline_cb >> >> include/qemu/plugin.h | 42 +++++++---- >> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++- >> plugins/plugin.h | 12 +++- >> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++-------- >> plugins/api.c | 39 ++++++++++ >> plugins/core.c | 109 ++++++++++++++++++++-------- >> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++-- >> plugins/qemu-plugins.symbols | 2 + >> 8 files changed, 466 insertions(+), 84 deletions(-) > > The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing. > > Is this intentional? > Do you mean there is no multiline commit message for those changes? Indeed, for some of those patches, the change is a single line commit message. > > Cheers, > Gustavo
On 5/2/24 4:45 PM, Pierrick Bouvier wrote: > Hi Gustavo, > > On 5/2/24 12:16, Gustavo Romero wrote: >> Hi Pierrick, >> >> On 5/2/24 3:08 PM, Pierrick Bouvier wrote: >>> This series implement two new operations for plugins: >>> - Store inline allows to write a specific value to a scoreboard. >>> - Conditional callback executes a callback only when a given condition is true. >>> The condition is evaluated inline. >>> >>> It's possible to mix various inline operations (add, store) with conditional >>> callbacks, allowing efficient "trap" based counters. >>> >>> It builds on top of new scoreboard API, introduced in the previous series. >>> >>> NOTE: Two patches still need review >>> >>> v2 >>> -- >>> >>> - fixed issue with udata not being passed to conditional callback >>> - added specific test for this in tests/plugin/inline.c (udata was NULL before). >>> >>> v3 >>> -- >>> >>> - rebased on top of "plugins: Rewrite plugin code generation": >>> 20240316015720.3661236-1-richard.henderson@linaro.org >>> - single pass code generation >>> - small cleanups for code generation >>> >>> v4 >>> -- >>> >>> - remove op field from qemu_plugin_inline_cb >>> - use tcg_constant_i64 to load immediate value to store >>> >>> v5 >>> -- >>> >>> - rebase on top of master now that Richard's series was merged >>> >>> Pierrick Bouvier (9): >>> plugins: prepare introduction of new inline ops >>> plugins: extract generate ptr for qemu_plugin_u64 >>> plugins: add new inline op STORE_U64 >>> tests/plugin/inline: add test for STORE_U64 inline op >>> plugins: conditional callbacks >>> tests/plugin/inline: add test for conditional callback >>> plugins: distinct types for callbacks >>> plugins: extract cpu_index generate >>> plugins: remove op from qemu_plugin_inline_cb >>> >>> include/qemu/plugin.h | 42 +++++++---- >>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++- >>> plugins/plugin.h | 12 +++- >>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++-------- >>> plugins/api.c | 39 ++++++++++ >>> plugins/core.c | 109 ++++++++++++++++++++-------- >>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++-- >>> plugins/qemu-plugins.symbols | 2 + >>> 8 files changed, 466 insertions(+), 84 deletions(-) >> >> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing. >> >> Is this intentional? >> > > Do you mean there is no multiline commit message for those changes? > Indeed, for some of those patches, the change is a single line commit message. I just see a commit title and the Signed-off-by. For example, in 8/9 I see the following on git log: commit f518898aa09b42e317b887237bb75a432b477c6d Author: Pierrick Bouvier <pierrick.bouvier@linaro.org> Date: Thu May 2 11:08:46 2024 -0700 plugins: extract cpu_index generate Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> It has only the title: "plugins: extract cpu_index generate" and R-b and S-b, so no description about the changes. Cheers, Gustavo
Hi Pierrick, On 5/2/24 5:09 PM, Gustavo Romero wrote: > On 5/2/24 4:45 PM, Pierrick Bouvier wrote: >> Hi Gustavo, >> >> On 5/2/24 12:16, Gustavo Romero wrote: >>> Hi Pierrick, >>> >>> On 5/2/24 3:08 PM, Pierrick Bouvier wrote: >>>> This series implement two new operations for plugins: >>>> - Store inline allows to write a specific value to a scoreboard. >>>> - Conditional callback executes a callback only when a given condition is true. >>>> The condition is evaluated inline. >>>> >>>> It's possible to mix various inline operations (add, store) with conditional >>>> callbacks, allowing efficient "trap" based counters. >>>> >>>> It builds on top of new scoreboard API, introduced in the previous series. >>>> >>>> NOTE: Two patches still need review >>>> >>>> v2 >>>> -- >>>> >>>> - fixed issue with udata not being passed to conditional callback >>>> - added specific test for this in tests/plugin/inline.c (udata was NULL before). >>>> >>>> v3 >>>> -- >>>> >>>> - rebased on top of "plugins: Rewrite plugin code generation": >>>> 20240316015720.3661236-1-richard.henderson@linaro.org >>>> - single pass code generation >>>> - small cleanups for code generation >>>> >>>> v4 >>>> -- >>>> >>>> - remove op field from qemu_plugin_inline_cb >>>> - use tcg_constant_i64 to load immediate value to store >>>> >>>> v5 >>>> -- >>>> >>>> - rebase on top of master now that Richard's series was merged >>>> >>>> Pierrick Bouvier (9): >>>> plugins: prepare introduction of new inline ops >>>> plugins: extract generate ptr for qemu_plugin_u64 >>>> plugins: add new inline op STORE_U64 >>>> tests/plugin/inline: add test for STORE_U64 inline op >>>> plugins: conditional callbacks >>>> tests/plugin/inline: add test for conditional callback >>>> plugins: distinct types for callbacks >>>> plugins: extract cpu_index generate >>>> plugins: remove op from qemu_plugin_inline_cb >>>> >>>> include/qemu/plugin.h | 42 +++++++---- >>>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++- >>>> plugins/plugin.h | 12 +++- >>>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++-------- >>>> plugins/api.c | 39 ++++++++++ >>>> plugins/core.c | 109 ++++++++++++++++++++-------- >>>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++-- >>>> plugins/qemu-plugins.symbols | 2 + >>>> 8 files changed, 466 insertions(+), 84 deletions(-) >>> >>> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing. >>> >>> Is this intentional? >>> >> >> Do you mean there is no multiline commit message for those changes? >> Indeed, for some of those patches, the change is a single line commit message. > > I just see a commit title and the Signed-off-by. For example, in 8/9 > I see the following on git log: > > commit f518898aa09b42e317b887237bb75a432b477c6d > Author: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Date: Thu May 2 11:08:46 2024 -0700 > > plugins: extract cpu_index generate > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > It has only the title: "plugins: extract cpu_index generate" > and R-b and S-b, so no description about the changes. The description (the body of the commit message) is mentioned here: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message After quickly looking at the committed logs, I do notice some commits missing it, but I really believe it's important to have it for the reasons outlined in these guidelines. Cheers, Gustavo
On 5/2/24 13:48, Gustavo Romero wrote: > Hi Pierrick, > > On 5/2/24 5:09 PM, Gustavo Romero wrote: >> On 5/2/24 4:45 PM, Pierrick Bouvier wrote: >>> Hi Gustavo, >>> >>> On 5/2/24 12:16, Gustavo Romero wrote: >>>> Hi Pierrick, >>>> >>>> On 5/2/24 3:08 PM, Pierrick Bouvier wrote: >>>>> This series implement two new operations for plugins: >>>>> - Store inline allows to write a specific value to a scoreboard. >>>>> - Conditional callback executes a callback only when a given condition is true. >>>>> The condition is evaluated inline. >>>>> >>>>> It's possible to mix various inline operations (add, store) with conditional >>>>> callbacks, allowing efficient "trap" based counters. >>>>> >>>>> It builds on top of new scoreboard API, introduced in the previous series. >>>>> >>>>> NOTE: Two patches still need review >>>>> >>>>> v2 >>>>> -- >>>>> >>>>> - fixed issue with udata not being passed to conditional callback >>>>> - added specific test for this in tests/plugin/inline.c (udata was NULL before). >>>>> >>>>> v3 >>>>> -- >>>>> >>>>> - rebased on top of "plugins: Rewrite plugin code generation": >>>>> 20240316015720.3661236-1-richard.henderson@linaro.org >>>>> - single pass code generation >>>>> - small cleanups for code generation >>>>> >>>>> v4 >>>>> -- >>>>> >>>>> - remove op field from qemu_plugin_inline_cb >>>>> - use tcg_constant_i64 to load immediate value to store >>>>> >>>>> v5 >>>>> -- >>>>> >>>>> - rebase on top of master now that Richard's series was merged >>>>> >>>>> Pierrick Bouvier (9): >>>>> plugins: prepare introduction of new inline ops >>>>> plugins: extract generate ptr for qemu_plugin_u64 >>>>> plugins: add new inline op STORE_U64 >>>>> tests/plugin/inline: add test for STORE_U64 inline op >>>>> plugins: conditional callbacks >>>>> tests/plugin/inline: add test for conditional callback >>>>> plugins: distinct types for callbacks >>>>> plugins: extract cpu_index generate >>>>> plugins: remove op from qemu_plugin_inline_cb >>>>> >>>>> include/qemu/plugin.h | 42 +++++++---- >>>>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++- >>>>> plugins/plugin.h | 12 +++- >>>>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++-------- >>>>> plugins/api.c | 39 ++++++++++ >>>>> plugins/core.c | 109 ++++++++++++++++++++-------- >>>>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++-- >>>>> plugins/qemu-plugins.symbols | 2 + >>>>> 8 files changed, 466 insertions(+), 84 deletions(-) >>>> >>>> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing. >>>> >>>> Is this intentional? >>>> >>> >>> Do you mean there is no multiline commit message for those changes? >>> Indeed, for some of those patches, the change is a single line commit message. >> >> I just see a commit title and the Signed-off-by. For example, in 8/9 >> I see the following on git log: >> >> commit f518898aa09b42e317b887237bb75a432b477c6d >> Author: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> Date: Thu May 2 11:08:46 2024 -0700 >> >> plugins: extract cpu_index generate >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> >> It has only the title: "plugins: extract cpu_index generate" >> and R-b and S-b, so no description about the changes. > > The description (the body of the commit message) is mentioned here: > > https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message > > After quickly looking at the committed logs, I do notice some commits missing it, > but I really believe it's important to have it for the reasons outlined in these > guidelines. > You're right, I just sent a v6 with more elaborated commit messages. Thanks, Pierrick > > Cheers, > Gustavo