Re: [Minios-devel] [UNIKRAFT PATCH 2/2] plat/kvm: Update VGA console cursor location

[Florian Schmidt <florian@xxxxxxxxx>]

On 08/02/2018 07:03 AM, Florian Schmidt wrote:
> > > >   /* Hardware text mode color constants. */
> > > > @@ -109,6 +111,36 @@ static void vga_scroll(void)
> > > >               = vga_entry(' ', terminal_color);
> > > >   }
> > > > +static void vga_update_cursor(void)
> > > > +{
> > > > +    unsigned long irq_flags;
> > > > +    uint8_t old;
> > > > +    uint8_t ioas;  // VGA Input/Output select
> > > > +    uint16_t areg; // VGA address register
> > > > +    uint16_t dreg; // VGA data register
> > > > +
> > > > +    local_irq_save(irq_flags);
> > I wonder if it makes sense to protect the whole 
> > "_libkvmplat_vga_putc()" function where this function is just called 
> > from. This would truly enable printing from multiple threads since it 
> > protects the global state in this VGA driver.
> Hmm. I'm not a big fan of disabling interrupts for large amounts of 
> code, including several functions. Long strings can take time to print 
> out, and I'm not sure I feel comfortable disabling interrupts for that 
> long If at all, I would suggest doing that with other kinds of locking 
> mechanisms. I'm also wondering, though, whether it shouldn't be the job 
> of whatever's calling coutk. The typical behavior on most other systems 
> is that, if you print from several threads without synchronization, your 
> output will be interleaved, right?
Nevermind, I see you were talking about the putc function (it really is 
a bit too early in the morning). So this wouldn't disable interrupts for 
massive amounts of time if the string is longer, because it works 
char-by-char.
Nevertheless, I'm still not sure. Should we disable interrupts for the 
whole function? Should we disable interrupts at all? My reasoning was: I 
disable interrupts there because I interact with (vritualized) hardware. 
If I get interupted after setting areg, but before writing to dreg, this 
could end in me writing to some other place, which sounds bad and could 
leave the hardware in a broken state. On the other hand, your worry is 
about leaving terminal_{column,row} in a broken state in 
_libkvmplat_vga_putc(), right? That's also a good point, but I wouldn't 
disable interrupts for that.
What about adding a "vga mutex" to vga_console.c and using it in 
_libkvmplat_vga_putc(), dependent on libuklock becing compiled in? That 
would give us locking in those case. I can't think of a very realistic 
scenario in which one would have several threads running in one 
unikernel (and them all printing out stuff) without any locking 
primitives, and thus libuklock not compiled in.
> > > > +    /* Location of the address and data registers is variable and 
> > > > denoted
> > > > +     * by the least significant bit in the Input/Output register.
> > > > +     */
> > > > +    ioas = inb(0x3cc) & 0x1;
> > > > +    if (ioas) {
> > > > +        areg = 0x3d4;
> > > > +        dreg = 0x3d5;
> > > > +    } else {
> > > > +        areg = 0x3b4;
> > > > +        dreg = 0x3b5;
> > > > +    }
> > Is this changing during runtime or could be figure out the location 
> > during initialization and use it?
> I honestly don't know. I wouldn't expect this to change after it is set 
> once, but I don't know for sure. Supposedly, 0x3b is mostly there for 
> compatibility with older monochrome adapters, so it probably should be 
> 0x3d pretty much always. If we go by the "this is a driver for QEMU VGA 
> only", we can probably scrap it altogether. Otherwise, it's *probably* 
> fine to just check it once in the beginning.
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel