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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAAHg+Hg1u-md_5hSWxyVOOrg_f9=h0tO5iv2WQL-U-MyTMAV9g@mail.gmail.com>
Date:	Tue, 23 Apr 2013 11:23:38 +0530
From:	Pranavkumar Sawargaonkar <pranavkumar@...aro.org>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Anup Patel <anup.patel@...aro.org>,
	"kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linaro-kernel@...ts.linaro.org" <linaro-kernel@...ts.linaro.org>,
	Patch Tracking <patches@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] arm64: Early printk support for virtio-mmio console devices.

On 22 April 2013 10:45, Rusty Russell <rusty@...tcorp.com.au> wrote:
> Anup Patel <anup.patel@...aro.org> writes:
>> On 22 April 2013 06:51, Rusty Russell <rusty@...tcorp.com.au> wrote:
>>>
>>> Pranavkumar Sawargaonkar <pranavkumar@...aro.org> writes:
>>> > On 18 April 2013 12:21, Rusty Russell <rusty@...tcorp.com.au> wrote:
>>> >>
>>> >> PranavkumarSawargaonkar <pranavkumar@...aro.org> writes:
>>> >> > From: Pranavkumar Sawargaonkar <pranavkumar@...aro.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);
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/