On Sun, May 26, 2019 at 11:38:37PM -0700, Mike Larkin wrote: > On Wed, May 22, 2019 at 08:05:50PM -0500, Katherine Rohl wrote: > > Hi, > > > > Adjusted NS8250 behavior in vmd(8) so it gets detected as an 8250 and not a > > 16450 by OpenBSD’s boot process. Also generalized some of the COM1-specific > > I/O address definitions to support adding COM2 (and COM3, and COM4…) in the > > future. > > > > Tested by logging into my VM with the virtual serial console and > > reinstalling 6.5, everything was fine. The boot process detected an NS8250. > > > > It occurred to me that by always returning 0xFF, a guest might write 0xFF to > scratch, then read it back, and we'd always return 0xFF. They might then > think that the scratch register worked. That's pretty bizarre, but I've seen > a lot of bizarre behaviour in various guests. >
Theo pointed out that this is probably the right way (always return 0xFF), so I'll commit the original version. Thanks. -ml > The slightly revised version below just inverts whatever was written, ensuring > that whatever is read is not what was written. This should make anyone doing > such calculation think the scratch doesn't work. > > Tested on Linux and OpenBSD guests. > > Thoughts? > > -ml > > Index: ns8250.c > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/ns8250.c,v > retrieving revision 1.20 > diff -u -p -a -u -r1.20 ns8250.c > --- ns8250.c 11 Mar 2019 17:08:52 -0000 1.20 > +++ ns8250.c 27 May 2019 06:34:38 -0000 > @@ -74,6 +74,7 @@ ns8250_init(int fd, uint32_t vmid) > } > com1_dev.fd = fd; > com1_dev.irq = 4; > + com1_dev.portid = NS8250_COM1; > com1_dev.rcv_pending = 0; > com1_dev.vmid = vmid; > com1_dev.byte_out = 0; > @@ -509,7 +510,8 @@ vcpu_process_com_scr(struct vm_exit *vei > /* > * vei_dir == VEI_DIR_OUT : out instruction > * > - * Write to SCR > + * The 8250 does not have a scratch register. Make sure that whatever > + * was written is not what gets read back. > */ > if (vei->vei.vei_dir == VEI_DIR_OUT) { > com1_dev.regs.scr = vei->vei.vei_data; > @@ -517,9 +519,11 @@ vcpu_process_com_scr(struct vm_exit *vei > /* > * vei_dir == VEI_DIR_IN : in instruction > * > - * Read from SCR > + * Read from SCR - invert whatever was previously written, > + * to ensure the guest doesn't think the scratch register > + * works. > */ > - set_return_data(vei, com1_dev.regs.scr); > + set_return_data(vei, ~com1_dev.regs.scr); > } > } > > @@ -647,6 +651,7 @@ ns8250_restore(int fd, int con_fd, uint3 > } > com1_dev.fd = con_fd; > com1_dev.irq = 4; > + com1_dev.portid = NS8250_COM1; > com1_dev.rcv_pending = 0; > com1_dev.vmid = vmid; > com1_dev.byte_out = 0; > Index: ns8250.h > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/ns8250.h,v > retrieving revision 1.7 > diff -u -p -a -u -r1.7 ns8250.h > --- ns8250.h 11 Mar 2019 17:08:52 -0000 1.7 > +++ ns8250.h 27 May 2019 06:31:13 -0000 > @@ -18,14 +18,30 @@ > /* > * Emulated 8250 UART > */ > -#define COM1_DATA 0x3f8 > -#define COM1_IER 0x3f9 > -#define COM1_IIR 0x3fa > -#define COM1_LCR 0x3fb > -#define COM1_MCR 0x3fc > -#define COM1_LSR 0x3fd > -#define COM1_MSR 0x3fe > -#define COM1_SCR 0x3ff > +#define COM1_BASE 0x3f8 > +#define COM1_DATA COM1_BASE+COM_OFFSET_DATA > +#define COM1_IER COM1_BASE+COM_OFFSET_IER > +#define COM1_IIR COM1_BASE+COM_OFFSET_IIR > +#define COM1_LCR COM1_BASE+COM_OFFSET_LCR > +#define COM1_MCR COM1_BASE+COM_OFFSET_MCR > +#define COM1_LSR COM1_BASE+COM_OFFSET_LSR > +#define COM1_MSR COM1_BASE+COM_OFFSET_MSR > +#define COM1_SCR COM1_BASE+COM_OFFSET_SCR > + > +#define COM_OFFSET_DATA 0 > +#define COM_OFFSET_IER 1 > +#define COM_OFFSET_IIR 2 > +#define COM_OFFSET_LCR 3 > +#define COM_OFFSET_MCR 4 > +#define COM_OFFSET_LSR 5 > +#define COM_OFFSET_MSR 6 > +#define COM_OFFSET_SCR 7 > + > +/* ns8250 port identifier */ > +enum ns8250_portid { > + NS8250_COM1, > + NS8250_COM2, > +}; > > /* ns8250 UART registers */ > struct ns8250_regs { > @@ -50,6 +66,7 @@ struct ns8250_dev { > struct event rate; > struct event wake; > struct timeval rate_tv; > + enum ns8250_portid portid; > int fd; > int irq; > int rcv_pending;