On 14.02.20 21:42, Michael Hinton wrote:
> Hello,
>
>
> I wanted to get some feedback on how Jailhouse is implementing newlines
> for console printing.
>
>
> Here's the code:
>
> https://github.com/siemens/jailhouse/blob/92db71f257fabd3c08fa4b99498fa61a41ea831d/inmates/lib/printk.c#L60-L63
>
> https://github.com/siemens/jailhouse/blob/92db71f257fabd3c08fa4b99498fa61a41ea831d/hypervisor/uart.c#L25-L28
>
>
> You can see that the code is doing this:
>
>
> if (c == '\n')
>
> c = '\r';
>
> else
>
> c = *msg++;
>
>
> So if the last character printed was '\n'(the Line Feed character), this
> code injects an '\r'(the Carriage Return character). I’m assuming the
> reason for this is just in case the console output is on Windows.
> Looking through the git history, it looks like this pattern has been in
> Jailhouse since the very beginning.
>
>
> This seems incorrect to me. The thing is, Windows newlines are CR+ LF,
> in that order, while Jailhouse is printing LF+ CR, which doesn’t match
> Unix _or_ Windows. See
> https://en.wikipedia.org/wiki/Newline#Representation. However, maybe
> there is a good reason for this that I don’t see yet.
>
>
> I’m consuming the Jailhouse console output in Linux, not Windows, but
> this still causes trouble because the extra CRafter the LFcan mess up
> awk (and possibly other line-based parsing tools). For more details, see
> https://stackoverflow.com/questions/60203007/awk-is-only-matching-the-first-line-when-matching-against-first-column.
>
>
> One solution is to reverse the order: inject CR _before_ printing LF.
> Another solution is to not inject CR at all, and leave it to the users
> of printk() to manually insert CR when needed.
>
>
> Let me know what you all think.
>
Well, we need the CR because we are also writing to real UARTs. If there
are tooling issues with the current reverse order (which was simpler to
implement IIRC), we can flip that, e.g. like this:
diff --git a/hypervisor/uart.c b/hypervisor/uart.c
index a43773c2..4f4eba4e 100644
--- a/hypervisor/uart.c
+++ b/hypervisor/uart.c
@@ -19,16 +19,19 @@ struct uart_chip *uart = NULL;
void uart_write(const char *msg)
{
- char c = 0;
+ char c, cached_c = 0;
while (1) {
- if (c == '\n')
- c = '\r';
- else
- c = *msg++;
+ c = cached_c ? : *msg++;
if (!c)
break;
+ cached_c = 0;
+ if (c == '\n') {
+ cached_c = c;
+ c = '\r';
+ }
+
while (uart->is_busy(uart))
cpu_relax();
if (panic_in_progress && panic_cpu != phys_processor_id())
Jan
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/eae57764-d282-e692-f113-b3360c6a06ad%40web.de.
