> 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?
> + }
> +
> /* Make sure the console is always "hardwired". */
> - delay(10000); /* wait for output to finish
> */
> if (is_console) {
> SET(sc->sc_hwflags, COM_HW_CONSOLE);
> }
> diff --git a/sys/sys/cpu.h b/sys/sys/cpu.h
> index 0393f9c0058a..3f8a0d1ea684 100644
> --- a/sys/sys/cpu.h
> +++ b/sys/sys/cpu.h
> @@ -50,6 +50,14 @@ void cpu_idle(void);
> #endif
> #endif
>
> +#ifndef __HAVE_VIRTUAL_HOST_P
> +static __inline bool __unused
> +inside_vm_guest(void)
> +{
> + return 0;
<pedant>Please return false here if you’re going to return a bool
type.</pedant> :-)
> +}
> +#endif /* !__HAVE_VIRTUAL_HOST_P */
In general, I don’t object to having something like inside_vm_guest(), but I
think we need to maybe put some definition around what returning “true” allows.
-- thorpej
