[Qemu-devel] virtio-rng

[Qemu-devel] virtio-rng

[Ian Molton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi folks,

I'm writing a virtio-rng host-side driver for qemu-kvm, and I've got
something up and running that works, and will pass data gathered from a
char device on the host through to the virtio-rng driver on a guest copy
of linux.

Ultimately it'll get its entropy from egd as well, but for now I'd like
to solve the issue in hw/qdev.c.

in qdev_init_chardev() the return value is picked based upon the name of
the device. For now, I've added a third 'if clause' to match for my
driver and pass through the CharDriverState * fron vl.c for my rng
driver, however I'd like to solve this properly.

I think a simple name->pointer type matching system would work fine,
however I'd like to know if anyone else has sorted this yet, or if I
should be doing things differently altogether.

also, what does '_hds' stand for? eg. 'virtcon_hds'[]

Thanks,

- -Ian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJK+y1LAAoJEFIjE1w7L6YHlI4P/i4QPbsIASmCLtI9RiEtF7LF
NguXv+HCNme8MK6bQpHKLYIJ1q/DSImeeJPjqe/PF1unOBP0d4GmsC+qnLGj4CZn
LCRiqf+ZgVPI/fX8TQ8TOzN9o31EOnWmxOKuj+zDDhbRxG/mY2OzOQCLEBkQ26GL
Pru981FBVJiEPP2JnWnR6Hn+IZaazcyo+ychk0UhQYasmv8XTC75DiiProhyXFJk
TXa8UN+lWd1U1a0mmmg1gxttQrYZiC5i4+k609HY8v3poZ0kjmlH0hIUHt+3FHE8
pA1WL/B26EhNem3ZazGGJ8sNdGg60VTjEBSHyGveCdUtiQU2TWWESBOesQoALkoE
8mONeytgsv7GZ9sh0a1RTInvSsA2TyV/hzQphOtTl6SNEETlGEsFNJpLk+6GeO89
g5A3vdgsOL18ymWxDTf/Ts14mK4+QMa9x4T2Cpttip1LuOwDoV/PCKfpg1q9TT+d
C04TFR7qgl6eAsErHhHlFYmWNcMHn00t9jOEBUTMjIi5xKwsCr+ktWxF0yxLQdlu
co7gLZd4ck4N8GMltPjIXGMnqWLsemf4b7Gpg8wtIjmK5f7l58qVZW0Qsdx9ujJc
MLxQwKvk5OdE+uGMBUDynAGW926Fu/QhdrAhWSbzDFrIjysTY6fzXgNaQa+8HoL9
5fB9UiotqwfYy95HnZV9
=ABHJ
-----END PGP SIGNATURE-----

Re: [Qemu-devel] virtio-rng

[Paul Brook]

> I'm writing a virtio-rng host-side driver for qemu-kvm, and I've got
> something up and running that works, and will pass data gathered from a
> char device on the host through to the virtio-rng driver on a guest copy
> of linux.

Why do you need a special device? Isn't a regular serial data stream (i.e. 
multiport virtio-console) sufficient? You can then connect that to your source 
of random data (e.g. /dev/random) without requiring any changes to qemu.

Paul

Re: [Qemu-devel] virtio-rng

[Gerd Hoffmann]

   Hi,

> in qdev_init_chardev() the return value is picked based upon the name of
> the device. For now, I've added a third 'if clause' to match for my
> driver and pass through the CharDriverState * fron vl.c for my rng
> driver, however I'd like to solve this properly.

Ignore qdev_init_chardev() ...

> I think a simple name->pointer type matching system would work fine,
> however I'd like to know if anyone else has sorted this yet, or if I
> should be doing things differently altogether.

Use a chardev property (look at serial.c, "isa-serial" device).

Then you'll configure it on the qemu command line like this:

   -chardev $backend,id=some-name-here,more-chardev-parameters
   -device virtio-rng,chardev=some-name-here

HTH,
   Gerd

Re: [Qemu-devel] virtio-rng

