[PATCH 0/2] Early printk support for virtio console devices.

[PATCH 0/2] Early printk support for virtio console devices.

[Pranavkumar Sawargaonkar]

This patch-set implements early printk support for virtio console devices without using any hypercalls.

The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.

This implementation adds:
1. Early writeonly register named early_wr in virtio console's config space.
2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.

Early write mechanism:
1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.

Pranavkumar Sawargaonkar (2):
  virtio: console: Add early writeonly register to config space
  arm64: earlyprintk support for virtio-mmio console

 Documentation/virtual/virtio-spec.txt |   13 ++++++++++--
 arch/arm64/kernel/early_printk.c      |   35 +++++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_console.h   |    3 +++
 3 files changed, 49 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] virtio: console: Add early writeonly register to config space

[Pranavkumar Sawargaonkar]

This patch adds a early_wr register (writeonly) in config space of virtio console device which can be used for debugging.

The patch also updates virtio-spec in Documentation to reflect this feature addition in virtio console.

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Signed-off-by: Anup Patel <anup.patel@linaro.org>
---
 Documentation/virtual/virtio-spec.txt |   13 +++++++++++--
 include/uapi/linux/virtio_console.h   |    3 +++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/virtio-spec.txt b/Documentation/virtual/virtio-spec.txt
index 0d6ec85..bc4e8b5 100644
--- a/Documentation/virtual/virtio-spec.txt
+++ b/Documentation/virtual/virtio-spec.txt
@@ -1927,11 +1927,18 @@ Ports 2 onwards only if VIRTIO_CONSOLE_F_MULTIPORT is set
     ports; configuration fields nr_ports and max_nr_ports are
     valid and control virtqueues will be used.
 
+  VIRTIO_CONSOLE_F_EARLY_WRITE(2) Device has support for early
+  write. Configuration field early_wr is valid.
+
   Device configuration layout The size of the console is supplied
   in the configuration space if the VIRTIO_CONSOLE_F_SIZE feature
   is set. Furthermore, if the VIRTIO_CONSOLE_F_MULTIPORT feature
   is set, the maximum number of ports supported by the device can
-  be fetched.struct virtio_console_config {
+  be fetched. If VIRTIO_CONSOLE_F_EARLY_WRITE is set then one can
+  use early write to output single character without initializing
+  virtio queues.
+
+  struct virtio_console_config {
 
 	u16 cols;
 
@@ -1941,7 +1948,9 @@ Ports 2 onwards only if VIRTIO_CONSOLE_F_MULTIPORT is set
 
 	u32 max_nr_ports;
 
-};
+	u32 early_wr;
+
+  };
 
   Device Initialization
 
diff --git a/include/uapi/linux/virtio_console.h b/include/uapi/linux/virtio_console.h
index ee13ab6..7cd84b5 100644
--- a/include/uapi/linux/virtio_console.h
+++ b/include/uapi/linux/virtio_console.h
@@ -38,6 +38,7 @@
 /* Feature bits */
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
 #define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
+#define VIRTIO_CONSOLE_F_EARLY_WRITE 2	/* Does host support early write? */
 
 #define VIRTIO_CONSOLE_BAD_ID		(~(u32)0)
 
@@ -48,6 +49,8 @@ struct virtio_console_config {
 	__u16 rows;
 	/* max. number of ports this device can hold */
 	__u32 max_nr_ports;
+	/* early write register */
+	__u32 early_wr;
 } __attribute__((packed));
 
 /*
-- 
1.7.9.5

[PATCH 1/2] virtio: console: Add early writeonly register to config space

[Rusty Russell]

Pranavkumar Sawargaonkar <pranavkumar@linaro.org> writes:
> This patch adds a early_wr register (writeonly) in config space of virtio console device which can be used for debugging.
>
> The patch also updates virtio-spec in Documentation to reflect this feature addition in virtio console.

Perhaps change the name to VIRTIO_CONSOLE_F_EMERGENCY_WRITE.  But let's
resolve the qemu discussion first...

Cheers,
Rusty.

[PATCH 2/2] arm64: earlyprintk support for virtio-mmio console

[Pranavkumar Sawargaonkar]

This patch implements earlyprintk based on virtio console using early_wr config register.

Kernel args for using this will be: earlyprintk=virtio-console,<phys_address>

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Signed-off-by: Anup Patel <anup.patel@linaro.org>
---
 arch/arm64/kernel/early_printk.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm64/kernel/early_printk.c b/arch/arm64/kernel/early_printk.c
index ac974f4..52ac6b0 100644
--- a/arch/arm64/kernel/early_printk.c
+++ b/arch/arm64/kernel/early_printk.c
@@ -25,6 +25,9 @@
 
 #include <linux/amba/serial.h>
 #include <linux/serial_reg.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_mmio.h>
+#include <linux/virtio_console.h>
 
 static void __iomem *early_base;
 static void (*printch)(char ch);
@@ -53,6 +56,37 @@ static void smh_printch(char ch)
 }
 
 /*
+ * VIRTIO MMIO console single character Tx.
+ */
+static void virtio_console_printch(char ch)
+{
+	u32 tmp;
+	static u32 init_done;
+	static u32 can_write;
+	struct virtio_console_config *p = early_base + VIRTIO_MMIO_CONFIG;
+
+	if (!init_done) {
+		tmp = readl_relaxed(early_base + VIRTIO_MMIO_DEVICE_ID);
+		if (tmp != VIRTIO_ID_CONSOLE) {
+			init_done = 1;
+			return;
+		}
+
+		tmp = readl_relaxed(early_base + VIRTIO_MMIO_HOST_FEATURES);
+		if (!(tmp & (1 << VIRTIO_CONSOLE_F_EARLY_WRITE))) {
+			init_done = 1;
+			return;
+		}
+
+		init_done = 1;
+		can_write = 1;
+	}
+
+	if (can_write)
+		writeb_relaxed(ch, &p->early_wr);
+}
+
+/*
  * 8250/16550 (8-bit aligned registers) single character TX.
  */
 static void uart8250_8bit_printch(char ch)
