From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Date: Wed, 11 Oct 2023 09:25:56 +0200 Subject: [PATCH 3/6] RISC-V: KVM: Forward SBI DBCN extension to user-space In-Reply-To: References: <20231010170503.657189-1-apatel@ventanamicro.com> <20231010170503.657189-4-apatel@ventanamicro.com> <2023101048-attach-drift-d77b@gregkh> Message-ID: <2023101105-oink-aerospace-989e@gregkh> List-Id: To: kvm-riscv@lists.infradead.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Wed, Oct 11, 2023 at 12:02:30PM +0530, Anup Patel wrote: > On Tue, Oct 10, 2023 at 10:45?PM Greg Kroah-Hartman > wrote: > > > > On Tue, Oct 10, 2023 at 10:35:00PM +0530, Anup Patel wrote: > > > The SBI DBCN extension needs to be emulated in user-space > > > > Why? > > The SBI debug console is similar to a console port available to > KVM Guest so the KVM user space tool (i.e. QEMU-KVM or > KVMTOOL) can redirect the input/output of SBI debug console > wherever it wants (e.g. telnet, file, stdio, etc). > > We forward SBI DBCN calls to KVM user space so that the > in-kernel KVM does not need to be aware of the guest > console devices. Hint, my "Why" was attempting to get you to write a better changelog description, which would include the above information. Please read the kernel documentation for hints on how to do this so that we know what why changes are being made. > > > so let > > > us forward console_puts() call to user-space. > > > > What could go wrong! > > > > Why does userspace have to get involved in a console message? Why is > > this needed at all? The kernel can not handle userspace consoles as > > obviously they have to be re-entrant and irq safe. > > As mentioned above, these are KVM guest console messages which > the VMM (i.e. KVM user-space) can choose to manage on its own. If it chooses not to, what happens? > This is more about providing flexibility to KVM user-space which > allows it to manage guest console devices. Why not use the normal virtio console device interface instead of making a riscv-custom one? Where is the userspace side of this interface at? Where are the patches to handle this new api you added? > > > > > > > > > Signed-off-by: Anup Patel > > > --- > > > arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 + > > > arch/riscv/include/uapi/asm/kvm.h | 1 + > > > arch/riscv/kvm/vcpu_sbi.c | 4 ++++ > > > arch/riscv/kvm/vcpu_sbi_replace.c | 31 +++++++++++++++++++++++++++ > > > 4 files changed, 37 insertions(+) > > > > > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > index 8d6d4dce8a5e..a85f95eb6e85 100644 > > > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > @@ -69,6 +69,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm; > > > +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor; > > > > > > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h > > > index 917d8cc2489e..60d3b21dead7 100644 > > > --- a/arch/riscv/include/uapi/asm/kvm.h > > > +++ b/arch/riscv/include/uapi/asm/kvm.h > > > @@ -156,6 +156,7 @@ enum KVM_RISCV_SBI_EXT_ID { > > > KVM_RISCV_SBI_EXT_PMU, > > > KVM_RISCV_SBI_EXT_EXPERIMENTAL, > > > KVM_RISCV_SBI_EXT_VENDOR, > > > + KVM_RISCV_SBI_EXT_DBCN, > > > KVM_RISCV_SBI_EXT_MAX, > > > > You just broke a user/kernel ABI here, why? > > The KVM_RISCV_SBI_EXT_MAX only represents the number > of entries in "enum KVM_RISCV_SBI_EXT_ID" so we are not > breaking "enum KVM_RISCV_SBI_EXT_ID" rather appending > new ID to existing enum. So you are sure that userspace never actually tests or sends that _MAX value anywhere? If not, why is it even needed? thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 57A56CD98F6 for ; Wed, 11 Oct 2023 07:26:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=gSQ0o6Jebhh5UDN7Gfej//Kulu1iSHvzEBpdifkRn0I=; b=apc+37PWgnRmJl eV/P1Strm3q74tJQkCpUpF9lzNtd152e1QCwa6VLJ3imGY24yz1JPKrPqHXv488l7vqUJe4d5tWdl Bd+MvQPDS2GR+2yzrFzbJjhG1ELNTkrimaPxGh0n14uAv8BgTyaVja142DwwOWlEl3Git+TVKhiYG +/uHDECHx9V4sKCotG61mU2QlUcJbTSU/t7lYu69bn6WM7QIwrH9TBShNmP18bTIwX6wpaUUfysTs VavQ6sG1OK8/GxrkyHoHbo806X4OHVlrwxjV2knboi/u/ofU2vClWhXO56RsxtuCYT3JI/uAmyTXu yVyryL85BKWaoAKgj6BQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qqTbc-00F4sT-0D; Wed, 11 Oct 2023 07:26:08 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qqTbV-00F4qu-0p; Wed, 11 Oct 2023 07:26:06 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by ams.source.kernel.org (Postfix) with ESMTP id 9F0D2B81F46; Wed, 11 Oct 2023 07:25:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 72CD2C433C7; Wed, 11 Oct 2023 07:25:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1697009159; bh=I+hjt1PIAmeTlSR8yMwMC5kINnpTg7kd2uxBgq828hE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=vvUjAcwiuU1Y7Tj7a0D35EZe0SxqN+mW7fhJhdPKPZHsA7bkpi+kq6XyZgKWRJkuv wFoBLopdpeV8uJrtE3LTjN5hQ4GGXvHKw8zhDirGSmsdZTrIVkOmuPDRDKHXj1+LnL 4btIOyNUCM+4qGWuJA+oAlJPtLbETjMO8IeYC9sw= Date: Wed, 11 Oct 2023 09:25:56 +0200 From: Greg Kroah-Hartman To: Anup Patel Cc: Paolo Bonzini , Atish Patra , Palmer Dabbelt , Paul Walmsley , Jiri Slaby , Conor Dooley , Andrew Jones , kvm@vger.kernel.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-serial@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/6] RISC-V: KVM: Forward SBI DBCN extension to user-space Message-ID: <2023101105-oink-aerospace-989e@gregkh> References: <20231010170503.657189-1-apatel@ventanamicro.com> <20231010170503.657189-4-apatel@ventanamicro.com> <2023101048-attach-drift-d77b@gregkh> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231011_002601_606802_2C714729 X-CRM114-Status: GOOD ( 27.12 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org T24gV2VkLCBPY3QgMTEsIDIwMjMgYXQgMTI6MDI6MzBQTSArMDUzMCwgQW51cCBQYXRlbCB3cm90 ZToKPiBPbiBUdWUsIE9jdCAxMCwgMjAyMyBhdCAxMDo0NeKAr1BNIEdyZWcgS3JvYWgtSGFydG1h bgo+IDxncmVna2hAbGludXhmb3VuZGF0aW9uLm9yZz4gd3JvdGU6Cj4gPgo+ID4gT24gVHVlLCBP Y3QgMTAsIDIwMjMgYXQgMTA6MzU6MDBQTSArMDUzMCwgQW51cCBQYXRlbCB3cm90ZToKPiA+ID4g VGhlIFNCSSBEQkNOIGV4dGVuc2lvbiBuZWVkcyB0byBiZSBlbXVsYXRlZCBpbiB1c2VyLXNwYWNl Cj4gPgo+ID4gV2h5Pwo+IAo+IFRoZSBTQkkgZGVidWcgY29uc29sZSBpcyBzaW1pbGFyIHRvIGEg Y29uc29sZSBwb3J0IGF2YWlsYWJsZSB0bwo+IEtWTSBHdWVzdCBzbyB0aGUgS1ZNIHVzZXIgc3Bh Y2UgdG9vbCAoaS5lLiBRRU1VLUtWTSBvcgo+IEtWTVRPT0wpIGNhbiByZWRpcmVjdCB0aGUgaW5w dXQvb3V0cHV0IG9mIFNCSSBkZWJ1ZyBjb25zb2xlCj4gd2hlcmV2ZXIgaXQgd2FudHMgKGUuZy4g IHRlbG5ldCwgZmlsZSwgc3RkaW8sIGV0YykuCj4gCj4gV2UgZm9yd2FyZCBTQkkgREJDTiBjYWxs cyB0byBLVk0gdXNlciBzcGFjZSBzbyB0aGF0IHRoZQo+IGluLWtlcm5lbCBLVk0gZG9lcyBub3Qg bmVlZCB0byBiZSBhd2FyZSBvZiB0aGUgZ3Vlc3QKPiBjb25zb2xlIGRldmljZXMuCgpIaW50LCBt eSAiV2h5IiB3YXMgYXR0ZW1wdGluZyB0byBnZXQgeW91IHRvIHdyaXRlIGEgYmV0dGVyIGNoYW5n ZWxvZwpkZXNjcmlwdGlvbiwgd2hpY2ggd291bGQgaW5jbHVkZSB0aGUgYWJvdmUgaW5mb3JtYXRp b24uICBQbGVhc2UgcmVhZCB0aGUKa2VybmVsIGRvY3VtZW50YXRpb24gZm9yIGhpbnRzIG9uIGhv dyB0byBkbyB0aGlzIHNvIHRoYXQgd2Uga25vdyB3aGF0CndoeSBjaGFuZ2VzIGFyZSBiZWluZyBt YWRlLgoKPiA+ID4gc28gbGV0Cj4gPiA+IHVzIGZvcndhcmQgY29uc29sZV9wdXRzKCkgY2FsbCB0 byB1c2VyLXNwYWNlLgo+ID4KPiA+IFdoYXQgY291bGQgZ28gd3JvbmchCj4gPgo+ID4gV2h5IGRv ZXMgdXNlcnNwYWNlIGhhdmUgdG8gZ2V0IGludm9sdmVkIGluIGEgY29uc29sZSBtZXNzYWdlPyAg V2h5IGlzCj4gPiB0aGlzIG5lZWRlZCBhdCBhbGw/ICBUaGUga2VybmVsIGNhbiBub3QgaGFuZGxl IHVzZXJzcGFjZSBjb25zb2xlcyBhcwo+ID4gb2J2aW91c2x5IHRoZXkgaGF2ZSB0byBiZSByZS1l bnRyYW50IGFuZCBpcnEgc2FmZS4KPiAKPiBBcyBtZW50aW9uZWQgYWJvdmUsIHRoZXNlIGFyZSBL Vk0gZ3Vlc3QgY29uc29sZSBtZXNzYWdlcyB3aGljaAo+IHRoZSBWTU0gKGkuZS4gS1ZNIHVzZXIt c3BhY2UpIGNhbiBjaG9vc2UgdG8gbWFuYWdlIG9uIGl0cyBvd24uCgpJZiBpdCBjaG9vc2VzIG5v dCB0bywgd2hhdCBoYXBwZW5zPwoKPiBUaGlzIGlzIG1vcmUgYWJvdXQgcHJvdmlkaW5nIGZsZXhp YmlsaXR5IHRvIEtWTSB1c2VyLXNwYWNlIHdoaWNoCj4gYWxsb3dzIGl0IHRvIG1hbmFnZSBndWVz dCBjb25zb2xlIGRldmljZXMuCgpXaHkgbm90IHVzZSB0aGUgbm9ybWFsIHZpcnRpbyBjb25zb2xl IGRldmljZSBpbnRlcmZhY2UgaW5zdGVhZCBvZiBtYWtpbmcKYSByaXNjdi1jdXN0b20gb25lPwoK V2hlcmUgaXMgdGhlIHVzZXJzcGFjZSBzaWRlIG9mIHRoaXMgaW50ZXJmYWNlIGF0PyAgV2hlcmUg YXJlIHRoZSBwYXRjaGVzCnRvIGhhbmRsZSB0aGlzIG5ldyBhcGkgeW91IGFkZGVkPwoKPiAKPiA+ Cj4gPiA+Cj4gPiA+IFNpZ25lZC1vZmYtYnk6IEFudXAgUGF0ZWwgPGFwYXRlbEB2ZW50YW5hbWlj cm8uY29tPgo+ID4gPiAtLS0KPiA+ID4gIGFyY2gvcmlzY3YvaW5jbHVkZS9hc20va3ZtX3ZjcHVf c2JpLmggfCAgMSArCj4gPiA+ICBhcmNoL3Jpc2N2L2luY2x1ZGUvdWFwaS9hc20va3ZtLmggICAg IHwgIDEgKwo+ID4gPiAgYXJjaC9yaXNjdi9rdm0vdmNwdV9zYmkuYyAgICAgICAgICAgICB8ICA0 ICsrKysKPiA+ID4gIGFyY2gvcmlzY3Yva3ZtL3ZjcHVfc2JpX3JlcGxhY2UuYyAgICAgfCAzMSAr KysrKysrKysrKysrKysrKysrKysrKysrKysKPiA+ID4gIDQgZmlsZXMgY2hhbmdlZCwgMzcgaW5z ZXJ0aW9ucygrKQo+ID4gPgo+ID4gPiBkaWZmIC0tZ2l0IGEvYXJjaC9yaXNjdi9pbmNsdWRlL2Fz bS9rdm1fdmNwdV9zYmkuaCBiL2FyY2gvcmlzY3YvaW5jbHVkZS9hc20va3ZtX3ZjcHVfc2JpLmgK PiA+ID4gaW5kZXggOGQ2ZDRkY2U4YTVlLi5hODVmOTVlYjZlODUgMTAwNjQ0Cj4gPiA+IC0tLSBh L2FyY2gvcmlzY3YvaW5jbHVkZS9hc20va3ZtX3ZjcHVfc2JpLmgKPiA+ID4gKysrIGIvYXJjaC9y aXNjdi9pbmNsdWRlL2FzbS9rdm1fdmNwdV9zYmkuaAo+ID4gPiBAQCAtNjksNiArNjksNyBAQCBl eHRlcm4gY29uc3Qgc3RydWN0IGt2bV92Y3B1X3NiaV9leHRlbnNpb24gdmNwdV9zYmlfZXh0X2lw aTsKPiA+ID4gIGV4dGVybiBjb25zdCBzdHJ1Y3Qga3ZtX3ZjcHVfc2JpX2V4dGVuc2lvbiB2Y3B1 X3NiaV9leHRfcmZlbmNlOwo+ID4gPiAgZXh0ZXJuIGNvbnN0IHN0cnVjdCBrdm1fdmNwdV9zYmlf ZXh0ZW5zaW9uIHZjcHVfc2JpX2V4dF9zcnN0Owo+ID4gPiAgZXh0ZXJuIGNvbnN0IHN0cnVjdCBr dm1fdmNwdV9zYmlfZXh0ZW5zaW9uIHZjcHVfc2JpX2V4dF9oc207Cj4gPiA+ICtleHRlcm4gY29u c3Qgc3RydWN0IGt2bV92Y3B1X3NiaV9leHRlbnNpb24gdmNwdV9zYmlfZXh0X2RiY247Cj4gPiA+ ICBleHRlcm4gY29uc3Qgc3RydWN0IGt2bV92Y3B1X3NiaV9leHRlbnNpb24gdmNwdV9zYmlfZXh0 X2V4cGVyaW1lbnRhbDsKPiA+ID4gIGV4dGVybiBjb25zdCBzdHJ1Y3Qga3ZtX3ZjcHVfc2JpX2V4 dGVuc2lvbiB2Y3B1X3NiaV9leHRfdmVuZG9yOwo+ID4gPgo+ID4gPiBkaWZmIC0tZ2l0IGEvYXJj aC9yaXNjdi9pbmNsdWRlL3VhcGkvYXNtL2t2bS5oIGIvYXJjaC9yaXNjdi9pbmNsdWRlL3VhcGkv YXNtL2t2bS5oCj4gPiA+IGluZGV4IDkxN2Q4Y2MyNDg5ZS4uNjBkM2IyMWRlYWQ3IDEwMDY0NAo+ ID4gPiAtLS0gYS9hcmNoL3Jpc2N2L2luY2x1ZGUvdWFwaS9hc20va3ZtLmgKPiA+ID4gKysrIGIv YXJjaC9yaXNjdi9pbmNsdWRlL3VhcGkvYXNtL2t2bS5oCj4gPiA+IEBAIC0xNTYsNiArMTU2LDcg QEAgZW51bSBLVk1fUklTQ1ZfU0JJX0VYVF9JRCB7Cj4gPiA+ICAgICAgIEtWTV9SSVNDVl9TQklf RVhUX1BNVSwKPiA+ID4gICAgICAgS1ZNX1JJU0NWX1NCSV9FWFRfRVhQRVJJTUVOVEFMLAo+ID4g PiAgICAgICBLVk1fUklTQ1ZfU0JJX0VYVF9WRU5ET1IsCj4gPiA+ICsgICAgIEtWTV9SSVNDVl9T QklfRVhUX0RCQ04sCj4gPiA+ICAgICAgIEtWTV9SSVNDVl9TQklfRVhUX01BWCwKPiA+Cj4gPiBZ b3UganVzdCBicm9rZSBhIHVzZXIva2VybmVsIEFCSSBoZXJlLCB3aHk/Cj4gCj4gVGhlIEtWTV9S SVNDVl9TQklfRVhUX01BWCBvbmx5IHJlcHJlc2VudHMgdGhlIG51bWJlcgo+IG9mIGVudHJpZXMg aW4gImVudW0gS1ZNX1JJU0NWX1NCSV9FWFRfSUQiIHNvIHdlIGFyZSBub3QKPiBicmVha2luZyAi ZW51bSBLVk1fUklTQ1ZfU0JJX0VYVF9JRCIgcmF0aGVyIGFwcGVuZGluZwo+IG5ldyBJRCB0byBl eGlzdGluZyBlbnVtLgoKU28geW91IGFyZSBzdXJlIHRoYXQgdXNlcnNwYWNlIG5ldmVyIGFjdHVh bGx5IHRlc3RzIG9yIHNlbmRzIHRoYXQgX01BWAp2YWx1ZSBhbnl3aGVyZT8gIElmIG5vdCwgd2h5 IGlzIGl0IGV2ZW4gbmVlZGVkPwoKdGhhbmtzLAoKZ3JlZyBrLWgKCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LXJpc2N2IG1haWxpbmcgbGlzdAps aW51eC1yaXNjdkBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3Jn L21haWxtYW4vbGlzdGluZm8vbGludXgtcmlzY3YK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9D80CD98F6 for ; Wed, 11 Oct 2023 07:26:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344715AbjJKH0F (ORCPT ); Wed, 11 Oct 2023 03:26:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39998 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344424AbjJKH0E (ORCPT ); Wed, 11 Oct 2023 03:26:04 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8A45C8F; Wed, 11 Oct 2023 00:25:59 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 72CD2C433C7; Wed, 11 Oct 2023 07:25:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1697009159; bh=I+hjt1PIAmeTlSR8yMwMC5kINnpTg7kd2uxBgq828hE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=vvUjAcwiuU1Y7Tj7a0D35EZe0SxqN+mW7fhJhdPKPZHsA7bkpi+kq6XyZgKWRJkuv wFoBLopdpeV8uJrtE3LTjN5hQ4GGXvHKw8zhDirGSmsdZTrIVkOmuPDRDKHXj1+LnL 4btIOyNUCM+4qGWuJA+oAlJPtLbETjMO8IeYC9sw= Date: Wed, 11 Oct 2023 09:25:56 +0200 From: Greg Kroah-Hartman To: Anup Patel Cc: Paolo Bonzini , Atish Patra , Palmer Dabbelt , Paul Walmsley , Jiri Slaby , Conor Dooley , Andrew Jones , kvm@vger.kernel.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-serial@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/6] RISC-V: KVM: Forward SBI DBCN extension to user-space Message-ID: <2023101105-oink-aerospace-989e@gregkh> References: <20231010170503.657189-1-apatel@ventanamicro.com> <20231010170503.657189-4-apatel@ventanamicro.com> <2023101048-attach-drift-d77b@gregkh> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org On Wed, Oct 11, 2023 at 12:02:30PM +0530, Anup Patel wrote: > On Tue, Oct 10, 2023 at 10:45 PM Greg Kroah-Hartman > wrote: > > > > On Tue, Oct 10, 2023 at 10:35:00PM +0530, Anup Patel wrote: > > > The SBI DBCN extension needs to be emulated in user-space > > > > Why? > > The SBI debug console is similar to a console port available to > KVM Guest so the KVM user space tool (i.e. QEMU-KVM or > KVMTOOL) can redirect the input/output of SBI debug console > wherever it wants (e.g. telnet, file, stdio, etc). > > We forward SBI DBCN calls to KVM user space so that the > in-kernel KVM does not need to be aware of the guest > console devices. Hint, my "Why" was attempting to get you to write a better changelog description, which would include the above information. Please read the kernel documentation for hints on how to do this so that we know what why changes are being made. > > > so let > > > us forward console_puts() call to user-space. > > > > What could go wrong! > > > > Why does userspace have to get involved in a console message? Why is > > this needed at all? The kernel can not handle userspace consoles as > > obviously they have to be re-entrant and irq safe. > > As mentioned above, these are KVM guest console messages which > the VMM (i.e. KVM user-space) can choose to manage on its own. If it chooses not to, what happens? > This is more about providing flexibility to KVM user-space which > allows it to manage guest console devices. Why not use the normal virtio console device interface instead of making a riscv-custom one? Where is the userspace side of this interface at? Where are the patches to handle this new api you added? > > > > > > > > > Signed-off-by: Anup Patel > > > --- > > > arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 + > > > arch/riscv/include/uapi/asm/kvm.h | 1 + > > > arch/riscv/kvm/vcpu_sbi.c | 4 ++++ > > > arch/riscv/kvm/vcpu_sbi_replace.c | 31 +++++++++++++++++++++++++++ > > > 4 files changed, 37 insertions(+) > > > > > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > index 8d6d4dce8a5e..a85f95eb6e85 100644 > > > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > @@ -69,6 +69,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm; > > > +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor; > > > > > > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h > > > index 917d8cc2489e..60d3b21dead7 100644 > > > --- a/arch/riscv/include/uapi/asm/kvm.h > > > +++ b/arch/riscv/include/uapi/asm/kvm.h > > > @@ -156,6 +156,7 @@ enum KVM_RISCV_SBI_EXT_ID { > > > KVM_RISCV_SBI_EXT_PMU, > > > KVM_RISCV_SBI_EXT_EXPERIMENTAL, > > > KVM_RISCV_SBI_EXT_VENDOR, > > > + KVM_RISCV_SBI_EXT_DBCN, > > > KVM_RISCV_SBI_EXT_MAX, > > > > You just broke a user/kernel ABI here, why? > > The KVM_RISCV_SBI_EXT_MAX only represents the number > of entries in "enum KVM_RISCV_SBI_EXT_ID" so we are not > breaking "enum KVM_RISCV_SBI_EXT_ID" rather appending > new ID to existing enum. So you are sure that userspace never actually tests or sends that _MAX value anywhere? If not, why is it even needed? thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A8C40CD98F6 for ; Wed, 11 Oct 2023 07:26:58 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.a=rsa-sha256 header.s=korg header.b=vvUjAcwi; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4S548d2c26z3cRF for ; Wed, 11 Oct 2023 18:26:57 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.a=rsa-sha256 header.s=korg header.b=vvUjAcwi; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=linuxfoundation.org (client-ip=145.40.68.75; helo=ams.source.kernel.org; envelope-from=gregkh@linuxfoundation.org; receiver=lists.ozlabs.org) Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4S547c5JXsz2yQ8 for ; Wed, 11 Oct 2023 18:26:03 +1100 (AEDT) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by ams.source.kernel.org (Postfix) with ESMTP id 9F0D2B81F46; Wed, 11 Oct 2023 07:25:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 72CD2C433C7; Wed, 11 Oct 2023 07:25:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1697009159; bh=I+hjt1PIAmeTlSR8yMwMC5kINnpTg7kd2uxBgq828hE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=vvUjAcwiuU1Y7Tj7a0D35EZe0SxqN+mW7fhJhdPKPZHsA7bkpi+kq6XyZgKWRJkuv wFoBLopdpeV8uJrtE3LTjN5hQ4GGXvHKw8zhDirGSmsdZTrIVkOmuPDRDKHXj1+LnL 4btIOyNUCM+4qGWuJA+oAlJPtLbETjMO8IeYC9sw= Date: Wed, 11 Oct 2023 09:25:56 +0200 From: Greg Kroah-Hartman To: Anup Patel Subject: Re: [PATCH 3/6] RISC-V: KVM: Forward SBI DBCN extension to user-space Message-ID: <2023101105-oink-aerospace-989e@gregkh> References: <20231010170503.657189-1-apatel@ventanamicro.com> <20231010170503.657189-4-apatel@ventanamicro.com> <2023101048-attach-drift-d77b@gregkh> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-serial@vger.kernel.org, kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Atish Patra , linux-kernel@vger.kernel.org, Conor Dooley , Palmer Dabbelt , kvm-riscv@lists.infradead.org, Paul Walmsley , Paolo Bonzini , linux-riscv@lists.infradead.org, Jiri Slaby , Andrew Jones Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Wed, Oct 11, 2023 at 12:02:30PM +0530, Anup Patel wrote: > On Tue, Oct 10, 2023 at 10:45 PM Greg Kroah-Hartman > wrote: > > > > On Tue, Oct 10, 2023 at 10:35:00PM +0530, Anup Patel wrote: > > > The SBI DBCN extension needs to be emulated in user-space > > > > Why? > > The SBI debug console is similar to a console port available to > KVM Guest so the KVM user space tool (i.e. QEMU-KVM or > KVMTOOL) can redirect the input/output of SBI debug console > wherever it wants (e.g. telnet, file, stdio, etc). > > We forward SBI DBCN calls to KVM user space so that the > in-kernel KVM does not need to be aware of the guest > console devices. Hint, my "Why" was attempting to get you to write a better changelog description, which would include the above information. Please read the kernel documentation for hints on how to do this so that we know what why changes are being made. > > > so let > > > us forward console_puts() call to user-space. > > > > What could go wrong! > > > > Why does userspace have to get involved in a console message? Why is > > this needed at all? The kernel can not handle userspace consoles as > > obviously they have to be re-entrant and irq safe. > > As mentioned above, these are KVM guest console messages which > the VMM (i.e. KVM user-space) can choose to manage on its own. If it chooses not to, what happens? > This is more about providing flexibility to KVM user-space which > allows it to manage guest console devices. Why not use the normal virtio console device interface instead of making a riscv-custom one? Where is the userspace side of this interface at? Where are the patches to handle this new api you added? > > > > > > > > > Signed-off-by: Anup Patel > > > --- > > > arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 + > > > arch/riscv/include/uapi/asm/kvm.h | 1 + > > > arch/riscv/kvm/vcpu_sbi.c | 4 ++++ > > > arch/riscv/kvm/vcpu_sbi_replace.c | 31 +++++++++++++++++++++++++++ > > > 4 files changed, 37 insertions(+) > > > > > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > index 8d6d4dce8a5e..a85f95eb6e85 100644 > > > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > @@ -69,6 +69,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm; > > > +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental; > > > extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor; > > > > > > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h > > > index 917d8cc2489e..60d3b21dead7 100644 > > > --- a/arch/riscv/include/uapi/asm/kvm.h > > > +++ b/arch/riscv/include/uapi/asm/kvm.h > > > @@ -156,6 +156,7 @@ enum KVM_RISCV_SBI_EXT_ID { > > > KVM_RISCV_SBI_EXT_PMU, > > > KVM_RISCV_SBI_EXT_EXPERIMENTAL, > > > KVM_RISCV_SBI_EXT_VENDOR, > > > + KVM_RISCV_SBI_EXT_DBCN, > > > KVM_RISCV_SBI_EXT_MAX, > > > > You just broke a user/kernel ABI here, why? > > The KVM_RISCV_SBI_EXT_MAX only represents the number > of entries in "enum KVM_RISCV_SBI_EXT_ID" so we are not > breaking "enum KVM_RISCV_SBI_EXT_ID" rather appending > new ID to existing enum. So you are sure that userspace never actually tests or sends that _MAX value anywhere? If not, why is it even needed? thanks, greg k-h