xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: George Dunlap <george.dunlap@eu.citrix.com>, xen-devel@lists.xen.org
Cc: Jan Beulich <beulich@suse.com>,
	Ian Jackson <ian.jackson@citrix.com>, Keir Fraser <keir@xen.org>,
	Ian Campbell <ian.campbell@citrix.com>, Tim Deegan <tim@xen.org>
Subject: Re: [PATCH v14 00/20] Introduce PVH domU support
Date: Mon, 11 Nov 2013 13:37:14 +0100	[thread overview]
Message-ID: <5280CF7A.50000@citrix.com> (raw)
In-Reply-To: <1383567306-6636-1-git-send-email-george.dunlap@eu.citrix.com>

On 04/11/13 13:14, George Dunlap wrote:
> Updates:
>  - Fixed bugs in v14:
>    Zombie domains, FreeBSD crash, Crash at 4GiB, HVM crash
>    (Thank you to Roger Pau Mone for fixes to the last 3)
>  - Completely eliminated PV emulation codepath
> 
> == RFC ==
> 
> We had talked about accepting the patch series as-is once I had the
> known bugs fixed; but I couldn't help making an attempt at using the
> HVM IO emulation codepaths so that we could completely eliminate
> having to use the PV emulation code, in turn eliminating some of the
> uglier "support" patches required to make the PV emulation code
> capable of running on a PVH guest.  The idea for "admin" pio ranges
> would be that we would use the vmx hardware to allow the guest direct
> access, rather than the "re-execute with guest GPRs" trick that PV
> uses.  (This functionality is not implememted by this patch series, so
> we would need to make sure it was sorted for the dom0 series.)
> 
> The result looks somewhat cleaner to me.  On the other hand, because
> string in & out instructions use the full emulation code, it means
> opening up an extra 6k lines of code to PVH guests, including all the
> complexity of the ioreq path.  (It doesn't actually send ioreqs, but
> since it shares much of the path, it shares much of the complexity.)
> Additionally, I'm not sure I've done it entirely correctly: the guest
> boots and the io instructions it executes seem to be handled
> correctly, but it may not be using the corner cases.
> 
> This also means no support for "legacy" forced invalid ops -- only native
> cpuid is supported in this series.
> 
> I have the fixes in another series, if people think it would be better
> to check in exactly what we had with bug fixes ASAP.
> 
> Other "open issues" on the design (which need not stop the series
> going in) include:
> 
>  - Whether a completely separate mode is necessary, or whether having
> just having HVM mode with some flags to disable / change certain
> functionality would be better
> 
>  - Interface-wise: Right now PVH is special-cased for bringing up
> CPUs.  Is this what we want to do going forward, or would it be better
> to try to make it more like PV (which was tried before and is hard), or more
> like HVM (which would involve having emulated APICs, &c &c).
> 
> == Summay ==
> 
> This patch series is a reworking of a series developed by Mukesh
> Rathor at Oracle.  The entirety of the design and development was done
> by him; I have only reworked, reorganized, and simplified things in a
> way that I think makes more sense.  The vast majority of the credit
> for this effort therefore goes to him.  This version is labelled v14
> because it is based on his most recent series, v11.
> 
> Because this is based on his work, I retain the "Signed-off-by" in
> patches which are based on his code.  This is not meant to imply that
> he supports the modified version, only that he is involved in
> certifying that the origin of the code for copyright purposes.
> 
> This patch series is broken down into several broad strokes:
> * Miscellaneous fixes or tweaks
> * Code motion, so future patches are simpler
> * Introduction of the "hvm_container" concept, which will form the
> basis for sharing codepaths between hvm and pvh
> * Start with PVH as an HVM container
> * Disable unneeded HVM functionality
> * Enable PV functionality
> * Disable not-yet-implemented functionality
> * Enable toolstack changes required to make PVH guests
> 
> This patch series can also be pulled from this git tree:
>  git://xenbits.xen.org/people/gdunlap/xen.git out/pvh-v14
> 
> The kernel code for PVH guests can be found here:
>  git://oss.oracle.com/git/mrathor/linux.git pvh.v9-muk-1
> (That repo/branch also contains a config file, pvh-config-file)
> 
> Changes in v14 can be found inline; major changes since v13 include:
> 
> * Various bug fixes
> 
> * Use HVM emulation for IO instructions
> 
> * ...thus removing many of the changes required to allow the PV
>   emulation codepath to work for PVH guests
> 
> Changes in v13 can be found inline; major changes since v12 include:
> 
> * Include Mukesh's toolstack patches (v4)
> 
> * Allocate hvm_param struct for PVH domains; remove patch disabling
>   memevents
> 
> For those who have been following the series as it develops, here is a
> summary of the major changes from Mukesh's series (v11->v12):
> 
> * Introduction of "has_hvm_container_*()" macros, rather than using
>   "!is_pv_*".  The patch which introduces this also does the vast
>   majority of the "heavy lifting" in terms of defining PVH.
> 
> * Effort is made to use as much common code as possible.  No separate
>   vmcs constructor, no separate vmexit handlers.  More of a "start
>   with everything and disable if necessary" approach rather than
>   "start with nothing and enable as needed" approach.
> 
> * One exception is arch_set_info_guest(), where a small amount of code
>   duplication meant a lot fewer "if(!is_pvh_domain())"s in awkward
>   places
> 
> * I rely on things being disabled at a higher level and passed down.
>   For instance, I no longer explicitly disable rdtsc exiting in
>   construct_vmcs(), since that will happen automatically when we're in
>   NEVER_EMULATE mode (which is currently enforced for PVH).  Similarly
>   for nested vmx and things relating to HAP mode.
> 
> * I have also done a slightly more extensive audit of is_pv_* and
>   is_hvm_* and tried to do more restrictions.
> 
> * I changed the "enable PVH by setting PV + HAP", replacing it instead
>   with a separate flag, just like the HVM case, since it makes sense
>   to plan on using shadow in the future (although it is 
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Mukesh Rathor <mukesh.rathor@oracle.com>
> CC: Jan Beulich <beulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Keir Fraser <keir@xen.org>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>

I've tested this new series, and all the bugs I've found in the previous
version are gone:

Tested-by: Roger Pau Monné <roger.pau@citrix.com>

      parent reply	other threads:[~2013-11-11 12:37 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-04 12:14 [PATCH v14 00/20] Introduce PVH domU support George Dunlap
2013-11-04 12:14 ` [PATCH v14 01/17] Allow vmx_update_debug_state to be called when v!=current George Dunlap
2013-11-04 16:01   ` Jan Beulich
2013-11-04 16:18   ` George Dunlap
2013-11-04 12:14 ` [PATCH v14 02/17] libxc: Move temporary grant table mapping to end of memory George Dunlap
2013-11-05 10:57   ` Roger Pau Monné
2013-11-05 11:01     ` Ian Campbell
2013-11-04 12:14 ` [PATCH v14 03/17] pvh prep: code motion George Dunlap
2013-11-04 16:14   ` Jan Beulich
2013-11-07 10:48     ` George Dunlap
2013-11-04 12:14 ` [PATCH v14 04/17] Introduce pv guest type and has_hvm_container macros George Dunlap
2013-11-04 16:20   ` Jan Beulich
2013-11-04 16:26     ` George Dunlap
2013-11-04 16:39     ` George Dunlap
2013-11-07 10:55     ` George Dunlap
2013-11-07 11:04       ` Jan Beulich
2013-11-07 11:11         ` George Dunlap
2013-11-04 12:14 ` [PATCH v14 05/17] pvh: Introduce PVH guest type George Dunlap
2013-11-06 23:28   ` Tim Deegan
2013-11-07 11:21     ` George Dunlap
2013-11-07 16:59       ` Tim Deegan
2013-11-04 12:14 ` [PATCH v14 06/17] pvh: Disable unneeded features of HVM containers George Dunlap
2013-11-04 16:21   ` George Dunlap
2013-11-04 16:37   ` Jan Beulich
2013-11-06 23:54   ` Tim Deegan
2013-11-07  9:00     ` Jan Beulich
2013-11-07 17:02       ` Tim Deegan
2013-11-04 12:14 ` [PATCH v14 07/17] pvh: vmx-specific changes George Dunlap
2013-11-04 16:19   ` George Dunlap
2013-11-04 16:42   ` Jan Beulich
2013-11-07  0:28     ` Tim Deegan
2013-11-07  0:27   ` Tim Deegan
2013-11-07 14:50     ` George Dunlap
2013-11-07 15:40       ` Andrew Cooper
2013-11-07 15:43         ` George Dunlap
2013-11-07 17:00       ` Tim Deegan
2013-11-04 12:14 ` [PATCH v14 08/17] pvh: Do not allow PVH guests to change paging modes George Dunlap
2013-11-04 12:14 ` [PATCH v14 09/17] pvh: PVH access to hypercalls George Dunlap
2013-11-04 12:14 ` [PATCH v14 10/17] pvh: Use PV e820 George Dunlap
2013-11-04 12:15 ` [PATCH v14 11/17] pvh: Set up more PV stuff in set_info_guest George Dunlap
2013-11-04 16:20   ` George Dunlap
2013-11-04 16:53   ` Jan Beulich
2013-11-07 15:51     ` George Dunlap
2013-11-07 16:10       ` Jan Beulich
2013-11-07 16:33         ` George Dunlap
2013-11-04 12:15 ` [PATCH v14 12/17] pvh: Use PV handlers for cpuid, and IO George Dunlap
2013-11-04 16:20   ` George Dunlap
2013-11-05  8:42   ` Jan Beulich
2013-11-07 16:50     ` George Dunlap
2013-11-04 12:15 ` [PATCH v14 13/17] pvh: Disable 32-bit guest support for now George Dunlap
2013-11-04 12:15 ` [PATCH v14 14/17] pvh: Restrict tsc_mode to NEVER_EMULATE " George Dunlap
2013-11-04 12:15 ` [PATCH v14 15/17] pvh: Documentation George Dunlap
2013-11-04 12:15 ` [PATCH v14 16/17] PVH xen tools: libxc changes to build a PVH guest George Dunlap
2013-11-04 12:15 ` [PATCH v14 17/17] PVH xen tools: libxl changes to create " George Dunlap
2013-11-04 16:59 ` [PATCH v14 00/20] Introduce PVH domU support Konrad Rzeszutek Wilk
2013-11-04 17:23   ` George Dunlap
2013-11-04 17:34     ` Tim Deegan
2013-11-08 15:41       ` George Dunlap
2013-11-08 15:53         ` George Dunlap
2013-11-08 17:01           ` Tim Deegan
2013-11-08 17:06             ` George Dunlap
2013-11-08 15:58         ` Konrad Rzeszutek Wilk
2013-11-07  1:11 ` Tim Deegan
2013-11-11 12:37 ` Roger Pau Monné [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5280CF7A.50000@citrix.com \
    --to=roger.pau@citrix.com \
    --cc=beulich@suse.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=keir@xen.org \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).