diff mbox

[RFC] arm64: Early printk support for virtio-mmio console devices.

Message ID 1366264344-28025-1-git-send-email-pranavkumar@linaro.org
State New
Headers show

Commit Message

PranavkumarSawargaonkar April 18, 2013, 5:52 a.m. UTC 
From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>

This patch implements early printk support for virtio-mmio 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 does not break existing hypercall based early print support.

This implementation adds:
1. Early read-write register named early_rw in virtio console's config space.
2. Two host feature flags namely VIRTIO_CONSOLE_F_EARLY_READ and VIRTIO_CONSOLE_F_EARLY_WRITE for telling guest about early-read and 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_rw register in config space of virtio console device.

Early read mechanism:
1. When a guest wants to in some character, it has to simply read the early_rw register in config space of virtio console device. Lets say we get 32-bit value X.
2. If most significant bit of X is set (i.e. X & 0x80000000 == 0x80000000) then least significant 8 bits of X represents input charaacter else guest need to try again reading early_rw register.

Note: This patch only includes kernel side changes for early printk, the host/hypervisor side emulation of early_rw register is out of scope here.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
---
 arch/arm64/kernel/early_printk.c    |   24 ++++++++++++++++++++++++
 include/uapi/linux/virtio_console.h |    4 ++++
 2 files changed, 28 insertions(+)

Comments

Rusty Russell April 18, 2013, 6:51 a.m. UTC | #1 
PranavkumarSawargaonkar <pranavkumar@linaro.org> writes:
> From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>
> This patch implements early printk support for virtio-mmio console devices without using any hypercalls.

This makes some sense, though not sure that early console *read* makes
much sense.  I can see the PCI version of this being useful as well.

The spec definition for this will be interesting, because it can be used
before the device is initialized (something which is generally
verboten).

Cheers,
Rusty.
PranavkumarSawargaonkar April 18, 2013, 7:32 a.m. UTC | #2 
On 18 April 2013 12:21, Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> PranavkumarSawargaonkar <pranavkumar@linaro.org> writes:
> > From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> >
> > This patch implements early printk support for virtio-mmio console devices without using any hypercalls.
>
> This makes some sense, though not sure that early console *read* makes
> much sense.  I can see the PCI version of this being useful as well.

Read can be useful for "mach-virt" which will have only virtio console
as a console device. Then if someone wants to have UEFI or any other
boot-loader emulation, which expects user to input few things, in that
case read might become handy.
>
>
> The spec definition for this will be interesting, because it can be used
> before the device is initialized (something which is generally
> verboten).
>
> Cheers,
> Rusty.

Thanks,
Pranav
Rusty Russell April 22, 2013, 1:21 a.m. UTC | #3 
Pranavkumar Sawargaonkar <pranavkumar@linaro.org> writes:
> On 18 April 2013 12:21, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>
>> PranavkumarSawargaonkar <pranavkumar@linaro.org> writes:
>> > From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> >
>> > This patch implements early printk support for virtio-mmio console devices without using any hypercalls.
>>
>> This makes some sense, though not sure that early console *read* makes
>> much sense.  I can see the PCI version of this being useful as well.
>
> Read can be useful for "mach-virt" which will have only virtio console
> as a console device. Then if someone wants to have UEFI or any other
> boot-loader emulation, which expects user to input few things, in that
> case read might become handy.

But implementing virtio inside a bootloader has already been done for
coreboot, for example.  A bootloader probably wants a virtio block
device, so a console is trivial.

A single writable field for debugging makes sense.  Anything more is far
less certain.