@@ -82,6 +116,7 @@ static const struct earlycon_match earlycon_match[] __initconst = {
 	{ .name = "smh", .printch = smh_printch, },
 	{ .name = "uart8250-8bit", .printch = uart8250_8bit_printch, },
 	{ .name = "uart8250-32bit", .printch = uart8250_32bit_printch, },
+	{ .name = "virtio-console", .printch = virtio_console_printch, },
 	{}
 };
 
-- 
1.7.9.5

[PATCH 0/2] Early printk support for virtio console devices.

[Alexander Graf]

On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:

> This patch-set implements early printk support for virtio console devices without using any hypercalls.
> 
> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
> 
> This implementation adds:
> 1. Early writeonly register named early_wr in virtio console's config space.
> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
> 
> Early write mechanism:
> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.

I won't nack this patch set, but I'll definitely express that I'm not happy with it.

MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.

I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.


Alex

[PATCH 0/2] Early printk support for virtio console devices.

[Peter Maydell]

On 26 April 2013 12:19, Alexander Graf <agraf@suse.de> wrote:
> MMIO registers are handled by a different layer than the virtio
> console itself. After the virtio refactoring in QEMU, they will
> be completely separate drivers.

Good point -- we don't really want to be mixing up the
transport and the backend. You can see it in the kvmtool
patch, in fact -- it introduces an "if this is virtio-console"
special case into the mmio.c file which previously was
entirely backend agnostic.

thanks
-- PMM

[PATCH 0/2] Early printk support for virtio console devices.

[Anup Patel]

On 26 April 2013 17:03, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 April 2013 12:19, Alexander Graf <agraf@suse.de> wrote:
>> MMIO registers are handled by a different layer than the virtio
>> console itself. After the virtio refactoring in QEMU, they will
>> be completely separate drivers.
>
> Good point -- we don't really want to be mixing up the
> transport and the backend. You can see it in the kvmtool
> patch, in fact -- it introduces an "if this is virtio-console"
> special case into the mmio.c file which previously was
> entirely backend agnostic.

Well, we can always have virtio device specific config registers
handle by virtio device backends and generic virtio config register
handled by transport.

kvmtool patch is hacky because it does not provide virtio device
specific config read/write callbacks.

>
> thanks
> -- PMM
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

--Anup

[PATCH 0/2] Early printk support for virtio console devices.

[Arnd Bergmann]

On Friday 26 April 2013 17:36:16 Anup Patel wrote:
> On 26 April 2013 17:03, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 26 April 2013 12:19, Alexander Graf <agraf@suse.de> wrote:
> >> MMIO registers are handled by a different layer than the virtio
> >> console itself. After the virtio refactoring in QEMU, they will
> >> be completely separate drivers.
> >
> > Good point -- we don't really want to be mixing up the
> > transport and the backend. You can see it in the kvmtool
> > patch, in fact -- it introduces an "if this is virtio-console"
> > special case into the mmio.c file which previously was
> > entirely backend agnostic.
> 
> Well, we can always have virtio device specific config registers
> handle by virtio device backends and generic virtio config register
> handled by transport.
> 
> kvmtool patch is hacky because it does not provide virtio device
> specific config read/write callbacks.

Couldn't kvmtool implement the interface used by smh_printch()
for early output instead?

Or if that's not a fitting inteface, maybe a psci call for writing
a character to the console?

	Arnd

[PATCH 0/2] Early printk support for virtio console devices.

[Anup Patel]

On 26 April 2013 18:03, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 26 April 2013 17:36:16 Anup Patel wrote:
>> On 26 April 2013 17:03, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > On 26 April 2013 12:19, Alexander Graf <agraf@suse.de> wrote:
>> >> MMIO registers are handled by a different layer than the virtio
>> >> console itself. After the virtio refactoring in QEMU, they will
>> >> be completely separate drivers.
>> >
>> > Good point -- we don't really want to be mixing up the
>> > transport and the backend. You can see it in the kvmtool
>> > patch, in fact -- it introduces an "if this is virtio-console"
>> > special case into the mmio.c file which previously was
>> > entirely backend agnostic.
>>
>> Well, we can always have virtio device specific config registers
>> handle by virtio device backends and generic virtio config register
>> handled by transport.
>>
>> kvmtool patch is hacky because it does not provide virtio device
>> specific config read/write callbacks.
>
> Couldn't kvmtool implement the interface used by smh_printch()
> for early output instead?
>
> Or if that's not a fitting inteface, maybe a psci call for writing
> a character to the console?
>
>         Arnd

I am curious about how smh-based or hypercall-based early prints would
be handled in following scenario:

"A board is running KVM ARM enabled kernel and linux console on serial
port. Now a user remotely connects to the board via telnet/ssh and
launches a VM with smh-based or hypercall-based earlyprintk."

In the above scenario, will smh-based or hypercall-based earlyprints
appear to user on remote shell or not ?

--Anup

[PATCH 0/2] Early printk support for virtio console devices.

[Arnd Bergmann]

On Friday 26 April 2013, Anup Patel wrote:
> I am curious about how smh-based or hypercall-based early prints would
> be handled in following scenario:
> 
> "A board is running KVM ARM enabled kernel and linux console on serial
> port. Now a user remotely connects to the board via telnet/ssh and
> launches a VM with smh-based or hypercall-based earlyprintk."
> 
> In the above scenario, will smh-based or hypercall-based earlyprints
> appear to user on remote shell or not ?

The implementation of the console device would still be done by KVM,
as for any other virtual device, so it shows up whereever kvm is
configured to output the console data.

	Arnd

[PATCH 0/2] Early printk support for virtio console devices.

[Marc Zyngier]

