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

Re: [Xen-devel] [RFC][v3][PATCH 1/6] xen:x86: record RMRR mappings



On 2014/8/18 17:51, Andrew Cooper wrote:
On 18/08/14 08:45, Chen, Tiejun wrote:
On 2014/8/15 17:39, Andrew Cooper wrote:
On 15/08/14 09:27, Tiejun Chen wrote:
We will expose RMRR mappings to VM so need to record all
RMRR mappings firstly.

Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
---
   xen/arch/x86/e820.c                |  2 ++
   xen/drivers/passthrough/vtd/dmar.c | 14 ++++++++++++++
   xen/include/asm-x86/e820.h         |  3 +++
   3 files changed, 19 insertions(+)

diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index 55fe0d6..db44afd 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -34,6 +34,8 @@ static bool_t __initdata e820_verbose;
   boolean_param("e820-verbose", e820_verbose);

   struct e820map e820;
+/* Used to record RMRR mapping. */
+rmrr_maps_t rmrr_maps;

   /*
    * This function checks if the entire range <start,end> is mapped
with type.
diff --git a/xen/drivers/passthrough/vtd/dmar.c
b/xen/drivers/passthrough/vtd/dmar.c
index 1152c3a..24321e3 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -29,6 +29,7 @@
   #include <xen/pci.h>
   #include <xen/pci_regs.h>
   #include <asm/string.h>
+#include <asm/e820.h>
   #include "dmar.h"
   #include "iommu.h"
   #include "extern.h"
@@ -80,6 +81,18 @@ static int __init acpi_register_rmrr_unit(struct
acpi_rmrr_unit *rmrr)
       return 0;
   }

+/* Record RMRR mapping to ready expose VM. */
+static int __init rmrr_maps_register(struct acpi_rmrr_unit *rmrr)
+{

You absolutely need some protection against calling this function more

Could you show this assumed scenario?

IMO, this should never be happened since we always use a e820 table to
cover RMRR range/entries.

There is, as far as I am aware, no upper bound to the number of RMRRs
reported by the firmware.  rmrr_maps.map is an array of length E820_MAX,

As you know all RMRR can be listed in a e820 table so its not possible to be over 128.

or 128.  If this function is called more than 128 times, you will start
clobbering Xen data.

And this is just a static function here, we also don't call that with >128.

Anyway, I can add something to deliver warning message like this,

+
+    if ( rmrr_map.nr_map >= E820MAX )
+        printk(XENLOG_WARNING
+ "Overflow RMRR mappings! Failed to record RMRR mappings.\n");


But as Jan pointed out in the other fork of this thread, this entire
function is not needed, and can be removed.

I'm a little bit disagreed here. Please see my reply to Jan.

Thanks
Tiejun

~Andrew



_______________________________________________
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®.