usr.sbin/bhyve: add rendezvous layer for initialization step of bhyve

[aokblast]

Summary

THere maybe multiple wait operation in different components of bhyve (e.g. rfb, sock)

In original implementation, each wait operation will block the main thread thus
we can not connect to the socket in arbitrary order. Moreover, the actual
order depends on the argument order pass to the bhyve program

To better address this, we should add a rendezvous layer that can wait
all component at the same time

[aokblast]

Free members

[markj]

Are there any other potential consumers of this interface, besides rfb? I see the comment that mentions why gdb can't participate, which is a bit unsatisfying but okay.

usr.sbin/bhyve/rendezvous.c

75

This interface behaves basically like a semaphore: rendezvous_add_member() increases the count, rendezvous_member_notify() decreases it, and rendezvous() waits for it to decrease to zero.

I think it could be implemented more simply: just have a single mutex, CV, and a counter. I don't see any reason to have per-consumer state either.

93

We should check somehow that this operation is always done before rendezvous() is called.

usr.sbin/bhyve/rfb.c

1327

Is there any need for these functions to be separate? It looks like we might as well automatically add the new member when it's created.

1333–1334

This conditional can simply be deleted.

[aokblast]

Update implementation by using only one mutex and cond variable

[aokblast]

Another consumer of rendezvous will be the raw socket over serial console that I have implemented a couple months ago. I haven't implement the wait operation on raw socket serial console. I would like to confirm the interface of rendezvous before I implement it.

BTW, for gdb, we can still wait gdb on rendezvous layer by deleting the set of vcpu0 and use rendezvous_notify. But I think setting vcpu0 maybe a better implementation when consider the behavior of gdb?

usr.sbin/bhyve/rendezvous.c

75

Ya, I think you are right. Fix it

93

Do you means the original implementation by separating add_member and member_create. If it is, I think the new implementation that combine these two have solve the problem

[aokblast]

Fix condition when there is no rendezvous member

[markj]

In D48241#1100375, @aokblast wrote:

Another consumer of rendezvous will be the raw socket over serial console that I have implemented a couple months ago. I haven't implement the wait operation on raw socket serial console. I would like to confirm the interface of rendezvous before I implement it.

I left some inline comments. It's hard to make judgements about the interface when there is only one consumer. It looks ok to me, but maybe the uart driver has some extra requirements.

BTW, for gdb, we can still wait gdb on rendezvous layer by deleting the set of vcpu0 and use rendezvous_notify. But I think setting vcpu0 maybe a better implementation when consider the behavior of gdb?

Actually, I do not understand why gdb cannot use the rendezvous layer. bhyve_start_vcpu() is called after rendezvous(), so the following would happen:

1. init_gdb() registers a notifier,
2. rendezvous() blocks until notification is received
3. a debugger attaches and new_connection() notifies the bhyverun thread
4. bhyverun adds vCPUs
5. vCPU threads block in gdb_cpu_add() until the debugger resumes the guest

What goes wrong here?

usr.sbin/bhyve/rendezvous.c

40

IMHO this facility should just be a couple of functions in bhyverun.c - it is specific to bhyve startup (e.g., it can only be used once) and not very abstract.

I think you can simply have a global struct in bhvyerun.c with public functions like

void bhyve_startup_block(); /* called to register a notifier and block startup */
void bhyve_startup_notify(); /* notify the blocked thread */

and have bhyve_startup_wait() in implemented privately in bhyverun.c.

The rendezvous implementation is good but the interface is not general enough to warrant its own file, IMHO.

96

We should assert that the counter is > 0.

115

pthread_mutex_lock(...);
while (softc->counter > 0)
    pthread_cond_wait(...);
pthread_mutex_unlock(...);

would be a bit simpler and avoids extra (un)locking when executing the loop multiple times.