Re: Question about tb_flush()
On Tue, May 26, 2026 at 9:23 AM Peter Maydell wrote: > On Tue, 26 May 2026 at 15:52, Peter Maydell > wrote: > > Some stuff has snuck back into qemu.h again (e.g. the > > prototype of init_main_thread() could live in user-internals.h > > but was put in the wrong place) > > I just sent a patchset moving a couple of prototypes out of qemu.h. > > > because this kind of "looks like > > a good place to dump random stuff" header is hard to keep clean. > > There's probably a better name than "qemu.h" for it. > > While I was doing it I thought about this part. I think in an > ideal world we would split qemu.h into different parts, e.g. > one header for the TaskState struct definition, one for the > user-access functions and macros, and so on (most of the QEMU > source files including qemu.h don't need everything it is > providing, only one of the three or four groups of APIs). > But doing this requires coordination across linux-user and bsd-user > so they both split the functions/types out of their qemu.h headers > together. That doesn't really seem worth doing to me, at least > not now while you still have a large backlog of bsd-user work > not upstream yet. > Yes. This is absolutely something that I want to do, which is why I asked the question. I'd like to get through the backlog first (or the bulk of the backlog) and then start to move to a common design pattern between linux-user and bsd-user and maybe find ways to refactor. One issue that I know of right now is we compile common-user once for all the builds, so it doesn't depend on TARGET defines, while what we need to do is also have something that compiles per TARGET for some of the sharing that I see as possible and likely desireable (see the recent thunk.c thread). While not worth doing TODAY, I see the light at the end of the tunnel for upstreaming and want to start thinking about it and start studying things so I can start a discussion in the coming months in an intelligent way. Warner
Re: Question about tb_flush()
On Tue, May 26, 2026 at 8:44 AM Philippe Mathieu-Daudé wrote: > On 26/5/26 16:39, Warner Losh wrote: > > On Tue, May 26, 2026 at 1:53 AM Philippe Mathieu-Daudé < > [email protected]> > > wrote: > > > >> On 24/5/26 18:57, Warner Losh wrote: > >>> On Sun, Feb 1, 2026 at 7:54 AM Alex Bennée > >> wrote: > >>> > Warner Losh writes: > > > OK. I've updated bsd-user fork from last year to yesterday. I had one > question. > > > > I see that we've gone from tb_flush() to queue_tb_flush(). Why was > >> that? > And is it a drop in? > > FreeBSD's fork has this in the thread creation: > > > > /* > >* If this is our first additional thread, we need to ensure we > >* generate code for parallel execution and flush old > >> translations. > >* Do this now so that the copy gets CF_PARALLEL too. > >*/ > > if (!(cpu->tcg_cflags & CF_PARALLEL)) { > > cpu->tcg_cflags |= CF_PARALLEL; > > tb_flush(cpu); > > } > > > > I think the right thing to do is to just change this to > queue_tb_flush(cpu), and that compiles. I'd like to get some > >> confirmation, > though. > > Did I parse the changes right? > > Yes, because ultimately we use safe work which: > > * Schedules the function @func for execution on the vCPU @cpu > asynchronously, > * while all other vCPUs are sleeping. > > >>> > >>> So I've done this in the latest blitz branch. I encountered one > gotcha. I > >>> replaced it in the one place, but that didn't work too well. I finally > >>> tracked down the issue: I had implemented the change in only one place > >>> instead of the three required locations. I also had to bring the > >>> begin_parallel_context() function over from linux-user. I like > >>> user-internal.h that's in linux user. What is your touchstone for > >> deciding > >>> where that code belongs in it? I'd like to do the same in bsd-user once > >> my > >>> upstream focus shift to refinement of blitz and maybe my next cool > thing > >>> there. > >> > >> All common code should go under common-user/ and include/user/. > >> > > > > Great! I look forward to growing this directory.. But I was trying to > ask a > > slightly > > different question: What belongs in user-internal.h? What's the current > > rubric > > for linux-user in that regard? > > We try to have cross-subsystem APIs exposed in include/. When we need to > share some structure / method only within a single subsystem, we do not > expose the prototype via include/ and add a local header. > > So linux-user/linux-user-internal.h should only contains things to share > within linux-user/, and not related to bsd-user. > It's slightly related to bsd-user since my intention was to create a bsd-user/user-internal.h since I liked the design. So I was curious what linux-user's process for deciding where to put different things since there's several include files now. I understand the qemu.h being too wide a net and wanting to limit what's in there (bsd-user has a decade of refactoring to catch up with there). I'm trying to understand the choices linux-user has made to use them as a baseline for future bsd-user refactoring. The things that are in there now don't seem to have an obvious unifying thread to them. Is this documented somewhere, or just in the different commits for the files? Warner
Re: Question about tb_flush()
On Tue, 26 May 2026 at 15:52, Peter Maydell wrote: > Some stuff has snuck back into qemu.h again (e.g. the > prototype of init_main_thread() could live in user-internals.h > but was put in the wrong place) I just sent a patchset moving a couple of prototypes out of qemu.h. > because this kind of "looks like > a good place to dump random stuff" header is hard to keep clean. > There's probably a better name than "qemu.h" for it. While I was doing it I thought about this part. I think in an ideal world we would split qemu.h into different parts, e.g. one header for the TaskState struct definition, one for the user-access functions and macros, and so on (most of the QEMU source files including qemu.h don't need everything it is providing, only one of the three or four groups of APIs). But doing this requires coordination across linux-user and bsd-user so they both split the functions/types out of their qemu.h headers together. That doesn't really seem worth doing to me, at least not now while you still have a large backlog of bsd-user work not upstream yet. thanks -- PMM
Re: Question about tb_flush()
On Tue, May 26, 2026 at 1:53 AM Philippe Mathieu-Daudé
wrote:
> On 24/5/26 18:57, Warner Losh wrote:
> > On Sun, Feb 1, 2026 at 7:54 AM Alex Bennée
> wrote:
> >
> >> Warner Losh writes:
> >>
> >>> OK. I've updated bsd-user fork from last year to yesterday. I had one
> >> question.
> >>>
> >>> I see that we've gone from tb_flush() to queue_tb_flush(). Why was
> that?
> >> And is it a drop in?
> >>> FreeBSD's fork has this in the thread creation:
> >>>
> >>> /*
> >>> * If this is our first additional thread, we need to ensure we
> >>> * generate code for parallel execution and flush old
> translations.
> >>> * Do this now so that the copy gets CF_PARALLEL too.
> >>> */
> >>> if (!(cpu->tcg_cflags & CF_PARALLEL)) {
> >>> cpu->tcg_cflags |= CF_PARALLEL;
> >>> tb_flush(cpu);
> >>> }
> >>>
> >>> I think the right thing to do is to just change this to
> >> queue_tb_flush(cpu), and that compiles. I'd like to get some
> confirmation,
> >> though.
> >>> Did I parse the changes right?
> >>
> >> Yes, because ultimately we use safe work which:
> >>
> >> * Schedules the function @func for execution on the vCPU @cpu
> >> asynchronously,
> >> * while all other vCPUs are sleeping.
> >>
> >
> > So I've done this in the latest blitz branch. I encountered one gotcha. I
> > replaced it in the one place, but that didn't work too well. I finally
> > tracked down the issue: I had implemented the change in only one place
> > instead of the three required locations. I also had to bring the
> > begin_parallel_context() function over from linux-user. I like
> > user-internal.h that's in linux user. What is your touchstone for
> deciding
> > where that code belongs in it? I'd like to do the same in bsd-user once
> my
> > upstream focus shift to refinement of blitz and maybe my next cool thing
> > there.
>
> All common code should go under common-user/ and include/user/.
>
Great! I look forward to growing this directory.. But I was trying to ask a
slightly
different question: What belongs in user-internal.h? What's the current
rubric
for linux-user in that regard?
Warner
Re: Question about tb_flush()
On Tue, 26 May 2026 at 15:40, Warner Losh wrote: > > Great! I look forward to growing this directory.. But I was trying to ask a > slightly > different question: What belongs in user-internal.h? What's the current rubric > for linux-user in that regard? For linux-user, the user-internals.h header is there because some places outside linux-user/ include qemu.h for various reasons. So (commit 3b249d2661c) I pulled as much as I could that really was only used inside linux-user/ out of linux-user/qemu.h. So it's not so much "what belongs in user-internal.h?" and more "what absolutely has to go in qemu.h and can't be moved elsewhere, because the code outside linux-user needs those definitions?". Some stuff has snuck back into qemu.h again (e.g. the prototype of init_main_thread() could live in user-internals.h but was put in the wrong place) because this kind of "looks like a good place to dump random stuff" header is hard to keep clean. There's probably a better name than "qemu.h" for it. thanks -- PMM
Re: Question about tb_flush()
On 26/5/26 16:39, Warner Losh wrote:
On Tue, May 26, 2026 at 1:53 AM Philippe Mathieu-Daudé
wrote:
On 24/5/26 18:57, Warner Losh wrote:
On Sun, Feb 1, 2026 at 7:54 AM Alex Bennée
wrote:
Warner Losh writes:
OK. I've updated bsd-user fork from last year to yesterday. I had one
question.
I see that we've gone from tb_flush() to queue_tb_flush(). Why was
that?
And is it a drop in?
FreeBSD's fork has this in the thread creation:
/*
* If this is our first additional thread, we need to ensure we
* generate code for parallel execution and flush old
translations.
* Do this now so that the copy gets CF_PARALLEL too.
*/
if (!(cpu->tcg_cflags & CF_PARALLEL)) {
cpu->tcg_cflags |= CF_PARALLEL;
tb_flush(cpu);
}
I think the right thing to do is to just change this to
queue_tb_flush(cpu), and that compiles. I'd like to get some
confirmation,
though.
Did I parse the changes right?
Yes, because ultimately we use safe work which:
* Schedules the function @func for execution on the vCPU @cpu
asynchronously,
* while all other vCPUs are sleeping.
So I've done this in the latest blitz branch. I encountered one gotcha. I
replaced it in the one place, but that didn't work too well. I finally
tracked down the issue: I had implemented the change in only one place
instead of the three required locations. I also had to bring the
begin_parallel_context() function over from linux-user. I like
user-internal.h that's in linux user. What is your touchstone for
deciding
where that code belongs in it? I'd like to do the same in bsd-user once
my
upstream focus shift to refinement of blitz and maybe my next cool thing
there.
All common code should go under common-user/ and include/user/.
Great! I look forward to growing this directory.. But I was trying to ask a
slightly
different question: What belongs in user-internal.h? What's the current
rubric
for linux-user in that regard?
We try to have cross-subsystem APIs exposed in include/. When we need to
share some structure / method only within a single subsystem, we do not
expose the prototype via include/ and add a local header.
So linux-user/linux-user-internal.h should only contains things to share
within linux-user/, and not related to bsd-user.
Re: Question about tb_flush()
On 24/5/26 18:57, Warner Losh wrote:
On Sun, Feb 1, 2026 at 7:54 AM Alex Bennée wrote:
Warner Losh writes:
OK. I've updated bsd-user fork from last year to yesterday. I had one
question.
I see that we've gone from tb_flush() to queue_tb_flush(). Why was that?
And is it a drop in?
FreeBSD's fork has this in the thread creation:
/*
* If this is our first additional thread, we need to ensure we
* generate code for parallel execution and flush old translations.
* Do this now so that the copy gets CF_PARALLEL too.
*/
if (!(cpu->tcg_cflags & CF_PARALLEL)) {
cpu->tcg_cflags |= CF_PARALLEL;
tb_flush(cpu);
}
I think the right thing to do is to just change this to
queue_tb_flush(cpu), and that compiles. I'd like to get some confirmation,
though.
Did I parse the changes right?
Yes, because ultimately we use safe work which:
* Schedules the function @func for execution on the vCPU @cpu
asynchronously,
* while all other vCPUs are sleeping.
So I've done this in the latest blitz branch. I encountered one gotcha. I
replaced it in the one place, but that didn't work too well. I finally
tracked down the issue: I had implemented the change in only one place
instead of the three required locations. I also had to bring the
begin_parallel_context() function over from linux-user. I like
user-internal.h that's in linux user. What is your touchstone for deciding
where that code belongs in it? I'd like to do the same in bsd-user once my
upstream focus shift to refinement of blitz and maybe my next cool thing
there.
All common code should go under common-user/ and include/user/.
Re: Question about tb_flush()
On Sun, Feb 1, 2026 at 7:54 AM Alex Bennée wrote:
> Warner Losh writes:
>
> > OK. I've updated bsd-user fork from last year to yesterday. I had one
> question.
> >
> > I see that we've gone from tb_flush() to queue_tb_flush(). Why was that?
> And is it a drop in?
> > FreeBSD's fork has this in the thread creation:
> >
> > /*
> > * If this is our first additional thread, we need to ensure we
> > * generate code for parallel execution and flush old translations.
> > * Do this now so that the copy gets CF_PARALLEL too.
> > */
> > if (!(cpu->tcg_cflags & CF_PARALLEL)) {
> > cpu->tcg_cflags |= CF_PARALLEL;
> > tb_flush(cpu);
> > }
> >
> > I think the right thing to do is to just change this to
> queue_tb_flush(cpu), and that compiles. I'd like to get some confirmation,
> though.
> > Did I parse the changes right?
>
> Yes, because ultimately we use safe work which:
>
> * Schedules the function @func for execution on the vCPU @cpu
> asynchronously,
> * while all other vCPUs are sleeping.
>
So I've done this in the latest blitz branch. I encountered one gotcha. I
replaced it in the one place, but that didn't work too well. I finally
tracked down the issue: I had implemented the change in only one place
instead of the three required locations. I also had to bring the
begin_parallel_context() function over from linux-user. I like
user-internal.h that's in linux user. What is your touchstone for deciding
where that code belongs in it? I'd like to do the same in bsd-user once my
upstream focus shift to refinement of blitz and maybe my next cool thing
there.
Warner
Re: Question about tb_flush()
Warner Losh writes:
> OK. I've updated bsd-user fork from last year to yesterday. I had one
> question.
>
> I see that we've gone from tb_flush() to queue_tb_flush(). Why was that? And
> is it a drop in?
> FreeBSD's fork has this in the thread creation:
>
> /*
> * If this is our first additional thread, we need to ensure we
> * generate code for parallel execution and flush old translations.
> * Do this now so that the copy gets CF_PARALLEL too.
> */
> if (!(cpu->tcg_cflags & CF_PARALLEL)) {
> cpu->tcg_cflags |= CF_PARALLEL;
> tb_flush(cpu);
> }
>
> I think the right thing to do is to just change this to queue_tb_flush(cpu),
> and that compiles. I'd like to get some confirmation, though.
> Did I parse the changes right?
Yes, because ultimately we use safe work which:
* Schedules the function @func for execution on the vCPU @cpu asynchronously,
* while all other vCPUs are sleeping.
>
> Everything else is mundane changes that were more annoying than difficult :)
>
> Warner
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: Question about tb_flush()
Hi Warner,
On 31/1/26 01:08, Warner Losh wrote:
OK. I've updated bsd-user fork from last year to yesterday. I had one
question.
I see that we've gone from tb_flush() to queue_tb_flush(). Why was that?
This fixed a race where a vCPU kept a ref to a flushed TB.
And is it a drop in?
FreeBSD's fork has this in the thread creation:
/*
* If this is our first additional thread, we need to ensure we
* generate code for parallel execution and flush old translations.
* Do this now so that the copy gets CF_PARALLEL too.
*/
if (!(cpu->tcg_cflags & CF_PARALLEL)) {
cpu->tcg_cflags |= CF_PARALLEL;
tb_flush(cpu);
}
I think the right thing to do is to just change this to
queue_tb_flush(cpu), and that compiles. I'd like to get some
confirmation, though. Did I parse the changes right?
You should change these 4 lines by:
begin_parallel_context(cpu);
which calls tb_flush__exclusive_or_serial(), itself described as:
* Used to flush all the translation blocks in the system. Mostly this
* is used to empty the code generation buffer after it is full.
* Sometimes it is used when it is simpler to flush everything than work
* out which individual translations are now invalid.
*
* Must be called from an exclusive or serial context, e.g.
* start_exclusive, vm_stop, or when there is only one vcpu. Note that
* start_exclusive cannot be called from within the cpu run loop, so
* this cannot be called from within target code.
See equivalent commit 430014bee7a ("linux-user: Split out
begin_parallel_context").
Everything else is mundane changes that were more annoying than difficult :)
This is reassuring :)
Warner