On 26/04/13 16:03, Arnd Bergmann wrote:
> On Friday 26 April 2013, Anup Patel wrote:
>> I am curious about how smh-based or hypercall-based early prints would
>> be handled in following scenario:
>>
>> "A board is running KVM ARM enabled kernel and linux console on serial
>> port. Now a user remotely connects to the board via telnet/ssh and
>> launches a VM with smh-based or hypercall-based earlyprintk."
>>
>> In the above scenario, will smh-based or hypercall-based earlyprints
>> appear to user on remote shell or not ?
> 
> The implementation of the console device would still be done by KVM,
> as for any other virtual device, so it shows up whereever kvm is
> configured to output the console data.

Actually, at least in the case of smh-based earlyprintk, the output only
appears in the model console, and never reach the rest of the kernel.

And any other non-MMIO approach (like a hypercall) will be very hard to
feed back to the console, as KVM itself has no idea of what a "console"
is. You'd need another side channel to userspace, hoping that it will be
able to merge the multiple streams.

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 0/2] Early printk support for virtio console devices.

[Peter Maydell]

On 26 April 2013 13:33, Arnd Bergmann <arnd@arndb.de> wrote:
> Couldn't kvmtool implement the interface used by smh_printch()
> for early output instead?
>
> Or if that's not a fitting inteface, maybe a psci call for writing
> a character to the console?

I suggested that earlier:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-April/005433.html

and Alex pointed out that s390-virtio experience with that
approach is that it's awkward for userspace.

-- PMM

[PATCH 0/2] Early printk support for virtio console devices.

[Rusty Russell]

Alexander Graf <agraf@suse.de> writes:
> On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:
>
>> This patch-set implements early printk support for virtio console devices without using any hypercalls.
>> 
>> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
>> 
>> This implementation adds:
>> 1. Early writeonly register named early_wr in virtio console's config space.
>> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
>> 
>> Early write mechanism:
>> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.
>
> I won't nack this patch set, but I'll definitely express that I'm not happy with it.
>
> MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.
>
> I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.

Well, this shouldn't be mmio-specific, but I kind of get what you mean.

I consider this misnamed: it's an emergency write facility.  Linux may
use it for an early console, but it's also useful for bringup and to
give a method of emitting errors like "the console ring is corrupt".

A valid implementation may well be to only offer it with some magic
qemu developer-only commandline and dump it to stdout.

So I think it has use, but less so if QEMU isn't ever going to implement
it.

Cheers,
Rusty.

[PATCH 0/2] Early printk support for virtio console devices.

[Alexander Graf]

Am 29.04.2013 um 05:09 schrieb Rusty Russell <rusty@rustcorp.com.au>:

> Alexander Graf <agraf@suse.de> writes:
>> On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:
>> 
>>> This patch-set implements early printk support for virtio console devices without using any hypercalls.
>>> 
>>> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
>>> 
>>> This implementation adds:
>>> 1. Early writeonly register named early_wr in virtio console's config space.
>>> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
>>> 
>>> Early write mechanism:
>>> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.
>> 
>> I won't nack this patch set, but I'll definitely express that I'm not happy with it.
>> 
>> MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.
>> 
>> I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.
> 
> Well, this shouldn't be mmio-specific, but I kind of get what you mean.
> 
> I consider this misnamed: it's an emergency write facility.  Linux may
> use it for an early console,

If Linux uses it for early console, you won't see any messages from before the virtio-console driver is initialized, because Linux thinks that it's all been printed out.

> but it's also useful for bringup and to
> give a method of emitting errors like "the console ring is corrupt".
> 
> A valid implementation may well be to only offer it with some magic
> qemu developer-only commandline and dump it to stdout.

Why implement it differently from other machines? There are facilities to call into firmware, so you could use that. There's the special Foundation model call that you could implement and reuse for this.

I don't see why anything like this has to live in virtio-mmio. Oh, and it should default to off.

Alex

> 
> So I think it has use, but less so if QEMU isn't ever going to implement
> it.
> 
> Cheers,
> Rusty.
> 

[PATCH 0/2] Early printk support for virtio console devices.

[Pranavkumar Sawargaonkar]

On 29 April 2013 17:52, Alexander Graf <agraf@suse.de> wrote:
>
>
> Am 29.04.2013 um 05:09 schrieb Rusty Russell <rusty@rustcorp.com.au>:
>
>> Alexander Graf <agraf@suse.de> writes:
>>> On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:
>>>
>>>> This patch-set implements early printk support for virtio console devices without using any hypercalls.
>>>>
>>>> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
>>>>
>>>> This implementation adds:
>>>> 1. Early writeonly register named early_wr in virtio console's config space.
>>>> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
>>>>
>>>> Early write mechanism:
>>>> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.
>>>
>>> I won't nack this patch set, but I'll definitely express that I'm not happy with it.
>>>
>>> MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.
>>>
>>> I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.
>>
>> Well, this shouldn't be mmio-specific, but I kind of get what you mean.
>>
>> I consider this misnamed: it's an emergency write facility.  Linux may
>> use it for an early console,
>
> If Linux uses it for early console, you won't see any messages from before the virtio-console driver is initialized, because Linux thinks that it's all been printed out.

So far i have tried this on foundation model for armv8 and i never saw
any missed out prints.

>
>> but it's also useful for bringup and to
>> give a method of emitting errors like "the console ring is corrupt".
>>
>> A valid implementation may well be to only offer it with some magic
>> qemu developer-only commandline and dump it to stdout.
>
> Why implement it differently from other machines? There are facilities to call into firmware, so you could use that. There's the special Foundation model call that you could implement and reuse for this.
>
> I don't see why anything like this has to live in virtio-mmio. Oh, and it should default to off.

All the boards may not have firmware to do this. Foundation model call
is also not useful on real hardware for armv8.  This is very helpful
in debugging a guest on real hardware when guest crashes in early
stages before virtio drivers come in to the picture.
This is not a virtio-mmio change it is just an addition of device
specific register in virtio console.
>
> Alex
>
>>
>> So I think it has use, but less so if QEMU isn't ever going to implement
>> it.
>>
>> Cheers,
>> Rusty.
>>
Thanks,
Pranav

