[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH 2/2] plat/kvm: Update VGA console cursor location
On 02.08.2018 07:39, Florian Schmidt wrote:
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.
Hum... I rather prefer disabling interrupts over a mutex here because
(1) mutex would not work when print from the interrupt context (I know,
this is a bad thing but it works for now and printing is anyway you
should not do to much because it is slow)
(2) it enables printing from interrupt context (again: I know this is
something bad but it might be still useful to have)
(3) I do not like to many dependencies to general libraries implied by
the platform. libuklock require libuksched and then you cant compile a
plain kvm Unikernel without scheduling anymore.
Can't you just disable interrupts for small period of time in the code,
e.g., when interacting with the registers and adding a character to the
VGA buffer?
+ /* 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.
I am not an expert either but depending on your explanation and what
I've seen so far it looks like we just need to do it once. ;-) What do
you think?
_______________________________________________
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
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