[Ian Molton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Gerd Hoffmann wrote:
>   Hi,

Hi!

Thanks for your reply - I should have posted to say I'd partially solved
this. I have a question though,

> Use a chardev property (look at serial.c, "isa-serial" device).
> 
> Then you'll configure it on the qemu command line like this:
> 
>   -chardev $backend,id=some-name-here,more-chardev-parameters
>   -device virtio-rng,chardev=some-name-here

I've done something similar (below)

My commandline looks like:

- -virtiorng dev=/dev/foo,rate=1234

I added some properties to my driver which are obviously then filled in
from the options code, and I do my init like this:

VirtIODevice *virtio_rng_init(DeviceState *dev, RNGConf *rngdev)
{
    VirtIORng *s;
    s = (VirtIORng *)virtio_common_init("virtio-rng",
                                            VIRTIO_ID_RNG,
                                            0, sizeof(VirtIORng));

    if (!s)
        return NULL;

    s->vdev.get_features = virtio_rng_get_features;

    s->vq = virtio_add_queue(&s->vdev, 128, virtio_rng_handle);
    s->chr = qemu_chr_open("vrngdev", rngdev->device, NULL);

    ...

    return &s->vdev;
}

Is this 'not the right way' ? I think the commandline looks much simpler
this way.

OTOH, I can see that this might help with my next problem, which is that
I want to write EGD support for virtio-rng, so I'm guessing my
commandline would (in your scheme) need to be something like:


    -tcpsocketthing $addr.of.egd.server,id=some-name-here,
     more-egd-client-options
    -device virtio-rng,tcpsocketthing=some-name-here,rate=$limit

I guess the idea here is to seperate the host related config from the
guest OS setup ?

This doesnt, however, get around my problem that the -device option
doesnt know how to parse OPT_SIZE types, though. (I'd like to be able to
specify 'rate' in B/s or KB/s (or even kbits/sec, more appropriately))

- -Ian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJLAUWCAAoJEFIjE1w7L6YHi7YP/1JXi1T1T3/GUG8o+ZTXJbV5
KUCkwZct7bAqMaZyxsbkbqo947tNjL006kupvUwRsfS/Hrp2uf9K/+o0tTGr87Kl
ec7sWAU67q1ydhFQQYcAnUeBEe2qO9qIXC2d32QOPOTBftwmmCYMSRob+VG0jOvy
eBN9ZdnUM+vGWmFfAxH8U1Wcx4sxEni+QZ36cFSCwAVfimxqkj9h2sI4yYVmIwWI
3VN6BERQgBGMp81fRY+QiVFGSXEPfLi/ppI8Ta6MOO3dB91XzsqDcMNIHIJwmxFz
S2uSMl9py3P5jGb2hhHHO56Fvl8paBOEvVm1SsOMXID9MpnrSHJZmlnKAe+P0nY2
EX+pvvqJZwA2OS15fjzypmho0WZtvS/HdrvEp9SQcM6dgfyVapR6H/GLrqV1iEZ5
uarKBbFKuWVCl8rWc4dZm6sMDfIuzjUgeMZyEhUx3dHVpzzd1B0zIZOVFyI7Fde1
+ox+lvm4EXBV/hji4GCc3E1pJSlmQtokSSTuaVrAXrwJRJcZpX3cjsQad5S5OOrI
zXYgs2H/ITINxnVjxkPTHty1txcZrs2Wvjbs8LvkE7LsZuquy+maVN2VS7ahwy3q
qCAkfm8Yp1QVmPM1sAlhuMPCrtQKl4VAt825PbpjEd4DNHZb1hnK8s7dc3NpVKcv
NK+vr3fUldUlsU+6Om0r
=yNv7
-----END PGP SIGNATURE-----

Re: [Qemu-devel] virtio-rng

[Gerd Hoffmann]

On 11/16/09 13:28, Ian Molton wrote:

> I've done something similar (below)
>
> My commandline looks like:
>
> - -virtiorng dev=/dev/foo,rate=1234
>
> I added some properties to my driver which are obviously then filled in
> from the options code, and I do my init like this:

There is no need for a new -virtiorng switch.  qdev allows to set any 
property when creating devices via -device on the command line.

> VirtIODevice *virtio_rng_init(DeviceState *dev, RNGConf *rngdev)
> {
>      VirtIORng *s;
>      s = (VirtIORng *)virtio_common_init("virtio-rng",
>                                              VIRTIO_ID_RNG,
>                                              0, sizeof(VirtIORng));
>
>      if (!s)
>          return NULL;
>
>      s->vdev.get_features = virtio_rng_get_features;
>
>      s->vq = virtio_add_queue(&s->vdev, 128, virtio_rng_handle);
>      s->chr = qemu_chr_open("vrngdev", rngdev->device, NULL);
>
>      ...
>
>      return&s->vdev;
> }
>
> Is this 'not the right way' ? I think the commandline looks much simpler
> this way.

The point is to separate host and guest state.  -chardev ist host state, 
the device is guest state, and they are linked using the chardev name.

> OTOH, I can see that this might help with my next problem, which is that
> I want to write EGD support for virtio-rng, so I'm guessing my
> commandline would (in your scheme) need to be something like:
>
>
>      -tcpsocketthing $addr.of.egd.server,id=some-name-here,
>       more-egd-client-options
>      -device virtio-rng,tcpsocketthing=some-name-here,rate=$limit

Maybe ...

    -chardev socket,id=egd,host=egd.domain.tld,port=whatever
    -device virtio-rng,chardev=egd

... ?

> This doesnt, however, get around my problem that the -device option
> doesnt know how to parse OPT_SIZE types, though. (I'd like to be able to
> specify 'rate' in B/s or KB/s (or even kbits/sec, more appropriately))

Yes, there is no qdev property type (yet) which can parse those 
postfixes.  Which is no reason to not add one though ;)

cheers,
   Gerd

Re: [Qemu-devel] virtio-rng

[Ian Molton]

[-- Attachment #1: Type: text/plain, Size: 1804 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Gerd Hoffmann wrote:

> Maybe ...
> 
>    -chardev socket,id=egd,host=egd.domain.tld,port=whatever
>    -device virtio-rng,chardev=egd

I've had a go at modifying virtio-console.c to use these semantics,
attached below. I'd appreciate it if you could let me know if this is
'the right way'.

>> This doesnt, however, get around my problem that the -device option
>> doesnt know how to parse OPT_SIZE types, though. (I'd like to be able to
>> specify 'rate' in B/s or KB/s (or even kbits/sec, more appropriately))
> 
> Yes, there is no qdev property type (yet) which can parse those
> postfixes.  Which is no reason to not add one though ;)

I'll look into it ;-)

Heres my patch to virtio-console. The device is now specified like this:

- -chardev file,path=/path/to/testfile,id=test
- -device virtio-console-pci,chardev=test

(for example)

TTFN!

- -Ian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJLAZK2AAoJEFIjE1w7L6YHS0UP/0v/0a42Mon+SsUfowv7EpKw
0clLC/JCTcNFzSD1PqFdYV5E2kqCeMHHRS2DsCjjGFxgC5uOD7tuBLp66dk2vxvh
0QnIXkVD9FRBfRS5E2VtjterYmp2Fu+EBE3ugtAiKZhtQWVtmiQfRVqrqClpz8hd
4l/RXxTNkjJE5yRm9J7tBZ9lZqJ5tKTxqQtQV2+pPghOX7zAu4tsv3kLk9LY6SZr
dfUfv1iXiNwiZh3Z7pKEiezzJKtoPS5y30wIszPziv2Ef/V153hxED35nT6y1+1i
16wknBJgFUG7Hp0GrprUp6N5pRTX2f7X07IXhPdMyX79J9RtS8Vg7+5aX6nsK5m0
8B0WvXZqn6i+DHGrcXNPw0IpkP/MDCTVSn15O9TsiyKLnkKfjHlQHzhDkVb9yMub
bkd4uE8e9i+vqIY57kqsvcfGytcw4g3bHzT2SasKKyBjGVcwahQT4zDI9PjX7CUg
vxqktTy3y4de/ijrzuIy5e/oCK6b9AAKI0dJJJfH6NsP1ljMldEeJlkLKRheWBy9
2AqdkmQhD3eD5fo5idmp9oxx0okfg/p3v5IdLTUadgIYSL70J61fb0TL6hBNo8TS
uuCoZdgeKXD470tUXWvrWPxIACvG8ZAMYhz3CGXrFGb8WJjt1Rb0DXvHbr2Z8Wdn
2PwPSOliAY0iPfzo79Ke
=TM+C
-----END PGP SIGNATURE-----

[-- Attachment #2: 0001-virtio-console-Remove-ugly-device-reg.-hack.patch --]
[-- Type: text/x-patch, Size: 7688 bytes --]

>From 40a7a43484176194490a7980741a6d4764c10fb1 Mon Sep 17 00:00:00 2001
From: Ian Molton <ian.molton@collabora.co.uk>
Date: Mon, 16 Nov 2009 17:49:18 +0000
Subject: [PATCH] virtio-console: Remove ugly device reg. hack

	This patch removes the unsightly hack in qdev_init_chardev() that
was used in order to pass the host device to virtio-console.

	It does not remove the limit of only one console which is imposed by
the use of the virtio-pci proxy in virtio-pci.c, however if this were fixed,
it would be possible to register numerous consoles without further modification
of the virtio-console qdev.

Signed-off-by: Ian Molton <ian.molton@collabora.co.uk>
---
 hw/pc.c             |    9 ---------
 hw/ppc440_bamboo.c  |    7 -------
 hw/qdev.c           |    9 ++-------
 hw/virtio-console.c |    8 ++++----
 sysemu.h            |    6 ------
 vl.c                |   37 -------------------------------------
 6 files changed, 6 insertions(+), 70 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index fb73a54..1d0c870 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1368,15 +1368,6 @@ static void pc_init1(ram_addr_t ram_size,
 	extboot_init(info->bdrv, 1);
     }
 
-    /* Add virtio console devices */
-    if (pci_enabled) {
-        for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
-            if (virtcon_hds[i]) {
-                pci_create_simple(pci_bus, -1, "virtio-console-pci");
-            }
-        }
-    }
-
 #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
     if (kvm_enabled()) {
         add_assigned_devices(pci_bus, assigned_devices, assigned_devices_index);
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index 25417e3..c94c961 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -109,13 +109,6 @@ static void bamboo_init(ram_addr_t ram_size,
     env = ppc440ep_init(&ram_size, &pcibus, pci_irq_nrs, 1, cpu_model);
 
     if (pcibus) {
-        /* Add virtio console devices */
-        for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
-            if (virtcon_hds[i]) {
-                pci_create_simple(pcibus, -1, "virtio-console-pci");
-            }
-        }
-
         /* Register network interfaces. */
         for (i = 0; i < nb_nics; i++) {
             /* There are no PCI NICs on the Bamboo board, but there are
diff --git a/hw/qdev.c b/hw/qdev.c
index 373ddfc..237da57 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -321,13 +321,8 @@ void qdev_machine_creation_done(void)
 CharDriverState *qdev_init_chardev(DeviceState *dev)
 {
     static int next_serial;
-    static int next_virtconsole;
-    /* FIXME: This is a nasty hack that needs to go away.  */
-    if (strncmp(dev->info->name, "virtio", 6) == 0) {
-        return virtcon_hds[next_virtconsole++];
-    } else {
-        return serial_hds[next_serial++];
-    }
+
+    return serial_hds[next_serial++];
 }
 
 BusState *qdev_get_parent_bus(DeviceState *dev)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index bc3681f..8fe8a5d 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -21,7 +21,7 @@ typedef struct VirtIOConsole
 {
     VirtIODevice vdev;
     VirtQueue *ivq, *ovq;
-    CharDriverState *chr;
+    CharDriverState chr;
 } VirtIOConsole;
 
 static VirtIOConsole *to_virtio_console(VirtIODevice *vdev)
@@ -39,7 +39,7 @@ static void virtio_console_handle_output(VirtIODevice *vdev, VirtQueue *vq)
         int d;
 
         for (d = 0; d < elem.out_num; d++) {
-            len += qemu_chr_write(s->chr, (uint8_t *)elem.out_sg[d].iov_base,
+            len += qemu_chr_write(&s->chr, (uint8_t *)elem.out_sg[d].iov_base,
                                   elem.out_sg[d].iov_len);
         }
         virtqueue_push(vq, &elem, len);
@@ -137,8 +137,8 @@ VirtIODevice *virtio_console_init(DeviceState *dev, CharDriverState *chr)
     s->ivq = virtio_add_queue(&s->vdev, 128, virtio_console_handle_input);
     s->ovq = virtio_add_queue(&s->vdev, 128, virtio_console_handle_output);
 
-    s->chr = qdev_init_chardev(dev);
-    qemu_chr_add_handlers(s->chr, vcon_can_read, vcon_read, vcon_event, s);
+    memcpy(&s->chr, chr, sizeof(*chr));
+    qemu_chr_add_handlers(&s->chr, vcon_can_read, vcon_read, vcon_event, s);
 
     register_savevm("virtio-console", -1, 1, virtio_console_save, virtio_console_load, s);
 
diff --git a/sysemu.h b/sysemu.h
index 8455743..1a494e5 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -229,12 +229,6 @@ extern CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 
 extern CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 
-/* virtio consoles */
-
-#define MAX_VIRTIO_CONSOLES 1
-
-extern CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
-
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
 #ifdef HAS_AUDIO
diff --git a/vl.c b/vl.c
index 60eccee..a26e7ac 100644
--- a/vl.c
+++ b/vl.c
@@ -212,7 +212,6 @@ static int no_frame = 0;
 int no_quit = 0;
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
-CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
 #ifdef TARGET_I386
 int win2k_install_hack = 0;
 int rtc_td_hack = 0;
@@ -4673,8 +4672,6 @@ int main(int argc, char **argv, char **envp)
     int serial_device_index;
     const char *parallel_devices[MAX_PARALLEL_PORTS];
     int parallel_device_index;
-    const char *virtio_consoles[MAX_VIRTIO_CONSOLES];
-    int virtio_console_index;
     const char *loadvm = NULL;
     QEMUMachine *machine;
     const char *cpu_model;
@@ -4750,10 +4747,6 @@ int main(int argc, char **argv, char **envp)
         parallel_devices[i] = NULL;
     parallel_device_index = 0;
 
-    for(i = 0; i < MAX_VIRTIO_CONSOLES; i++)
-        virtio_consoles[i] = NULL;
-    virtio_console_index = 0;
-
     monitor_devices[0] = "vc:80Cx24C";
     for (i = 1; i < MAX_MONITOR_DEVICES; i++) {
         monitor_devices[i] = NULL;
@@ -5216,14 +5209,6 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
-            case QEMU_OPTION_virtiocon:
-                if (virtio_console_index >= MAX_VIRTIO_CONSOLES) {
-                    fprintf(stderr, "qemu: too many virtio consoles\n");
-                    exit(1);
-                }
-                virtio_consoles[virtio_console_index] = optarg;
-                virtio_console_index++;
-                break;
             case QEMU_OPTION_parallel:
                 if (parallel_device_index >= MAX_PARALLEL_PORTS) {
                     fprintf(stderr, "qemu: too many parallel ports\n");
@@ -5776,20 +5761,6 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-    for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
-        const char *devname = virtio_consoles[i];
-        if (devname && strcmp(devname, "none")) {
-            char label[32];
-            snprintf(label, sizeof(label), "virtcon%d", i);
-            virtcon_hds[i] = qemu_chr_open(label, devname, NULL);
-            if (!virtcon_hds[i]) {
-                fprintf(stderr, "qemu: could not open virtio console '%s': %s\n",
-                        devname, strerror(errno));
-                exit(1);
-            }
-        }
-    }
-
     module_call_init(MODULE_INIT_DEVICE);
 
     if (watchdog) {
@@ -5918,14 +5889,6 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-    for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
-        const char *devname = virtio_consoles[i];
-        if (virtcon_hds[i] && devname) {
-            if (strstart(devname, "vc", 0))
-                qemu_chr_printf(virtcon_hds[i], "virtio console%d\r\n", i);
-        }
-    }
-
     if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
         fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
                 gdbstub_dev);
-- 
1.6.5


[-- Attachment #3: 0001-virtio-console-Remove-ugly-device-reg.-hack.patch.sig --]
[-- Type: application/pgp-signature, Size: 543 bytes --]

Re: [Qemu-devel] virtio-rng

[Jamie Lokier]

Ian Molton wrote:
> Heres my patch to virtio-console. The device is now specified like this:
> 
> - -chardev file,path=/path/to/testfile,id=test
> - -device virtio-console-pci,chardev=test

It'd be nice if some options on the qemu command line (or config file)
resulted in the guest kernel getting entropy (assuming a kernel with
the appropriate support), without having to run a special
entropy-injection daemon in the guest.

virtio-console can be used for that, but it'd be necessary to have a
way to tell the guest kernel to treat that particular device as an
entropy source.  Even just a standard name.

-- Jamie

Re: [Qemu-devel] virtio-rng

[Ian Molton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jamie Lokier wrote:
> Ian Molton wrote:
>> Heres my patch to virtio-console. The device is now specified like this:
>>
>> - -chardev file,path=/path/to/testfile,id=test
>> - -device virtio-console-pci,chardev=test

Note, I think the patch above is broken and slipped by me because I
forgot to make clean - I'll doublecheck tomorrow.

> It'd be nice if some options on the qemu command line (or config file)
> resulted in the guest kernel getting entropy (assuming a kernel with
> the appropriate support), without having to run a special
> entropy-injection daemon in the guest.

How, though - AFAIK linux has no drivers specifically intended to feed
entropy into the kernels pool - all the hwrng drivers use a userspace
tool to do so.

I still intend to submit my virtio-rng driver, if thats what you mean,
since it presents the data via the same routes as all the other hw rng
sources. I feel this approach has value.

- -Ian

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJLAd1fAAoJEFIjE1w7L6YHqk4QAKSUMZ9VUJss4YWecDPS52DF
Jc7sbDEDuk6bRziRqY8nYMovOzalXgbcfSR3ElhZ4lAEGAGwDP1Qa9IbLh1vf083
UgLZa6FRSWbf5A72+Jl3vLzFnzraWp7Y3m6/qX4mv+NHyBRrBGJ5RwJ7DCTeW8I0
9YScryKZiwPdH2CW+ZUj1+mdbbX/n28mJzUHByRkUgLmKHEXgjNdSRBgtM8/YTia
yEO2arQC0Oq+t+r/cjy0B5HTg2Qq+mqFIr9gTIIN8xuVmEuX2mNoT5BSJwGoOWYS
+tjTusCXQ/SF1FgGBmNaFddtUTLge1op1gKtGEwsX8QqYmLCo9TBhDzfVZB+U+jM
m5ACEHYFCAuYQWOMsajcO78wy6q5yKFE1MJrP69EGR9wtGHNeXVsRQ0EGTqH+rs2
m66+Y0sqzvRvaYqUHSvZMNUf1FWEapkwDLYfJ87twdL1OIqOA5knBMrmZO+rF6bK
RKXGeNK/sTnvL0OEXkzeeaw8Em7ykeyVJi9rO9J8XIKH6cE/XC1ab7uNyFrSIGS6
ELvhbrzJ/c5vp/mZQOaMYrj+1+2QAv+0DpF4DDpQOuiuXFR4O6vh+Tz/qjoFxgzF
Dkh4ggPmbbRWjYS0XNa0p6zvlLC+cKXKnkrOnsxcbre9CQsl4fgMtoAIqApNdlmB
HNoo62fsogVUHaJyJVu7
=eD2D
-----END PGP SIGNATURE-----

Re: [Qemu-devel] virtio-rng

[Jamie Lokier]

Ian Molton wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Jamie Lokier wrote:
> > Ian Molton wrote:
> >> Heres my patch to virtio-console. The device is now specified like this:
> >>
> >> - -chardev file,path=/path/to/testfile,id=test
> >> - -device virtio-console-pci,chardev=test
> 
> Note, I think the patch above is broken and slipped by me because I
> forgot to make clean - I'll doublecheck tomorrow.
> 
> > It'd be nice if some options on the qemu command line (or config file)
> > resulted in the guest kernel getting entropy (assuming a kernel with
> > the appropriate support), without having to run a special
> > entropy-injection daemon in the guest.
> 
> How, though - AFAIK linux has no drivers specifically intended to feed
> entropy into the kernels pool - all the hwrng drivers use a userspace
> tool to do so.

You're right, with the reason being hardware randomness should be
tested before being passed back to the kernel as trusted entropy, as
it might not be functioning properly or might not have the quality it
claims to have.

With VMs, in some circumstances it might be preferable to trust the
host when it says it's providing already-tested entropy.  After all
the host has total control over the guest anyway, and the host entropy
has already been run through the same checks.

So I think virtio-rng could benefit form being a special case, if the
host says "I assert this is entropy", you might inject it directly,
and thus work even with guests that aren't running the rngd daemon for
one reason or another.  (E.g. embedded system guests.)

> I still intend to submit my virtio-rng driver, if thats what you mean,
> since it presents the data via the same routes as all the other hw rng
> sources. I feel this approach has value.

Suddenly I'm intrigued by this "intend to submit" as I see a
virtio-rng driver already in the 2.6 tree... What have I misunderstood?

Thanks,
-- Jamie

Re: [Qemu-devel] virtio-rng

[Ian Molton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jamie Lokier wrote:
> Ian Molton wrote:
>
> With VMs, in some circumstances it might be preferable to trust the
> host when it says it's providing already-tested entropy.  After all
> the host has total control over the guest anyway, and the host entropy
> has already been run through the same checks.

I dont know. The guests might not necessarily trust the host. I can
certainly see some benefit of not running the checks twice, however, and
this applies to some other hw rng drivers too - One in particular I know
will shut down if it detects that its entropy source(s) have gone bad.

> So I think virtio-rng could benefit form being a special case, if the
> host says "I assert this is entropy", you might inject it directly,
> and thus work even with guests that aren't running the rngd daemon for
> one reason or another.  (E.g. embedded system guests.)

I wonder if a 'rngd-lite' might not be an easier solution. I cant
imagine theres going to be much performance hit.

That said, even on my full-fat x86-64 box here, rngd weighs in at just
32KB and only needs libc...

>> I still intend to submit my virtio-rng driver, if thats what you mean,
>> since it presents the data via the same routes as all the other hw rng
>> sources. I feel this approach has value.
> 
> Suddenly I'm intrigued by this "intend to submit" as I see a
> virtio-rng driver already in the 2.6 tree... What have I misunderstood?

host side driver for qemu... I guess the proper term is qdev ?

- -Ian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJLAmphAAoJEFIjE1w7L6YHm5cP/0jotxofq3OUTaPtqUHGTw8K
PJ8sfkcNvJ/4GSEbCYCXcRsSqnG32R+w6LF8lOInWsi3BJsNUqRBgInGnDzC8/kV
1vkhRQELUBgNlLes+pG6GoaQZSVnQ8Z6HbNYSxyyZ+DqESR2+f0Gm8j+QdyTpXxS
ARVoDGOM+IduZL//NMy4+hBPUGymosGKepVmaT/9cVPubGumq/f+mf21AdwCPEvk
JIlRV1asqbBU7jyut5uVULpXwqygc6+kkZl6IzIPdv9BbcI9KersikL9srXZHJa9
JtyTjXdE9lsogkJkWD5Y9yL0o9oBuQdAKD8WeN+v//imzhbcuQ5kASiGMSkRo4eD
yi8oP8PIN+vhI6MhLKML7B6n4Li+xDegNxgH1qSeB3IxovTUwuVoyK7C3GIw9Kt8
h/B6FQ0gE7yNtqsFMz4m2+vWdN9ZkNPX3o5bv5DDbPiKfVUZYu0wuwCcvtD3Wbq5
SMf1rxHCgRx3B526bJGWpgeataztp1B1B2+ml2Mdbgb2r35QOaTw/ENpt4n68o/z
ITzOAOLlPHkUDLlurkQ8jMX7rT9W/NlL/y60jImjgv5zxqggEkmMlQqSnUKwIXRE
CAZAq9+6eQ/vf6r/rY8GlVVfKr5L8tRovNSbX35vbKkjpvf+aWDr66ON+bDh2Mn+
Z5LmO4HSRe/2M7e4D8HV
=9LtM
-----END PGP SIGNATURE-----

Re: [Qemu-devel] virtio-rng

[Amit Shah]

On (Mon) Nov 16 2009 [17:58:22], Ian Molton wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Gerd Hoffmann wrote:
> 
> > Maybe ...
> > 
> >    -chardev socket,id=egd,host=egd.domain.tld,port=whatever
> >    -device virtio-rng,chardev=egd
> 
> I've had a go at modifying virtio-console.c to use these semantics,
> attached below. I'd appreciate it if you could let me know if this is
> 'the right way'.

I'm doing that as part of revamping of the virtio-console driver. You
can see the latest patch I sent out here:

http://patchwork.ozlabs.org/patch/36901/

		Amit

Re: [Qemu-devel] virtio-rng

[Ian Molton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Amit Shah wrote:
> (Any reason to take this off-list?)

None other than hitting reply rather than reply-all. CCing list once
more :-)

>> Either way, you still need to specify the properties - they've just
>> moved into the console driver in your patch by the look of it.
> 
> Yeah; there are two ways of setting properties -- from the command line
> or from code.
> 
> What you're doing is something like
> 
> -virtiorng,a=foo,b=bar

Ah, I see why we're at cross purposes here - I thought we'd moved to
discussing virtio-console. I've already moved to qdev based init for
virtio-rng - 'fixing' virtio-console was where I learnt about the 'new
way' (qdev).

>> I'd prefer to do the same for virtio-rng,

Here I refer to you having (and I didnt apply the patch so I may have
misread) dropped the virtio-pci proxy from virtio-console. Once I've got
a tree I can apply your patch to I'll take another look :-)

> You should work on the qemu.git tree (use -enable-kvm for testing :).
> KVM will get the changes automatically when your changes get merged in
> qemu and the kvm maintainers next merge the qemu git repo.

I'll do that then ;-)

>> Yes, I saw that. Would it not be better to generate the device / chardev
>> pair dynamically though, rather than preserve the icky old array?
> 
> I don't understand what you mean by 'dynamically'.

Rather than have that array virtcon_hds at all, create the pairs of host
side data and qdev properties during parsing the commandline - no need
to store them up and iterate through the later, or to set an arbitrary
limit on how many can be specified in that way.

Might be an idea for qemu to warn people that this syntax will be
deprecated, too...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJLAoSmAAoJEFIjE1w7L6YHcq4P/jlzcNvWHqqkRQkdQWsy6JUh
Kh3LDh23nuK9u+aoFgXP8gpej8jT6+cAdzQ+opk/JnPQXDW4nSWcdN5HKPAmTKMP
+N78mx7w0WAOFPXd33ZRvP9mqIir6wPmwibC1aUyk6Q6+KH9Io+IDvH6qfrkIung
mYHL3WSDObkcHa6+F1ADxYA9TWr9eib6lOu8EG4LgSeJRq48+2b++kvhd8mFcMdg
yu1ed7V0U43rMhbIrAtlQsp3BWlF3KdrciFtlXyLJ9iBeSp8H0IUn+XWt1nqvNdP
T47J6Weiv7wap2RqSj7Pf5xFs45zyw2ismsfmR9iMraanISF+laZSoBynT/Et7eg
d8BoEE/PGsYeFcRm1cpETPkO7fRQxXmd8To6IqomWmT0r41KcEW9Bcs7/MIZcPpo
LtH3jZqbsd5vLEIf769rs3g38gmkNpq3sfDMJ+a5xRW9qdVOmeWfD4bPpqwIPYrq
MRF/YSZch+aLsKx7Ql+CxM+r/xY7HscIS/4hfIC8OzKnEOxO+jGiV4Mf9RMfOBZw
DdXv9YKeu3gemhguKo4NEoTonmvcv6PvhVplu5T4+wJCMjI0T2QkaJmNQFqQSfQj
FB+Qri0ad8GY5H6eirvXq/6qBHtuMP+9fJn0A/ErvlXTBKYJg+VeBQSraSylOy4c
ii9UNqk7klAw4lN7hnuP
=af4Q
-----END PGP SIGNATURE-----

Re: [Qemu-devel] virtio-rng

[Amit Shah]

On (Tue) Nov 17 2009 [11:10:32], Ian Molton wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Amit Shah wrote:
> > (Any reason to take this off-list?)
> 
> None other than hitting reply rather than reply-all. CCing list once
> more :-)
> 
> >> Either way, you still need to specify the properties - they've just
> >> moved into the console driver in your patch by the look of it.
> > 
> > Yeah; there are two ways of setting properties -- from the command line
> > or from code.
> > 
> > What you're doing is something like
> > 
> > -virtiorng,a=foo,b=bar
> 
> Ah, I see why we're at cross purposes here - I thought we'd moved to
> discussing virtio-console. I've already moved to qdev based init for
> virtio-rng - 'fixing' virtio-console was where I learnt about the 'new
> way' (qdev).

I actually meant to talk about virtio-console but had the virtio-rng
example ready in mind. I thought you did the same thing for console too?
Sorry I've not really looked at the patch in detail so you can disregard
that comment.

> >> I'd prefer to do the same for virtio-rng,
> 
> Here I refer to you having (and I didnt apply the patch so I may have
> misread) dropped the virtio-pci proxy from virtio-console. Once I've got
> a tree I can apply your patch to I'll take another look :-)

It slightly works differently now: virtio is supposed to be
pci-agnostic. So what now happens is:

virtio-console hooks on to the virtio-serial-bus via -device virtconsole
virtio-serial-bus hooks on to the virtio-pci-proxy via -device
virtio-serial-pci

> >> Yes, I saw that. Would it not be better to generate the device / chardev
> >> pair dynamically though, rather than preserve the icky old array?
> > 
> > I don't understand what you mean by 'dynamically'.
> 
> Rather than have that array virtcon_hds at all, create the pairs of host
> side data and qdev properties during parsing the commandline - no need
> to store them up and iterate through the later, or to set an arbitrary
> limit on how many can be specified in that way.

qdev won't understand the old-style commandline syntax; so once
-virtioconsole is encounters, all parsing of the arguments for that
param are to be done by the code that handles -virtioconsole. The array
is maintained because multiple virtio-consoles could be spawned, upto a
max. of MAX_VIRTIO_CONSOLES. But, as it stands, MAX_VIRTIO_CONSOLES is 1
and so the array can be dropped, but a local var. would still be needed
to fetch the chardev and then init it properly using qemu_opt.

> Might be an idea for qemu to warn people that this syntax will be
> deprecated, too...

Yes, that should be done (maybe for 1-2 releases).

		Amit