[PATCH 0/2] Early printk support for virtio console devices.

[Alexander Graf]

On 29.04.2013, at 14:48, Pranavkumar Sawargaonkar wrote:

> On 29 April 2013 17:52, Alexander Graf <agraf@suse.de> wrote:
>> 
>> 
>> Am 29.04.2013 um 05:09 schrieb Rusty Russell <rusty@rustcorp.com.au>:
>> 
>>> Alexander Graf <agraf@suse.de> writes:
>>>> On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:
>>>> 
>>>>> This patch-set implements early printk support for virtio console devices without using any hypercalls.
>>>>> 
>>>>> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
>>>>> 
>>>>> This implementation adds:
>>>>> 1. Early writeonly register named early_wr in virtio console's config space.
>>>>> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
>>>>> 
>>>>> Early write mechanism:
>>>>> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.
>>>> 
>>>> I won't nack this patch set, but I'll definitely express that I'm not happy with it.
>>>> 
>>>> MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.
>>>> 
>>>> I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.
>>> 
>>> Well, this shouldn't be mmio-specific, but I kind of get what you mean.
>>> 
>>> I consider this misnamed: it's an emergency write facility.  Linux may
>>> use it for an early console,
>> 
>> If Linux uses it for early console, you won't see any messages from before the virtio-console driver is initialized, because Linux thinks that it's all been printed out.
> 
> So far i have tried this on foundation model for armv8 and i never saw
> any missed out prints.

It's what I got back on s390. If you run upstream QEMU, you'll simply lose half of your log messages. Because in upstream QEMU, the side channel used for early printk got nack'ed.

> 
>> 
>>> but it's also useful for bringup and to
>>> give a method of emitting errors like "the console ring is corrupt".
>>> 
>>> A valid implementation may well be to only offer it with some magic
>>> qemu developer-only commandline and dump it to stdout.
>> 
>> Why implement it differently from other machines? There are facilities to call into firmware, so you could use that. There's the special Foundation model call that you could implement and reuse for this.
>> 
>> I don't see why anything like this has to live in virtio-mmio. Oh, and it should default to off.
> 
> All the boards may not have firmware to do this. Foundation model call
> is also not useful on real hardware for armv8.  This is very helpful
> in debugging a guest on real hardware when guest crashes in early

Are you seriously considering to use virtio-console on real hardware?

> stages before virtio drivers come in to the picture.
> This is not a virtio-mmio change it is just an addition of device
> specific register in virtio console.

There are not device specific registers in virtio-console. Virtio-console lives behind a virtio bus which doesn't know what these registers are. Even if you shove it into config space, it'd be broken because config access happens without intercepts on some platforms.


Alex

[PATCH 0/2] Early printk support for virtio console devices.

[Rusty Russell]

Alexander Graf <agraf@suse.de> writes:
> There are not device specific registers in
> virtio-console. Virtio-console lives behind a virtio bus which doesn't
> know what these registers are.

You're not going to make coherent arguments without reading that actual
patch we're discussing.  And you're going to just waste everyone else's
time.

> Even if you shove it into config space,

Which is what the patch does...

> it'd be broken because config access happens without intercepts on
> some platforms.

But people keep assuming it, which is one reason virtio_ccw switched
from passive lguest-style config to active.  Only lguest and the old kvm
virtio use it, and lguest can change.

An emergency output is a reasonable idea, and this is a reasonable
implementation.  The question is practical: will it be used?  Because we
don't implement reasonable ideas which aren't going to be used.

Cheers,
Rusty.

[PATCH 0/2] Early printk support for virtio console devices.

[Anup Patel]

On Wed, May 1, 2013 at 7:37 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Alexander Graf <agraf@suse.de> writes:
>> There are not device specific registers in
>> virtio-console. Virtio-console lives behind a virtio bus which doesn't
>> know what these registers are.
>
> You're not going to make coherent arguments without reading that actual
> patch we're discussing.  And you're going to just waste everyone else's
> time.
>
>> Even if you shove it into config space,
>
> Which is what the patch does...
>
>> it'd be broken because config access happens without intercepts on
>> some platforms.
>
> But people keep assuming it, which is one reason virtio_ccw switched
> from passive lguest-style config to active.  Only lguest and the old kvm
> virtio use it, and lguest can change.
>
> An emergency output is a reasonable idea, and this is a reasonable
> implementation.  The question is practical: will it be used?  Because we
> don't implement reasonable ideas which aren't going to be used.

We are already using it for Guest bring-up on APM ARM64 board using
KVMTOOL and so far it has been really helpful in debugging KVM ARM64
guest issues.

Also, this is a very useful debugging method for Guest bring-up on
other architectures supported by KVM and other Hypervisors that
support VirtIO.

>
> Cheers,
> Rusty.
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

Regards,
Anup

[PATCH 0/2] Early printk support for virtio console devices.

[Will Deacon]

On Wed, May 01, 2013 at 07:53:49AM +0100, Anup Patel wrote:
> On Wed, May 1, 2013 at 7:37 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > Alexander Graf <agraf@suse.de> writes:
> >> There are not device specific registers in
> >> virtio-console. Virtio-console lives behind a virtio bus which doesn't
> >> know what these registers are.
> >
> > You're not going to make coherent arguments without reading that actual
> > patch we're discussing.  And you're going to just waste everyone else's
> > time.
> >
> >> Even if you shove it into config space,
> >
> > Which is what the patch does...
> >
> >> it'd be broken because config access happens without intercepts on
> >> some platforms.
> >
> > But people keep assuming it, which is one reason virtio_ccw switched
> > from passive lguest-style config to active.  Only lguest and the old kvm
> > virtio use it, and lguest can change.
> >
> > An emergency output is a reasonable idea, and this is a reasonable
> > implementation.  The question is practical: will it be used?  Because we
> > don't implement reasonable ideas which aren't going to be used.
> 
> We are already using it for Guest bring-up on APM ARM64 board using
> KVMTOOL and so far it has been really helpful in debugging KVM ARM64
> guest issues.
> 
> Also, this is a very useful debugging method for Guest bring-up on
> other architectures supported by KVM and other Hypervisors that
> support VirtIO.

