[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 30.07.2018 15:15, Florian Schmidt wrote:
On 07/30/2018 02:23 PM, Simon Kuenzer wrote:
On 29.07.2018 12:44, Florian Schmidt wrote:
On 07/29/2018 12:39 PM, Florian Schmidt wrote:
Update the location of the cursor as data is written to the console.
Note that this does not set the cursor location registers, hence all we

And I just realized this isn't very clear. This of course sets the "Cursor Location {High,Low}" registers, but it doesn't set the "Cursor {Start,End}" registers. I won't send a v2 for just that. Waiting for other potential comments instead.

Hum, I am still not getting it from the description. ;-) How is printing a block different from a blinking underline?

OK, you're right, I should explain this more.
There are a bunch of registers involved in setting the cursor. There is "Cursor Location {High,Low}", which is effectively a 16-bit register (split into 2, because registers are only addressable byte-wise). That register encodes the location of the cursor by just counting the text positions. The first one is 0, the 3d character on the 6th line is (5*VGA_WIDTH + 3), etc. Of course, height and width are mode dependent...

In addition, there are also two registers called "Cursor Start" and "Cursor End". Those registers encode, in scanlines (i.e., "pixels", kinda), the beginning and the end of the blinking cursor box. By setting Start to 0 and End to MAXLINES (which, again, is mode-dependent and can be read from yet another register; for 80x25, it is 0x0f), you get the full-size "box" cursor. In QEMU, the default for these values is 0x0d and 0x0e, which gives you the "blinking underline" instead of the "blinking box". Yes, this also means you can have a cursor that blink at the top of a character, or in the middle, if anyone ever wanted to...


Not sure whether all of that should go into the commit message though. ;-)

Thanks for the explanation. What about this as message?:

Update the location of the cursor as data is written to the console.
Since we only set the location of a blinking underline character, we use
the Cursor Location {High,Low} registers.




get is the blinking underline character, not the full-size block
character.

Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
---
  plat/kvm/x86/vga_console.c | 33 +++++++++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)

diff --git a/plat/kvm/x86/vga_console.c b/plat/kvm/x86/vga_console.c
index a1b5cf7..efcddde 100644
--- a/plat/kvm/x86/vga_console.c
+++ b/plat/kvm/x86/vga_console.c
@@ -29,6 +29,8 @@
  #include <sys/types.h>
  #include <stdint.h>
  #include <string.h>
+#include <x86/cpu.h>
+#include <x86/irq.h>
  #include <kvm-x86/vga_console.h>

Do you know if we better enable the cursor? I am not sure if we can assume a common state in which the boot loader left us.
https://wiki.osdev.org/Text_Mode_Cursor#Enabling_the_Cursor_2

  /* 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.

+    /* 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?

+
+    old = inb(areg);
+    outb(areg, 0x0e); // Cursor Location High
+    outb(dreg, ((terminal_row * VGA_WIDTH) + terminal_column) >> 8);
+    outb(areg, 0x0f); // Cursor Location Low
+    outb(dreg, ((terminal_row * VGA_WIDTH) + terminal_column) & 0xff);
+    outb(areg, old);
+    local_irq_restore(irq_flags);
+}
+
  static void vga_newline(void)
  {
      if (terminal_row == VGA_HEIGHT - 1)
@@ -160,4 +192,5 @@ void _libkvmplat_vga_putc(char c)
          }
          break;
      }
+    vga_update_cursor();
  }


Thanks,

Simon


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.