On Wed, Feb 05, 2025 at 09:34:21AM -0800, Jason Thorpe wrote:
>
> > On Feb 4, 2025, at 11:40 PM, Simon Burge <sim...@netbsd.org> wrote:
> >
> > Emile `iMil' Heitor wrote:
> >
> >> com_attach_subr() in sys/dev/ic/com.c has a delay(10000) waiting for
> >> output to finish, this is very unlikely to be useful inside a virtual
> >> machine and slows down boot time in such machine.
> >> I propose the following approach:
> >
> > Expanding on Martin's suggestion, I wonder if something like this
> > is better (completely untested!)? This way we'd always have
> > inside_vm_guest() available to remove the #ifdef check from any
> > consumers and reduce clutter a little bit.
>
> I have slightly broader concerns. :-)
>
> > diff --git a/sys/arch/x86/include/cpu.h b/sys/arch/x86/include/cpu.h
> > index 6c26dc624b1..94c99946bc9 100644
> > --- a/sys/arch/x86/include/cpu.h
> > +++ b/sys/arch/x86/include/cpu.h
> > @@ -558,6 +558,12 @@ vm_guest_is_pvh(void)
> > }
> > }
> >
> > +#define __HAVE_VIRTUAL_HOST_P
>
> First, kind of hand-waving the com(4)-specific parts here, but, the style we
> generally have lacks the _P for the __HAVE_* macros. For example:
>
> - __HAVE_ATOMIC_OPERATIONS
> - __HAVE_SYSCALL_INTERN
> - __HAVE_FAST_SOFTINTS
>
> etc.
>
> > +static __inline bool __unused
> > +inside_vm_guest(void)
> > +{
> > + return vm_guest != VM_GUEST_NO;
> > +}
> > +
> > /* cpu_topology.c */
> > void x86_cpu_topology(struct cpu_info *);
> > diff --git a/sys/dev/ic/com.c b/sys/dev/ic/com.c
> > index 539b6597accd..f835e493184c 100644
> > --- a/sys/dev/ic/com.c
> > +++ b/sys/dev/ic/com.c
> > @@ -589,8 +589,15 @@ com_attach_subr(struct com_softc *sc)
> > break;
> > }
> >
> > + if (!inside_vm_guest()) {
> > + /*
> > + * Wait for output to finish. No need for
> > + * a delay on virtual machines.
> > + */
> > + delay(10000);
>
> As for the com(4)-specific bit: why are we inserting a blanket 10ms delay *at
> all*? If we’re concerned about garbling output-in-progress on real hardware,
> isn’t there some register we can look at, or something?
I had the same question. Now I have looked at the code. I still have
the same question. :-)
I guess that it has something to do with letting the Tx FIFO empty before
(potentially) reconfiguring it?
It looks like some of the FIFOs are 64 bytes long. Supposing a modest
bitrate of 9600 bps, it seems like it will take about 50 milliseconds to
empty a full FIFO. At 300 bps, more than 1.5 seconds.
David
--
David Young
dyo...@pobox.com Urbana, IL (217) 721-9981