FWIW: I've hacked together a small set of patches allowing us to use the
x86 ioport peripherals on arm/arm64 in kvmtool. This means you can use the
8250 for earlyprintk on the latter, and even as the main console if you
like since it has a device-tree binding.

I'll post the patches as an RFC to the kvm list.

Will

[PATCH 0/2] Early printk support for virtio console devices.

[Peter Maydell]

On 1 May 2013 03:07, Rusty Russell <rusty@rustcorp.com.au> wrote:
> An emergency output is a reasonable idea, and this is a reasonable
> implementation.  The question is practical: will it be used?  Because we
> don't implement reasonable ideas which aren't going to be used.

If you think it fits reasonably into the virtio spec (ie doesn't
implement things at the wrong level of the transport/backend
abstraction) then we can implement it in QEMU, and I think it
makes more sense to do this than to throw in a random extra
serial port.

To be actually useful we need to also specify something in
the device tree to say "here is where you will find your
emergency output and what it is".

thanks
-- PMM

[PATCH 0/2] Early printk support for virtio console devices.

[Rusty Russell]

Peter Maydell <peter.maydell@linaro.org> writes:
> On 1 May 2013 03:07, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> An emergency output is a reasonable idea, and this is a reasonable
>> implementation.  The question is practical: will it be used?  Because we
>> don't implement reasonable ideas which aren't going to be used.
>
> If you think it fits reasonably into the virtio spec (ie doesn't
> implement things at the wrong level of the transport/backend
> abstraction) then we can implement it in QEMU, and I think it
> makes more sense to do this than to throw in a random extra
> serial port.
>
> To be actually useful we need to also specify something in
> the device tree to say "here is where you will find your
> emergency output and what it is".

Hmm, I'm not sure that's true.  It looks like it needs:

1) An enhancment to the vdev->set_config callback to pass through (at
   least) an offset, probably offset and length.

2) An emerg_write() function ptr which can be called at any time, set by
   virtio_console.c's class_init:

static void emerg_write(VirtIOSerialPort *port, char c)
{
    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);

    if (vcon->chr)
        qemu_chr_fe_write(vcon->chr, &c, 1);
}

3) A routine to find an emerg-write-capable console in
   virtio_serial_bus.c (or just assume port 0?):

static VirtIOSerialPort *find_emerg_write_port(VirtIOSerial *vser)
{
    VirtIOSerialPort *port;

    QTAILQ_FOREACH(port, &vser->ports, next) {
        VirtIOSerialPortClass *vsc;
        vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);

        if (vsc->emerg_write) {
            return port;
        }
    }
    return NULL;
}

