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;

Reply via email to