[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [V10 PATCH 10/23] PVH xen: domain create, context switch related code changes

On 08/08/13 02:42, Mukesh Rathor wrote:
On Wed, 7 Aug 2013 11:48:26 +0100
George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:

On Wed, Jul 24, 2013 at 2:59 AM, Mukesh Rathor
<mukesh.rathor@xxxxxxxxxx> wrote:
          v->arch.guest_table = pagetable_from_page(cr3_page);
-        if ( c.nat->ctrlreg[1] )
+        if ( c.nat->ctrlreg[1] && !is_pvh_vcpu(v) )
              cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[1]);
              cr3_page = get_page_from_gfn(d, cr3_gfn, NULL,
P2M_ALLOC); @@ -954,6 +965,13 @@ int arch_set_info_guest(


+    if ( is_pvh_vcpu(v) )
+    {
+        /* Set VMCS fields. */
+        if ( (rc = pvh_set_vcpu_info(v, c.nat)) != 0 )
+            return rc;
+    }
BTW, would it make sense to pull the code above, which sets FS and GS
for PV guests into a separate function, and then do something like the

  is_pv_vcpu(v) ?
    pv_set_vcpu_info() :
Perhaps! But, these patches have been out for about 7 months now, and
are tested by us and Andrew for PV and HVM regressions. I prefer not
changing common code at this point, unless buggy. May be we can do an
incremental change later if you really want this function to be

I'm keen to get the patches in, but I'm also keen on making the changes as clean as possible. It's a lot harder to go through and clean things up later. I don't think this level of re-arranging should be too much additional delay, or have very much risk of regression.

Obviously the other maintainers may feel differently. I'm sorry I'm a bit late to the party and only now able to find time to give my input. Once I have a view of the whole series, I can come back and give maybe a "Not-perfectly-happy-but-wont-NACK" or something like that, and others can decide whether to hold out for the change or just get it done with. :-)

BTW the testing thing I don't think is a valid argument; we're early in the dev cycle, so a regression isn't a big deal; and in any case the risk of regression is the same whether the change happens before it's committed or afterwards.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.