Re: [Xen-devel] [v5][PATCH 03/10] xen:x86: define a new hypercall to get RMRR mappings

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2014/8/27 14:51, Jan Beulich wrote:
> > > > On 27.08.14 at 03:37, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2014/8/26 20:37, Jan Beulich wrote:
> > > For example, I had also asked you to adjust your patch titles, yet
> > Are you sure? I recheck all e-mails you replied to me but I don't find
> > this comment.
> http://lists.xenproject.org/archives/html/xen-devel/2014-08/msg00925.html
Seems in another e-mail thread, but I guess '(not just here)' means that 
should be valid here as well.
> > > they still come in the same bogus form (namely with colons rather
> > > than slashes as prefix separators - this ones should e.g. start with
> > > "xen/x86:", albeit I personally dislike the xen/ prefix and tend to
> > > strip it).
> > Anyway, I think you'd like to change all titles as follows:
> Along those lines, albeit some of the prefixes continue to be
> overly long:
>
> > 1> xen/vtd/rmrr: export acpi_rmrr_units
> > 2> xen/vtd/rmrr: introduce acpi_rmrr_unit_entries
> In these two, the trailing rmrr is meaningless: rmrr is not really
> a sub-component, and the rest of the title already establishes
> enough context.
Right.

1> xen/vtd: export acpi_rmrr_units
2> xen/vtd: introduce acpi_rmrr_unit_entries

> > 3> xen/x86: define a new hypercall to get RMRR mappings
> To avoid needless extra rounds, iirc the hypercall was requested
> to no longer be RMRR-centric. Hence the title shouldn't be either.
>
> > 4> tools/libxc: introduce hypercall for xc_reserved_device_memory_map
> > 5> tools/libxc: check if mmio BAR is out of RMRR mappings
> Similarly, this wouldn't be RMRR specific the either.
3> xen/x86: define a new hypercall to get reserved device memory map

> > 6> hvm_info_table: introduce nr_reserved_device_memory_map
> Here the prefix is rather odd: Knowing most of the sub-component
> placement throughout the code base quite well, I can't really identify
> which sub-component this is about. Please remember that the
> primary purpose of these prefixes is to make it easy to identify
> roughly which area of the code base a change affects. Hence this
> should neither be too fine grained (like you seemed to be picking
> e.g. individual header file names as prefixes) nor too coarse grained.
Agreed, so thanks for your guide.

> Just take on for yourself the viewing angle a maintainer or potential
> reviewer would take: Is this an area I should be looking at or I am
> interested in?
Hope this is fine,

6> tools/libxc: introduce nr_reserved_device_memory_map into hvm_info_table

> > 7> xen/x86: support xc_reserved_device_memory_map in compat case
> > 8> tools/firmware/hvmloader: introduce hypercall for
> >    xc_reserved_device_memory_map
> > 9> tools/firmware/hvmloader: check to reserve RMRR
> >    mappings in e820
> For these two just "hvmloader:" would seem to provide enough
> context.
8> hvmloader: check to reserve all device reserved memory in e820
9> hvmloader: introduce hypercall for xc_reserved_device_memory_map

Thanks
Tiejun

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