Re: [Xen-devel] [V11 PATCH 00/21]PVH xen: Phase I, Version 11 patches...

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Fri, Aug 23, 2013 at 9:49 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 23.08.13 at 03:18, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
>> Finally, I've the V11 set of patches.
>>
>> V11:
>>    - gdt union patch not needed anymore, so dropped it.
>>    - patch 17 made the last patch
>>    - merged patch 22 and 23.
>
> So I'd be okay with applying 1...8 and 10...16, provided
> - you, Mukesh, can confirm that 9 can safely be left out,
> - you, George, don't object to that (considering your comments
>   on v10).

1-8,10-16 I'm OK with the code for the most part, but the changesets
themselves leave something to be desired.

Many of the prep patches would be fine, and the e820 struct relocate
is OK as well (though the changelog entry isn't really good).

But the read_segment_register patch I think needs to be put in after
the is_pvh_*() patch, so the entire new bit of functionality comes in
one go.  And the guest_kernel_mode() change should be a separate
patch, since it performs a similar function to read_segment_register()
-- i.e., enabling the emulated PV ops.

In many cases, there are handfuls of other "!is_hvm" -> "is_pv"
scattered randomly throughout unrelated other changes.  And some of
the changes from patches 15-16 I think should be grouped together with
later changesets (e.g., all the irq-related ones in a single
changeset).

Also, I think that having a separate set of nearly-identical exit
handlers for PVH is a really bad idea.  Without them, however, pvh.c
is only a single small function long -- so I think we shouldn't bother
with pvh.c, and should just put that function into vmx.c.

All in all, I would personally prefer if you hold off until my series
re-work; I should have something by the end of next week.

My basic outline for the re-worked patch series looks like the
following (NOT one patch per bullet):
- Prep patches
- Introduce pvh domain type
- Disable unused HVM functionality
- Enable used PV functionality

What do you think?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel