Re: [PATCH v3 3/3] tile: enable VIRTIO support for KVM

[Chris Metcalf <cmetcalf@tilera.com>]

As I said to Gleb in the previous email - sorry for the delay in
replying to your thoughtful comments!


On 9/10/2013 8:47 AM, Paolo Bonzini wrote:
> Il 28/08/2013 22:58, Chris Metcalf ha scritto:
>> This change enables support for a virtio-based console,
>> network support, and block driver support.
>>
>> We remove some debug code in relocate_kernel_64.S that made raw
>> calls to the hv_console_putc Tilera hypervisor API, since everything
>> now should funnel through the early_hv_write() API.
>>
>> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
> Why couldn't this use the "regular" virtio-mmio interface?

We probably should!  We were working with a CentOS 6 style distribution,
which has an older version of qemu; we upgraded slightly to 0.13 in
the thought that minimizing version skew would help distribution compatibility.
That version doesn't have the virtio-mmio stuff.  But you're right, we probably
should return the virtio-mmio stuff to the community instead, even if we're
going to keep something like this patch in our local copy of KVM.

>>  static void early_hv_write(struct console *con, const char *s, unsigned n)
>>  {
>> +#ifdef CONFIG_KVM_GUEST
>> +     char buf[512];
>> +
>> +     if (n > sizeof(buf) - 1)
>> +             n = sizeof(buf) - 1;
>> +     memcpy(buf, s, n);
>> +     buf[n] = '\0';
>> +
>> +     hcall_virtio(KVM_VIRTIO_NOTIFY, __pa(buf));
> How can userspace know the difference between KVM_VIRTIO_NOTIFY with a
> string buffer, and KVM_VIRTIO_NOTIFY with a config space pointer?
>
> In fact, this looks like a completely separate hypercall, why not keep
> hv_console_putc?

Good point.  Right now in qemu the virtio hypercall with a KVM_VIRTIO_NOTIFY
reason either does a virtio_queue_notify(), if the address is not in RAM,
or a print, if it is.  It does seem we could just have separate calls;
the reason we grouped it in with the KVM_VIRTIO stuff instead of implementing
it with the hv_console_write() API is just that it uses the virtio_console
API to do the work.  But we probably could do it the other way too, and
that might arguably make more sense.  We'll think about it.

Thanks!

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com