[PATCH] bsd-user/main: Allow setting tb-size

[PATCH] bsd-user/main: Allow setting tb-size

[Ilya Leoshkevich]

While qemu-system can set tb-size using -accel tcg,tb-size=n, there
is no similar knob for qemu-bsd-user. Add one in a way similar to how
one-insn-per-tb is already handled.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 bsd-user/main.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index cc980e6f401..7c230b0c7a5 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -60,6 +60,7 @@ uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
 
 static bool opt_one_insn_per_tb;
+static unsigned long opt_tb_size;
 uintptr_t guest_base;
 bool have_guest_base;
 /*
@@ -169,6 +170,7 @@ static void usage(void)
            "                  (use '-d help' for a list of log items)\n"
            "-D logfile        write logs to 'logfile' (default stderr)\n"
            "-one-insn-per-tb  run with one guest instruction per emulated TB\n"
+           "-tb-size size     TCG translation block cache size\n"
            "-strace           log system calls\n"
            "-trace            [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
            "                  specify tracing options\n"
@@ -387,6 +389,11 @@ int main(int argc, char **argv)
             seed_optarg = optarg;
         } else if (!strcmp(r, "one-insn-per-tb")) {
             opt_one_insn_per_tb = true;
+        } else if (!strcmp(r, "tb-size")) {
+            r = argv[optind++];
+            if (qemu_strtoul(r, NULL, 0, &opt_tb_size)) {
+                usage();
+            }
         } else if (!strcmp(r, "strace")) {
             do_strace = 1;
         } else if (!strcmp(r, "trace")) {
@@ -452,6 +459,8 @@ int main(int argc, char **argv)
         accel_init_interfaces(ac);
         object_property_set_bool(OBJECT(accel), "one-insn-per-tb",
                                  opt_one_insn_per_tb, &error_abort);
+        object_property_set_int(OBJECT(accel), "tb-size",
+                                opt_tb_size, &error_abort);
         ac->init_machine(NULL);
     }
 
-- 
2.45.2

Re: [PATCH] bsd-user/main: Allow setting tb-size

[Philippe Mathieu-Daudé]

On 31/7/24 16:45, Ilya Leoshkevich wrote:
> While qemu-system can set tb-size using -accel tcg,tb-size=n, there
> is no similar knob for qemu-bsd-user. Add one in a way similar to how
> one-insn-per-tb is already handled.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   bsd-user/main.c | 9 +++++++++
>   1 file changed, 9 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] bsd-user/main: Allow setting tb-size

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]

On Wed, Jul 31, 2024 at 8:45 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:

> While qemu-system can set tb-size using -accel tcg,tb-size=n, there
> is no similar knob for qemu-bsd-user. Add one in a way similar to how
> one-insn-per-tb is already handled.
>

Cool! Are you using bsd-user and need this for some reason? Or is this
purely theoretical? Is there a larger context I can read about somewhere?

I'll merge it either way (so none of the above is a criticism, I'm genuinely
curious) , but I don't get too many bsd-user fixes and this one is unusual.


> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  bsd-user/main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index cc980e6f401..7c230b0c7a5 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -60,6 +60,7 @@ uintptr_t qemu_host_page_size;
>  intptr_t qemu_host_page_mask;
>
>  static bool opt_one_insn_per_tb;
> +static unsigned long opt_tb_size;
>  uintptr_t guest_base;
>  bool have_guest_base;
>  /*
> @@ -169,6 +170,7 @@ static void usage(void)
>             "                  (use '-d help' for a list of log items)\n"
>             "-D logfile        write logs to 'logfile' (default stderr)\n"
>             "-one-insn-per-tb  run with one guest instruction per emulated
> TB\n"
> +           "-tb-size size     TCG translation block cache size\n"
>             "-strace           log system calls\n"
>             "-trace
> [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
>             "                  specify tracing options\n"
> @@ -387,6 +389,11 @@ int main(int argc, char **argv)
>              seed_optarg = optarg;
>          } else if (!strcmp(r, "one-insn-per-tb")) {
>              opt_one_insn_per_tb = true;
> +        } else if (!strcmp(r, "tb-size")) {
> +            r = argv[optind++];
> +            if (qemu_strtoul(r, NULL, 0, &opt_tb_size)) {
> +                usage();
> +            }
>          } else if (!strcmp(r, "strace")) {
>              do_strace = 1;
>          } else if (!strcmp(r, "trace")) {
> @@ -452,6 +459,8 @@ int main(int argc, char **argv)
>          accel_init_interfaces(ac);
>          object_property_set_bool(OBJECT(accel), "one-insn-per-tb",
>                                   opt_one_insn_per_tb, &error_abort);
> +        object_property_set_int(OBJECT(accel), "tb-size",
> +                                opt_tb_size, &error_abort);
>          ac->init_machine(NULL);
>      }
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

I'll queue this to my bsd-user-2024-q3-2 branch. I hope to land it, just
after 9.1.0 release.

Warner

[-- Attachment #2: Type: text/html, Size: 3947 bytes --]

Re: [PATCH] bsd-user/main: Allow setting tb-size

[Philippe Mathieu-Daudé]

On 31/7/24 23:21, Warner Losh wrote:
> On Wed, Jul 31, 2024 at 8:45 AM Ilya Leoshkevich <iii@linux.ibm.com 
> <mailto:iii@linux.ibm.com>> wrote:
> 
>     While qemu-system can set tb-size using -accel tcg,tb-size=n, there
>     is no similar knob for qemu-bsd-user. Add one in a way similar to how
>     one-insn-per-tb is already handled.
> 
> 
> Cool! Are you using bsd-user and need this for some reason? Or is this
> purely theoretical? Is there a larger context I can read about somewhere?

Trying to keep user interface parity between linux/bsd.

Ideally this duplication should be unified in common-user/.

> I'll merge it either way (so none of the above is a criticism, I'm genuinely
> curious) , but I don't get too many bsd-user fixes and this one is unusual.
> 
>     Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org
>     <mailto:philmd@linaro.org>>
>     Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com
>     <mailto:iii@linux.ibm.com>>
>     ---
>       bsd-user/main.c | 9 +++++++++
>       1 file changed, 9 insertions(+)

Re: [PATCH] bsd-user/main: Allow setting tb-size

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]

On Wed, Jul 31, 2024 at 3:42 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 31/7/24 23:21, Warner Losh wrote:
> > On Wed, Jul 31, 2024 at 8:45 AM Ilya Leoshkevich <iii@linux.ibm.com
> > <mailto:iii@linux.ibm.com>> wrote:
> >
> >     While qemu-system can set tb-size using -accel tcg,tb-size=n, there
> >     is no similar knob for qemu-bsd-user. Add one in a way similar to how
> >     one-insn-per-tb is already handled.
> >
> >
> > Cool! Are you using bsd-user and need this for some reason? Or is this
> > purely theoretical? Is there a larger context I can read about somewhere?
>
> Trying to keep user interface parity between linux/bsd.
>
> Ideally this duplication should be unified in common-user/.
>

I'd love that.

Anyway, both of these patches queued to my branch


> > I'll merge it either way (so none of the above is a criticism, I'm
> genuinely
> > curious) , but I don't get too many bsd-user fixes and this one is
> unusual.
> >
> >     Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org
> >     <mailto:philmd@linaro.org>>
> >     Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com
> >     <mailto:iii@linux.ibm.com>>
> >     ---
> >       bsd-user/main.c | 9 +++++++++
> >       1 file changed, 9 insertions(+)
>
>

[-- Attachment #2: Type: text/html, Size: 2308 bytes --]

Re: [PATCH] bsd-user/main: Allow setting tb-size

[Ilya Leoshkevich]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

On Wed, 2024-07-31 at 15:21 -0600, Warner Losh wrote:
> On Wed, Jul 31, 2024 at 8: 45 AM Ilya Leoshkevich
> <iii@ linux. ibm. com> wrote: While qemu-system can set tb-size using
> -accel tcg,tb-size=n, there is no similar knob for qemu-bsd-user. Add
> one in a way similar to how one-insn-per-tb is already
> 
> 
> 
> On Wed, Jul 31, 2024 at 8:45 AM Ilya Leoshkevich <iii@linux.ibm.com>
> wrote:
> > While qemu-system can set tb-size using -accel tcg,tb-size=n, there
> > is no similar knob for qemu-bsd-user. Add one in a way similar to
> > how
> > one-insn-per-tb is already handled.
> > 
> 
> 
> Cool! Are you using bsd-user and need this for some reason? Or is
> this
> purely theoretical? Is there a larger context I can read about
> somewhere?

I needed this on Linux in order to debug an issue where I suspected
full TB invalidation may be an issue.
It turned out to be something completely different, but I found it
useful: setting it to, e.g., 4096 makes full TB invalidation very rare,
so if a problem is still reproducible, then the root causes is
something else.
Philippe suggested to implement this for BSD as well in order to keep
the interfaces in sync.

[...]


[-- Attachment #2: Type: text/html, Size: 2957 bytes --]

Re: [PATCH] bsd-user/main: Allow setting tb-size

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

On Thu, Aug 1, 2024 at 2:25 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:

> On Wed, 2024-07-31 at 15:21 -0600, Warner Losh wrote:
>
> On Wed, Jul 31, 2024 at 8: 45 AM Ilya Leoshkevich <iii@ linux. ibm. com>
> wrote: While qemu-system can set tb-size using -accel tcg,tb-size=n, there
> is no similar knob for qemu-bsd-user. Add one in a way similar to how
> one-insn-per-tb is already
> On Wed, Jul 31, 2024 at 8:45 AM Ilya Leoshkevich <iii@linux.ibm.com>
> wrote:
>
> While qemu-system can set tb-size using -accel tcg,tb-size=n, there
> is no similar knob for qemu-bsd-user. Add one in a way similar to how
> one-insn-per-tb is already handled.
>
>
> Cool! Are you using bsd-user and need this for some reason? Or is this
> purely theoretical? Is there a larger context I can read about somewhere?
>
>
> I needed this on Linux in order to debug an issue where I suspected full
> TB invalidation may be an issue.
> It turned out to be something completely different, but I found it useful:
> setting it to, e.g., 4096 makes full TB invalidation very rare, so if a
> problem is still reproducible, then the root causes is something else.
> Philippe suggested to implement this for BSD as well in order to keep the
> interfaces in sync.
>

Excellent! Thank you for taking the time to do this! And the other bug fix.
Both have been queued to my first post 9.1 pull branch.

Warner

[-- Attachment #2: Type: text/html, Size: 2546 bytes --]