From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Metcalf Subject: Re: [PATCH v3 3/3] tile: enable VIRTIO support for KVM Date: Mon, 30 Sep 2013 16:11:24 -0400 Message-ID: <5249DAEC.2090106@tilera.com> References: <20130826120445.GD8218@redhat.com> <54dadfbf688618a900310d10363b9c395fa3d322.1377736306.git.cmetcalf@tilera.com> <522F14F8.2000101@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Cc: , , Jan Kiszka , Gleb Natapov To: Paolo Bonzini Return-path: Received: from usmamail.tilera.com ([12.216.194.151]:33224 "EHLO USMAMAIL.TILERA.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756386Ab3I3ULZ (ORCPT ); Mon, 30 Sep 2013 16:11:25 -0400 In-Reply-To: <522F14F8.2000101@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 > 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