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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 30/09/2013 22:11, Chris Metcalf ha scritto:
> 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.

Thanks, that looks like the right thing to do.

The difference between s390-virtio and virtio-mmio is that s390 has a
single device that supports multiple "back-ends", with hotplug and
hot-unplug support.

virtio-mmio supports a fixed number of devices, defined in the board by
creating a number of instances of the "naked" virtio-mmio front-ends.

On the other hand, s390-virtio was never fully specified and is not part
of the virtio standardization effort (because s390 has now switched to a
different mechanism).

>>>  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.

Yeah, using virtio-console is just an implementation-dependent issue.  I
think it's better to keep the previous guest code for early printk.

Paolo