4) set_config in virtio_serial_bus.c to notice emergency writes:

    if (offset == offsetof(struct virtio_console_config, emerg_w) {
        VirtIOSerial *vser;
        vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
        VirtIOSerialPort *port;

        port = find_emerg_write_port(vser);
        if (port) {
            vsc->emerg_write(port, config.emerg_w);
        }
    }

Amit might have more clue... Amit?

Thanks,
Rusty.

From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Subject: virtio: console: Add early writeonly register to config space

This patch adds a emerg_wr register (writeonly) in config space of virtio console device which can be used for debugging.

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 include/uapi/linux/virtio_console.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/virtio_console.h b/include/uapi/linux/virtio_console.h
index ee13ab6..586678d 100644
--- a/include/uapi/linux/virtio_console.h
+++ b/include/uapi/linux/virtio_console.h
@@ -38,6 +38,7 @@
 /* Feature bits */
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
 #define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
+#define VIRTIO_CONSOLE_F_EMERG_WRITE 2	/* Does host support emergency write? */
 
 #define VIRTIO_CONSOLE_BAD_ID		(~(u32)0)
 
@@ -48,6 +49,8 @@ struct virtio_console_config {
 	__u16 rows;
 	/* max. number of ports this device can hold */
 	__u32 max_nr_ports;
+	/* emergency write register */
+	__u32 emerg_wr;
 } __attribute__((packed));
 
 /*

[PATCH 0/2] Early printk support for virtio console devices.

[Peter Maydell]

On 6 May 2013 06:11, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> To be actually useful we need to also specify something in
>> the device tree to say "here is where you will find your
>> emergency output and what it is".
>
> Hmm, I'm not sure that's true.  It looks like it needs:
>
> 1) An enhancment to the vdev->set_config callback to pass through (at
>    least) an offset, probably offset and length.
>
> 2) An emerg_write() function ptr which can be called at any time, set by
>    virtio_console.c's class_init:

That all looks like sensible QEMU implementation possibilities
but it seems to be a bit of a non-sequitur from "how do we
tell the kernel to actually use this?"

-- PMM

[PATCH 0/2] Early printk support for virtio console devices.

[Rusty Russell]

Peter Maydell <peter.maydell@linaro.org> writes:
> On 6 May 2013 06:11, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> To be actually useful we need to also specify something in
>>> the device tree to say "here is where you will find your
>>> emergency output and what it is".
>>
>> Hmm, I'm not sure that's true.  It looks like it needs:
>>
>> 1) An enhancment to the vdev->set_config callback to pass through (at
>>    least) an offset, probably offset and length.
>>
>> 2) An emerg_write() function ptr which can be called at any time, set by
>>    virtio_console.c's class_init:
>
> That all looks like sensible QEMU implementation possibilities
> but it seems to be a bit of a non-sequitur from "how do we
> tell the kernel to actually use this?"

You enable the feature in the virtio console device, and a kernel
compiled with EARLY_PRINTK will use it?

Cheers,
Rusty.

[PATCH 0/2] Early printk support for virtio console devices.

[Peter Maydell]

On 7 May 2013 05:46, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> That all looks like sensible QEMU implementation possibilities
>> but it seems to be a bit of a non-sequitur from "how do we
>> tell the kernel to actually use this?"
>
> You enable the feature in the virtio console device, and a kernel
> compiled with EARLY_PRINTK will use it?

Well, at the moment EARLY_PRINTK is hardcoded to
"use some specific UART or equivalent selected at
compile time". So the equivalent presumably would
be to hard-compile "use virtio-console", but then
how does that code know where the virtio-console
is in the address space?

-- PMM

[PATCH 0/2] Early printk support for virtio console devices.

[Christopher Covington]

On 05/07/2013 08:19 AM, Peter Maydell wrote:
> On 7 May 2013 05:46, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> That all looks like sensible QEMU implementation possibilities
>>> but it seems to be a bit of a non-sequitur from "how do we
>>> tell the kernel to actually use this?"
>>
>> You enable the feature in the virtio console device, and a kernel
>> compiled with EARLY_PRINTK will use it?
> 
> Well, at the moment EARLY_PRINTK is hardcoded to
> "use some specific UART or equivalent selected at
> compile time". So the equivalent presumably would
> be to hard-compile "use virtio-console", but then
> how does that code know where the virtio-console
> is in the address space?

arm64 uses a kernel argument.

Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

[PATCH 0/2] Early printk support for virtio console devices.

[Peter Maydell]

On 7 May 2013 16:52, Christopher Covington <cov@codeaurora.org> wrote:
> On 05/07/2013 08:19 AM, Peter Maydell wrote:
>> Well, at the moment EARLY_PRINTK is hardcoded to
>> "use some specific UART or equivalent selected at
>> compile time". So the equivalent presumably would
>> be to hard-compile "use virtio-console", but then
>> how does that code know where the virtio-console
>> is in the address space?
>
> arm64 uses a kernel argument.

This mixes up "information that the user provides to the
kernel" (ie configuration) with "information that QEMU or
kvmtool should provide to the kernel" (ie hardware
description), and would require QEMU/kvmtool to parse
the user's commandline tool to figure out if they
needed to override it or edit it (or alternatively,
would require the user to know internal details of
QEMU/kvmtool's address map for the guest). I think it
would be nicer to keep them separate.

thanks
-- PMM

[PATCH 0/2] Early printk support for virtio console devices.

[Catalin Marinas]

On Tue, May 07, 2013 at 04:58:23PM +0100, Peter Maydell wrote:
> On 7 May 2013 16:52, Christopher Covington <cov@codeaurora.org> wrote:
> > On 05/07/2013 08:19 AM, Peter Maydell wrote:
> >> Well, at the moment EARLY_PRINTK is hardcoded to
> >> "use some specific UART or equivalent selected at
> >> compile time". So the equivalent presumably would
> >> be to hard-compile "use virtio-console", but then
> >> how does that code know where the virtio-console
> >> is in the address space?
> >
> > arm64 uses a kernel argument.
> 
> This mixes up "information that the user provides to the
> kernel" (ie configuration) with "information that QEMU or
> kvmtool should provide to the kernel" (ie hardware
> description), and would require QEMU/kvmtool to parse
> the user's commandline tool to figure out if they
> needed to override it or edit it (or alternatively,
> would require the user to know internal details of
> QEMU/kvmtool's address map for the guest). I think it
> would be nicer to keep them separate.

I'm not sure it's worth it, the earlyprintk is meant for debugging the
early kernel booting code, not for general use (you have the normal
console for this). Someone doing platform port should know the address
of the uart and pass the right information on the kernel command line to
help debug early booting issues.

But if someone comes up with some bindings, I won't reject the patch.

-- 
Catalin

[PATCH 0/2] Early printk support for virtio console devices.

[Grant Likely]

On Wed, May 8, 2013 at 3:17 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, May 07, 2013 at 04:58:23PM +0100, Peter Maydell wrote:
>> On 7 May 2013 16:52, Christopher Covington <cov@codeaurora.org> wrote:
>> This mixes up "information that the user provides to the
>> kernel" (ie configuration) with "information that QEMU or
>> kvmtool should provide to the kernel" (ie hardware
>> description), and would require QEMU/kvmtool to parse
>> the user's commandline tool to figure out if they
>> needed to override it or edit it (or alternatively,
>> would require the user to know internal details of
>> QEMU/kvmtool's address map for the guest). I think it
>> would be nicer to keep them separate.
>
> I'm not sure it's worth it, the earlyprintk is meant for debugging the
> early kernel booting code, not for general use (you have the normal
> console for this). Someone doing platform port should know the address
> of the uart and pass the right information on the kernel command line to
> help debug early booting issues.
>
> But if someone comes up with some bindings, I won't reject the patch.

Nicolas and I have tossed back and forth the idea of having a trivial
binding in the DT which specifies a couple of physical addresses and a
mask; one write register, one status register, and the mask to say
which bit to care about in the status. Basically the bare minimum to
say "here is something that will output to an already set up serial
device" without any reference to what it actually is. Something like
that would theoretically work with any device that implements or
emulates a UART write register. The thinking is that if it is trivial
to parse then it becomes more useful as an early printk target and
usable by the zImage wrapper (arm32 bit of course). Neither of us ever
followed up on it though.

g.

[PATCH 0/2] Early printk support for virtio console devices.

[Pranavkumar Sawargaonkar]

Hi Rusty,

On 6 May 2013 10:41, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> On 1 May 2013 03:07, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>> An emergency output is a reasonable idea, and this is a reasonable
>>> implementation.  The question is practical: will it be used?  Because we
>>> don't implement reasonable ideas which aren't going to be used.
>>
>> If you think it fits reasonably into the virtio spec (ie doesn't
>> implement things at the wrong level of the transport/backend
>> abstraction) then we can implement it in QEMU, and I think it
>> makes more sense to do this than to throw in a random extra
>> serial port.
>>
>> To be actually useful we need to also specify something in
>> the device tree to say "here is where you will find your
>> emergency output and what it is".
>
> Hmm, I'm not sure that's true.  It looks like it needs:
>
> 1) An enhancment to the vdev->set_config callback to pass through (at
>    least) an offset, probably offset and length.
>
> 2) An emerg_write() function ptr which can be called at any time, set by
>    virtio_console.c's class_init:
>
> static void emerg_write(VirtIOSerialPort *port, char c)
> {
>     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
>
>     if (vcon->chr)
>         qemu_chr_fe_write(vcon->chr, &c, 1);
> }
>
> 3) A routine to find an emerg-write-capable console in
>    virtio_serial_bus.c (or just assume port 0?):
>
> static VirtIOSerialPort *find_emerg_write_port(VirtIOSerial *vser)
> {
>     VirtIOSerialPort *port;
>
>     QTAILQ_FOREACH(port, &vser->ports, next) {
>         VirtIOSerialPortClass *vsc;
>         vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
>
>         if (vsc->emerg_write) {
>             return port;
>         }
>     }
>     return NULL;
> }
>
> 4) set_config in virtio_serial_bus.c to notice emergency writes:
>
>     if (offset == offsetof(struct virtio_console_config, emerg_w) {
>         VirtIOSerial *vser;
>         vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
>         VirtIOSerialPort *port;
>
>         port = find_emerg_write_port(vser);
>         if (port) {
>             vsc->emerg_write(port, config.emerg_w);
>         }
>     }
>
> Amit might have more clue... Amit?
>
> Thanks,
> Rusty.
>
> From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Subject: virtio: console: Add early writeonly register to config space
>
> This patch adds a emerg_wr register (writeonly) in config space of virtio console device which can be used for debugging.
>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> ---
>  include/uapi/linux/virtio_console.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_console.h b/include/uapi/linux/virtio_console.h
> index ee13ab6..586678d 100644
> --- a/include/uapi/linux/virtio_console.h
> +++ b/include/uapi/linux/virtio_console.h
> @@ -38,6 +38,7 @@
>  /* Feature bits */
>  #define VIRTIO_CONSOLE_F_SIZE  0       /* Does host provide console size? */
>  #define VIRTIO_CONSOLE_F_MULTIPORT 1   /* Does host provide multiple ports? */
> +#define VIRTIO_CONSOLE_F_EMERG_WRITE 2 /* Does host support emergency write? */
>
>  #define VIRTIO_CONSOLE_BAD_ID          (~(u32)0)
>
> @@ -48,6 +49,8 @@ struct virtio_console_config {
>         __u16 rows;
>         /* max. number of ports this device can hold */
>         __u32 max_nr_ports;
> +       /* emergency write register */
> +       __u32 emerg_wr;
>  } __attribute__((packed));
>
>  /*

I will send above patch with V2 tagged with the addition of
documentation change and also separate arm64 early printk
implementation for the same.

Thanks,
Pranav

[PATCH 0/2] Early printk support for virtio console devices.

[Peter Maydell]

On 29 April 2013 13:22, Alexander Graf <agraf@suse.de> wrote:
> Oh, and it should default to off.

That would be a pretty unhelpful default. We should default
things so that console output from the kernel is available
as early as reasonably possible by default IMHO -- this
reduces the number of "nothing happens" user complaints
where you then have to explain how to enable early printk
so you can get some actual information about what happened.

-- PMM

[PATCH 0/2] Early printk support for virtio console devices.

[Alexander Graf]

On 29.04.2013, at 14:50, Peter Maydell wrote:

> On 29 April 2013 13:22, Alexander Graf <agraf@suse.de> wrote:
>> Oh, and it should default to off.
> 
> That would be a pretty unhelpful default. We should default
> things so that console output from the kernel is available
> as early as reasonably possible by default IMHO -- this
> reduces the number of "nothing happens" user complaints
> where you then have to explain how to enable early printk
> so you can get some actual information about what happened.

Users complaining that they only see half of their kernel log are even worse IMHO :)


Alex

[PATCH 0/2] Early printk support for virtio console devices.

[Rusty Russell]

Alexander Graf <agraf@suse.de> writes:
> Am 29.04.2013 um 05:09 schrieb Rusty Russell <rusty@rustcorp.com.au>:
>
>> Alexander Graf <agraf@suse.de> writes:
>>> On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:
>>> 
>>>> This patch-set implements early printk support for virtio console devices without using any hypercalls.
>>>> 
>>>> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
>>>> 
>>>> This implementation adds:
>>>> 1. Early writeonly register named early_wr in virtio console's config space.
>>>> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
>>>> 
>>>> Early write mechanism:
>>>> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.
>>> 
>>> I won't nack this patch set, but I'll definitely express that I'm not happy with it.
>>> 
>>> MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.
>>> 
>>> I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.
>> 
>> Well, this shouldn't be mmio-specific, but I kind of get what you mean.
>> 
>> I consider this misnamed: it's an emergency write facility.  Linux may
>> use it for an early console,
>
> If Linux uses it for early console, you won't see any messages from before the virtio-console driver is initialized, because Linux thinks that it's all been printed out.

If you can't support it, don't offer the feature.

>> but it's also useful for bringup and to
>> give a method of emitting errors like "the console ring is corrupt".
>> 
>> A valid implementation may well be to only offer it with some magic
>> qemu developer-only commandline and dump it to stdout.
>
> Why implement it differently from other machines? There are facilities to call into firmware, so you could use that. There's the special Foundation model call that you could implement and reuse for this.

Sure, for ARM.  We *have* a console device.  It's the logical place to
provide a simple write mechanism.  eg. consider bhyve on FreeBSD.

> I don't see why anything like this has to live in virtio-mmio. Oh, and it should default to off.

virtio-console, not virtio-mmio.

Cheers,
Rusty.

[PATCH 0/2] Early printk support for virtio console devices.

[Alexander Graf]

On 30.04.2013, at 02:32, Rusty Russell wrote:

> Alexander Graf <agraf@suse.de> writes:
>> Am 29.04.2013 um 05:09 schrieb Rusty Russell <rusty@rustcorp.com.au>:
>> 
>>> Alexander Graf <agraf@suse.de> writes:
>>>> On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:
>>>> 
>>>>> This patch-set implements early printk support for virtio console devices without using any hypercalls.
>>>>> 
>>>>> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
>>>>> 
>>>>> This implementation adds:
>>>>> 1. Early writeonly register named early_wr in virtio console's config space.
>>>>> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
>>>>> 
>>>>> Early write mechanism:
>>>>> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.
>>>> 
>>>> I won't nack this patch set, but I'll definitely express that I'm not happy with it.
>>>> 
>>>> MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.
>>>> 
>>>> I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.
>>> 
>>> Well, this shouldn't be mmio-specific, but I kind of get what you mean.
>>> 
>>> I consider this misnamed: it's an emergency write facility.  Linux may
>>> use it for an early console,
>> 
>> If Linux uses it for early console, you won't see any messages from before the virtio-console driver is initialized, because Linux thinks that it's all been printed out.
> 
> If you can't support it, don't offer the feature.

Fair enough.

> 
>>> but it's also useful for bringup and to
>>> give a method of emitting errors like "the console ring is corrupt".
>>> 
>>> A valid implementation may well be to only offer it with some magic
>>> qemu developer-only commandline and dump it to stdout.
>> 
>> Why implement it differently from other machines? There are facilities to call into firmware, so you could use that. There's the special Foundation model call that you could implement and reuse for this.
> 
> Sure, for ARM.  We *have* a console device.  It's the logical place to
> provide a simple write mechanism.  eg. consider bhyve on FreeBSD.

I don't quite see the point. The reason early printk works so well for x86 is that you have a UART at a predefined place.

> 
>> I don't see why anything like this has to live in virtio-mmio. Oh, and it should default to off.
> 
> virtio-console, not virtio-mmio.

How will it live in virtio-console? Virtio-console speaks virtio, not register values. If you put this into the config space you break the s390 virtio model.


Alex

[PATCH 0/2] Early printk support for virtio console devices.

[Anup Patel]

On Wed, May 1, 2013 at 5:56 AM, Alexander Graf <agraf@suse.de> wrote:
>
> On 30.04.2013, at 02:32, Rusty Russell wrote:
>
>> Alexander Graf <agraf@suse.de> writes:
>>> Am 29.04.2013 um 05:09 schrieb Rusty Russell <rusty@rustcorp.com.au>:
>>>
>>>> Alexander Graf <agraf@suse.de> writes:
>>>>> On 26.04.2013, at 13:04, Pranavkumar Sawargaonkar wrote:
>>>>>
>>>>>> This patch-set implements early printk support for virtio console devices without using any hypercalls.
>>>>>>
>>>>>> The current virtio early printk code in kernel expects that hypervisor will provide some mechanism generally a hypercall to support early printk. This patch-set does not break existing hypercall based early print support.
>>>>>>
>>>>>> This implementation adds:
>>>>>> 1. Early writeonly register named early_wr in virtio console's config space.
>>>>>> 2. Host feature flags namely VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-write capability in console device.
>>>>>>
>>>>>> Early write mechanism:
>>>>>> 1. When a guest wants to out some character, it has to simply write the character to early_wr register in config space of virtio console device.
>>>>>
>>>>> I won't nack this patch set, but I'll definitely express that I'm not happy with it.
>>>>>
>>>>> MMIO registers are handled by a different layer than the virtio console itself. After the virtio refactoring in QEMU, they will be completely separate drivers. So we'll be in a similar mess with early printk as we are on the s390-virtio machine, where early printk is done through hypercalls and thus we can't directly link it to the console output.
>>>>>
>>>>> I still don't see what the issue is with just implementing a small irq-less virtio driver for early printk.
>>>>
>>>> Well, this shouldn't be mmio-specific, but I kind of get what you mean.
>>>>
>>>> I consider this misnamed: it's an emergency write facility.  Linux may
>>>> use it for an early console,
>>>
>>> If Linux uses it for early console, you won't see any messages from before the virtio-console driver is initialized, because Linux thinks that it's all been printed out.
>>
>> If you can't support it, don't offer the feature.
>
> Fair enough.
>
>>
>>>> but it's also useful for bringup and to
>>>> give a method of emitting errors like "the console ring is corrupt".
>>>>
>>>> A valid implementation may well be to only offer it with some magic
>>>> qemu developer-only commandline and dump it to stdout.
>>>
>>> Why implement it differently from other machines? There are facilities to call into firmware, so you could use that. There's the special Foundation model call that you could implement and reuse for this.
>>
>> Sure, for ARM.  We *have* a console device.  It's the logical place to
>> provide a simple write mechanism.  eg. consider bhyve on FreeBSD.
>
> I don't quite see the point. The reason early printk works so well for x86 is that you have a UART at a predefined place.
>
>>
>>> I don't see why anything like this has to live in virtio-mmio. Oh, and it should default to off.
>>
>> virtio-console, not virtio-mmio.
>
> How will it live in virtio-console? Virtio-console speaks virtio, not register values. If you put this into the config space you break the s390 virtio model.

This is a small limitation of QEMU VirtIO implementation and it can be
easily fixed in QEMU VirtIO.

The VirtIO spec does not tell that VirtIO device specific config
registers are to be emulated by VirtIO transport device (MMIO or PCI).

As per VirtIO spec, the only register that should be emulated by
transport device are the generic VirtIO registers and the VirtIO
device specific config registers should be emulated by VirtIO device
backend.

>
>
> Alex
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--Anup