Re: [PATCH v5 00/16] KVM: Add minimal support for Xen HVM guests

From: Paolo Bonzini <pbonzini@redhat.com>
To: David Woodhouse <dwmw2@infradead.org>, kvm@vger.kernel.org
Cc: Ankur Arora <ankur.a.arora@oracle.com>,
	Joao Martins <joao.m.martins@oracle.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Sean Christopherson <seanjc@google.com>,
	graf@amazon.com, iaslan@amazon.de, pdurrant@amazon.com,
	aagch@amazon.com, fandree@amazon.com, hch@infradead.org
Subject: Re: [PATCH v5 00/16] KVM: Add minimal support for Xen HVM guests
Date: Thu, 28 Jan 2021 13:45:38 +0100	[thread overview]
Message-ID: <98732694-7e57-2457-2519-4f5d6dcc724c@redhat.com> (raw)
In-Reply-To: <20210111195725.4601-1-dwmw2@infradead.org>

On 11/01/21 20:57, David Woodhouse wrote:
> This patch set provides enough kernel support to allow hosting Xen HVM
> guests in KVM. It allows hypercalls to be trapped to userspace for
> handling, uses the existing KVM functions for writing system clock and
> pvclock information to Xen shared pages, and adds Xen runstate info and
> event channel upcall vector delivery.
> 
> It's based on the first section of a patch set that Joao posted as
> RFC last year^W^W in 2019:
> 
> https://lore.kernel.org/kvm/20190220201609.28290-1-joao.m.martins@oracle.com/
> 
> I've updated and reworked the original a bit, including (in my v1):
>   • Support for 32-bit guests
>   • 64-bit second support in wallclock
>   • Time counters for runnable/blocked states in runstate support
>   • Self-tests
>   • Fixed Viridian coexistence
>   • No new KVM_CAP_XEN_xxx, just more bits returned by KVM_CAP_XEN_HVM
> 
> v2:
>   • Remember the RCU read-critical sections on using the shared info pages
>   • Fix 32-bit build of compat structures (which we use there too)
>   • Use RUNSTATE_blocked as initial state not RUNSTATE_runnable
>   • Include documentation, add cosmetic KVM_XEN_HVM_CONFIG_HYPERCALL_MSR
> 
> v3:
>   • Stop mapping the shared pages; use kvm_guest_write_cached() instead.
>   • Use kvm_setup_pvclock_page() for Xen pvclock writes too.
>   • Fix CPU numbering confusion and update documentation accordingly.
>   • Support HVMIRQ_callback_vector delivery based on evtchn_upcall_pending.
> 
> v4:
>   • Rebase on top of the KVM changes merged into 5.11-rc1.
>   • Drop the kvm_{un,}map_gfn() cleanup as it isn't used since v2 anyway.
>   • Trivial cosmetic cleanup (superfluous parens, remove declaration of a
>     function removed in v3, etc.)
> 
> v5:
>   • Rebased onto kvm/next as of 2021-01-08 (commit 872f36eb0b0f4).
>   • Fix error handling for XEN_HVM_GET_ATTR.
>   • Stop moving struct kvm_host_map definition; it's not used any more.
>   • Add explicit padding to struct kvm_xen_hvm_attr to make it have
>     identical layout on 32-bit vs. 64-bit machines.

Sorry for the delay, this already looks pretty good though.  The only 
substantial issues are:

- the userspace get/set API

- the kvm_xen_has_interrupt() in the last patch.

I would be happy to get this in 5.12 if you can fix those two.

Paolo