Re: [Xen-devel] [v8][PATCH 04/17] update the existing hypercall to support XEN_DOMCTL_set_rdm

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Wednesday, December 10, 2014 5:01 PM
> 
> >>> On 10.12.14 at 04:39, <kevin.tian@xxxxxxxxx> wrote:
> > 1. It's more efficient for new people to start from a small, well-defined
> > task
> > in one area, and then spanning to adjacent areas gradually. Patience must
> > be given by the community to help them grow;
> 
> Yes. But if a large item like the RMRR one is being picked, this is a
> recipe for failure.

RMRR is already there, just broken. Here what we are doing is to fix it. 
Regarding
to that, as long as the small item doesn't cause new failures or regressions, it
should be fine.

> 
> > 2. Unfortunately this RMRR effort grows from original two patches (very
> VT-d
> > focused) to now v8 17 patches spanning many areas (including hypercall,
> mmu,
> > domain builder, hvmloader, etc.), and thus imposing both long learning curve
> > and lots of frustrations being no achievement returned. Though having a
> > complete
> > solution is desired, we need to help split the big task into step-by-step
> > approach
> > as long as:
> >     - the overall design is agreed
> >     - each step is self-contained
> >     - each step won't be wasted moving forward.
> > That way new people can see incremental achievements, instead of being hit
> > down before final success.
> >
> > Take this RMRR for example. Maybe we can split into steps like:
> >
> >     step-1: setup RMRR identify mapping in hypervisor. fail if confliction. 
> > no
> > user space changes
> >     step-2: expose RMRR knowledge to userspace and detect confliction in
> > domain builder and hvmloader
> >     step-3: reconcile e820/mmio to avoid confliction in a best effort
> >     step-4: miscellaneous enhancements, each can be ACK-ed individually:
> >             - handle guest CPU access to RMRR regions
> >             - handle conflicting RMRR regions
> >             - handle group RMRRs
> >             - re-enable USB device assignment
> >
> > That way Tiejun can focus on a self-contained task at one time, and then
> > observe
> > continuous acknowledgements for his work. We don't need to claim RMRR
> fully
> > ready in Xen until last patch is accepted, but that at least provides a way
> > to ack
> > new people when working on complex features and also allow for partial
> usage
> > on his work.
> 
> If only this wouldn't result in regressions when done in steps (like you
> outlined above, or likely also if split up in any other ways). Having to
> do this in one go is the price you/we have to pay for this not having
> got done properly from the beginning.

then let's sit down to clear the design first before going to review the detail.

> 
> > 3. Maintainers sometimes didn't give definitive guidance to the new people,
> > and the high-level design is not closed in the 1st place. e.g. when I 
> > thought
> > this v8
> > version has everyone agreed on the design, there's another comment from
> Jan
> > about using XENMEM_set_memory_map might be better, while back to Aug.
> > when Tiejun raised that option the answer is "I'm not sure". Note I
> > understand
> > as a maintainer he might not have definite answers for all opens. But w/o a
> > definitive guide new people may waste lots of effort on chasing a wrong
> > option,
> > which is definitely frustrating.
> 
> Main problem being that the maintainers to help here are primarily you,
> and only then me or others - after all this is a VT-d only problem, not a
> general IOMMU one. The fact that non-VT-d code gets touched doesn't
> matter when considering just the design. And that's why I had asked
> Tiejun to work with the two of you on getting the basis right. I don't
> know how much of that possibly happened without the public seeing it,
> but the results seem to suggest not all that much.

We worked with Tiejun on the design, and some level of code review (not much
as you did since it touches lots of different areas), and the version he sent 
out is 
what we discussed to be a right way to go. But since you tempt to have 
spontaneous different opinions in each series, let's close that first. We'll 
work
with Tiejun to send a design review request separately, and let's see how it 
works
and whether it may be split into small steps.

and... as you already noted, when 'RMRR' is a VT feature, the majority code
touched in the patch series are not in VT-d space. Somehow I view this is a
general feature development, i.e. how to reserve a resource from guest 
physical space. RMRR is just one client of this feature, and there could be
others. In such case, we need all the maintainers in corresponding areas to
help, meant you as the general maintainer, meant Tim for mmu part, and
meant Ian for domain builder part, etc... Regarding to design, we need all
on the table and come to agreement.

Thanks
Kevin


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