Thanks,
Rusty.
Anup Patel April 22, 2013, 3:10 a.m. UTC | #4 
On 22 April 2013 06:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Pranavkumar Sawargaonkar <pranavkumar@linaro.org> writes:
> > On 18 April 2013 12:21, Rusty Russell <rusty@rustcorp.com.au> wrote:
> >>
> >> PranavkumarSawargaonkar <pranavkumar@linaro.org> writes:
> >> > From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> >> >
> >> > This patch implements early printk support for virtio-mmio console
> >> > devices without using any hypercalls.
> >>
> >> This makes some sense, though not sure that early console *read* makes
> >> much sense.  I can see the PCI version of this being useful as well.
> >
> > Read can be useful for "mach-virt" which will have only virtio console
> > as a console device. Then if someone wants to have UEFI or any other
> > boot-loader emulation, which expects user to input few things, in that
> > case read might become handy.
>
> But implementing virtio inside a bootloader has already been done for
> coreboot, for example.  A bootloader probably wants a virtio block
> device, so a console is trivial.
>
> A single writable field for debugging makes sense.  Anything more is far
> less certain.

The early read can be handy for bootloader who don't want to implement
complete VirtIO programming.

IMHO, early read would be totally optional for host and will not
introduce any new config register so it is good to have in VirtIO
console spec. Also, without early read the read behavior of early_rw
field would be undefined in VirtIO console spec.

>
> Thanks,
> Rusty.

Best Regards,
Anup
Rusty Russell April 22, 2013, 5:15 a.m. UTC | #5 
Anup Patel <anup.patel@linaro.org> writes:
> On 22 April 2013 06:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>
>> Pranavkumar Sawargaonkar <pranavkumar@linaro.org> writes:
>> > On 18 April 2013 12:21, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> >>
>> >> PranavkumarSawargaonkar <pranavkumar@linaro.org> writes:
>> >> > From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> >> >
>> >> > This patch implements early printk support for virtio-mmio console
>> >> > devices without using any hypercalls.
>> >>
>> >> This makes some sense, though not sure that early console *read* makes
>> >> much sense.  I can see the PCI version of this being useful as well.
>> >
>> > Read can be useful for "mach-virt" which will have only virtio console
>> > as a console device. Then if someone wants to have UEFI or any other
>> > boot-loader emulation, which expects user to input few things, in that
>> > case read might become handy.
>>
>> But implementing virtio inside a bootloader has already been done for
>> coreboot, for example.  A bootloader probably wants a virtio block
>> device, so a console is trivial.
>>
>> A single writable field for debugging makes sense.  Anything more is far
>> less certain.
>
> The early read can be handy for bootloader who don't want to implement
> complete VirtIO programming.

I've said this twice already.  This is the last time.

1) We do not add complexity unless we have to.
2) Making it optional does not make it significantly less complex.
3) Having two ways of doing the same thing is always ugly.
4) Having an emergency output method makes sense for several use cases,
   an emergency input method does not.
5) A bootloader which uses virtio to get the images to boot can
   implement a console in less than 10 lines.
6) A bootloader which does not use any virtio devices but nonetheless
   wants to obtain input can implement a simple console in well under 100
   lines.  See below.

Finally, in case you still don't understand:

My task is to make this decision, and I've made it.  Arguing with me is
only useful if you bring new facts which you can change my mind.

Hope that clarifies,
Rusty.

#define VIRTIO_MMIO_GUEST_PAGE_SIZE	0x028
#define VIRTIO_MMIO_QUEUE_SEL		0x030
#define VIRTIO_MMIO_QUEUE_NUM_MAX	0x034
#define VIRTIO_MMIO_QUEUE_NUM		0x038
#define VIRTIO_MMIO_QUEUE_ALIGN		0x03c
#define VIRTIO_MMIO_QUEUE_PFN		0x040
#define VIRTIO_MMIO_QUEUE_NOTIFY	0x050

struct vring_desc {
	__u64 addr;
	__u32 len;
	__u16 flags;
	__u16 next;
};

struct vring_used_elem {
	__u32 id;
	__u32 len;
};

struct vconsole_ring {
        struct vring_desc desc[2];
	__u16 avail_flags;
	__u16 avail_idx;
	__u16 available[2];
	__u16 used_event_idx;
	__u16 pad; /* Hit 4-byte boundary */

	// A ring of used descriptor heads with free-running index.
	__u16 used_flags;
	__u16 used_idx;
	struct vring_used_elem used[2];
	__u16 avail_event_idx;
};

