From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joao Martins Subject: Re: [PATCH RFC 00/39] x86/KVM: Xen HVM guest support Date: Thu, 21 Feb 2019 11:45:32 +0000 Message-ID: <8b1f4912-4f92-69ae-ae01-d899d5640572@oracle.com> References: <20190220201609.28290-1-joao.m.martins@oracle.com> <35051310-c497-8ad5-4434-1b8426a317d2@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Ankur Arora , Boris Ostrovsky , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , x86@kernel.org, Juergen Gross , Stefano Stabellini , xen-devel@lists.xenproject.org To: Paolo Bonzini Return-path: In-Reply-To: <35051310-c497-8ad5-4434-1b8426a317d2@redhat.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 2/20/19 9:09 PM, Paolo Bonzini wrote: > On 20/02/19 21:15, Joao Martins wrote: >> 2. PV Driver support (patches 17 - 39) >> >> We start by redirecting hypercalls from the backend to routines >> which emulate the behaviour that PV backends expect i.e. grant >> table and interdomain events. Next, we add support for late >> initialization of xenbus, followed by implementing >> frontend/backend communication mechanisms (i.e. grant tables and >> interdomain event channels). Finally, introduce xen-shim.ko, >> which will setup a limited Xen environment. This uses the added >> functionality of Xen specific shared memory (grant tables) and >> notifications (event channels). > > I am a bit worried by the last patches, they seem really brittle and > prone to breakage. I don't know Xen well enough to understand if the > lack of support for GNTMAP_host_map is fixable, but if not, you have to > define a completely different hypercall. > I guess Ankur already answered this; so just to stack this on top of his comment. The xen_shim_domain() is only meant to handle the case where the backend has/can-have full access to guest memory [i.e. netback and blkback would work with similar assumptions as vhost?]. For the normal case, where a backend *in a guest* maps and unmaps other guest memory, this is not applicable and these changes don't affect that case. IOW, the PV backend here sits on the hypervisor, and the hypercalls aren't actual hypercalls but rather invoking shim_hypercall(). The call chain would go more or less like: gnttab_map_refs(map_ops, pages) HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,...) shim_hypercall() shim_hcall_gntmap() Our reasoning was that given we are already in KVM, why mapping a page if the user (i.e. the kernel PV backend) is himself? The lack of GNTMAP_host_map is how the shim determines its user doesn't want to map the page. Also, there's another issue where PV backends always need a struct page to reference the device inflight data as Ankur pointed out. > Of course, tests are missing. FWIW: this was deliberate as we wanted to get folks impressions before proceeding further with the work. > You should use the > tools/testing/selftests/kvm/ framework, and ideally each patch should > come with coverage for the newly-added code. > Got it. Cheers, Joao