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

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



On 2014/8/26 20:37, Jan Beulich wrote:
On 26.08.14 at 14:02, <andrew.cooper3@xxxxxxxxxx> wrote:
On 26/08/14 12:02, Tiejun Chen wrote:
+    case XENMEM_reserved_device_memory_map:
+    {
+        struct xen_mem_reserved_device_memory_map map;
+        XEN_GUEST_HANDLE(xen_mem_reserved_device_memory_t) buffer;
+        XEN_GUEST_HANDLE_PARAM(xen_mem_reserved_device_memory_t) buffer_param;
+        unsigned int i = 0;
+        static struct xen_mem_reserved_device_memory rmrr_map;

This absolutely must not be static, as I have indicated twice before.

Indeed, you should be reading what you're being told, and not
submit new patch versions without having addressed (by altering
the patch or addressing the comment verbally) previous comments.

This is my typo. I already see Andrew's comment to work that without any memory allocation, but really forget to remove 'static'.

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.

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:

1> xen/vtd/rmrr: export acpi_rmrr_units
2> xen/vtd/rmrr: introduce acpi_rmrr_unit_entries
3> xen/x86: define a new hypercall to get RMRR mappings
4> tools/libxc: introduce hypercall for xc_reserved_device_memory_map
5> tools/libxc: check if mmio BAR is out of RMRR mappings
6> hvm_info_table: introduce nr_reserved_device_memory_map
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
10> xen/vtd: make USB RMRR mapping safe


It is for that reason that I didn't comment on the last two or so
iterations of your patch series - the effort the repeated pointing
out of issues causes just makes it sit at the very end of the list of
things needing looking at).


I always read to address all comments but looks I may be missing something, I think I should check carefully.

Thanks
Tiejun

Jan



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


 


Rackspace

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