static char console_in;
static struct vconsole_ring vc_ring = {
        { { PHYSICAL_ADDR(console_in), 1, 2, 0 } },
        1, /* No interrupts */
}

static void post_buffer(void *base)
{
        vc_ring->avail_idx++;
        wmb();
        writel(0, base + VIRTIO_MMIO_QUEUE_NOTIFY);
}

bool vc_read(void *base, char *c)
{
        mb();
        if (vc_ring->used_idx != vc_ring->avail_idx)
                return false;
        *c = console_in;
        post_buffer(base);
        return true;
}

void vc_init(void *base)
{
        /* Alignment of 4 bytes, don't waste any. */
	writel(4, base + VIRTIO_MMIO_GUEST_PAGE_SIZE);

        /* Setup incoming vq. */
	writel(0, base + VIRTIO_MMIO_QUEUE_SEL);

        /* 2 elements */
	writel(2, base + VIRTIO_MMIO_QUEUE_NUM);
        /* Alignment of 4 bytes */
	writel(4, base + VIRTIO_MMIO_QUEUE_ALIGN);
        /* Location of queue. */
	writel(PHYSICAL_ADDR(&vc_ring) >> 4, base + VIRTIO_MMIO_QUEUE_PFN);

        post_buffer(base);
}
PranavkumarSawargaonkar April 23, 2013, 5:53 a.m. UTC | #6 
On 22 April 2013 10:45, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Anup Patel <anup.patel@linaro.org> writes:
>> On 22 April 2013 06:51, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>>
>>> Pranavkumar Sawargaonkar <pranavkumar@linaro.org> writes:
>>> > On 18 April 2013 12:21, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>> >>
>>> >> PranavkumarSawargaonkar <pranavkumar@linaro.org> writes:
>>> >> > From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>> >> >
>>> >> > This patch implements early printk support for virtio-mmio console
>>> >> > devices without using any hypercalls.
>>> >>
>>> >> This makes some sense, though not sure that early console *read* makes
>>> >> much sense.  I can see the PCI version of this being useful as well.
>>> >
>>> > Read can be useful for "mach-virt" which will have only virtio console
>>> > as a console device. Then if someone wants to have UEFI or any other
>>> > boot-loader emulation, which expects user to input few things, in that
>>> > case read might become handy.
>>>
>>> But implementing virtio inside a bootloader has already been done for
>>> coreboot, for example.  A bootloader probably wants a virtio block
>>> device, so a console is trivial.
>>>
>>> A single writable field for debugging makes sense.  Anything more is far
>>> less certain.
>>
>> The early read can be handy for bootloader who don't want to implement
>> complete VirtIO programming.
>
> I've said this twice already.  This is the last time.
>
> 1) We do not add complexity unless we have to.
> 2) Making it optional does not make it significantly less complex.
> 3) Having two ways of doing the same thing is always ugly.
> 4) Having an emergency output method makes sense for several use cases,
>    an emergency input method does not.
Okay,  i will restructure my patch by keeping just output write part
and post it back.

Thanks,
Pranav

> 5) A bootloader which uses virtio to get the images to boot can
>    implement a console in less than 10 lines.
> 6) A bootloader which does not use any virtio devices but nonetheless
>    wants to obtain input can implement a simple console in well under 100
>    lines.  See below.
>
> Finally, in case you still don't understand:
>
> My task is to make this decision, and I've made it.  Arguing with me is
> only useful if you bring new facts which you can change my mind.
>
> Hope that clarifies,
> Rusty.
>
> #define VIRTIO_MMIO_GUEST_PAGE_SIZE     0x028
> #define VIRTIO_MMIO_QUEUE_SEL           0x030
> #define VIRTIO_MMIO_QUEUE_NUM_MAX       0x034
> #define VIRTIO_MMIO_QUEUE_NUM           0x038
> #define VIRTIO_MMIO_QUEUE_ALIGN         0x03c
> #define VIRTIO_MMIO_QUEUE_PFN           0x040
> #define VIRTIO_MMIO_QUEUE_NOTIFY        0x050
>
> struct vring_desc {
>         __u64 addr;
>         __u32 len;
>         __u16 flags;
>         __u16 next;
> };
>
> struct vring_used_elem {
>         __u32 id;
>         __u32 len;
> };
>
> struct vconsole_ring {
>         struct vring_desc desc[2];
>         __u16 avail_flags;
>         __u16 avail_idx;
>         __u16 available[2];
>         __u16 used_event_idx;
>         __u16 pad; /* Hit 4-byte boundary */
>
>         // A ring of used descriptor heads with free-running index.
>         __u16 used_flags;
>         __u16 used_idx;
>         struct vring_used_elem used[2];
>         __u16 avail_event_idx;
> };
>
> static char console_in;
> static struct vconsole_ring vc_ring = {
>         { { PHYSICAL_ADDR(console_in), 1, 2, 0 } },
>         1, /* No interrupts */
> }
>
> static void post_buffer(void *base)
> {
>         vc_ring->avail_idx++;
>         wmb();
>         writel(0, base + VIRTIO_MMIO_QUEUE_NOTIFY);
> }
>
> bool vc_read(void *base, char *c)
> {
>         mb();
>         if (vc_ring->used_idx != vc_ring->avail_idx)
>                 return false;
>         *c = console_in;
>         post_buffer(base);
>         return true;
> }
>
> void vc_init(void *base)
> {
>         /* Alignment of 4 bytes, don't waste any. */
>         writel(4, base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
>
>         /* Setup incoming vq. */
>         writel(0, base + VIRTIO_MMIO_QUEUE_SEL);
>
>         /* 2 elements */
>         writel(2, base + VIRTIO_MMIO_QUEUE_NUM);
>         /* Alignment of 4 bytes */
>         writel(4, base + VIRTIO_MMIO_QUEUE_ALIGN);
>         /* Location of queue. */
>         writel(PHYSICAL_ADDR(&vc_ring) >> 4, base + VIRTIO_MMIO_QUEUE_PFN);
>
>         post_buffer(base);
> }
diff mbox

Patch

diff --git a/arch/arm64/kernel/early_printk.c b/arch/arm64/kernel/early_printk.c
index ac974f4..a82b5aa 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,26 @@  static void smh_printch(char ch)
 }
 
 /*
+ * VIRTIO MMIO based debug console.
+ */
+static void virtio_console_early_printch(char ch)
+{
+	u32 tmp;
+	struct virtio_console_config *p = early_base + VIRTIO_MMIO_CONFIG;
+
+	tmp = readl_relaxed(early_base + VIRTIO_MMIO_DEVICE_ID);
+	if (tmp != VIRTIO_ID_CONSOLE) {
+		return;
+	}
+
+	tmp = readl_relaxed(early_base + VIRTIO_MMIO_HOST_FEATURES);
+	if (!(tmp & (1 << VIRTIO_CONSOLE_F_EARLY_WRITE))) {
+		return;
+	}
+	writeb_relaxed(ch, &p->early_rw);
+}
+
+/*
  * 8250/16550 (8-bit aligned registers) single character TX.
  */
 static void uart8250_8bit_printch(char ch)
@@ -82,6 +105,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_early_printch, },
 	{}
 };
 
diff --git a/include/uapi/linux/virtio_console.h b/include/uapi/linux/virtio_console.h
index ee13ab6..1171cb4 100644
--- a/include/uapi/linux/virtio_console.h
+++ b/include/uapi/linux/virtio_console.h
@@ -38,6 +38,8 @@ 
 /* 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_READ 2	/* Does host support early read? */
+#define VIRTIO_CONSOLE_F_EARLY_WRITE 3	/* Does host support early write? */
 
 #define VIRTIO_CONSOLE_BAD_ID		(~(u32)0)
 
@@ -48,6 +50,8 @@  struct virtio_console_config {
 	__u16 rows;
 	/* max. number of ports this device can hold */
 	__u32 max_nr_ports;
+	/* early read/write register */
+	__u32 early_rw;
 } __attribute__((packed));
 
 /*