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

Re: [Xen-devel] [v4][PATCH 4/9] tools:libxc: check if mmio BAR is out of RMRR mappings

On 2014/8/27 10:40, Chen, Tiejun wrote:
On 2014/8/27 10:20, Ian Campbell wrote:
On Wed, 2014-08-27 at 09:46 +0800, Chen, Tiejun wrote:
On 2014/8/27 4:36, Ian Campbell wrote:
On Fri, 2014-08-22 at 18:09 +0800, Tiejun Chen wrote:

+    /* We should check if mmio range is out of RMRR mapping.
+     *
+     * Assume we have one entry if not enough we'll expand.
+     */

The usual approach with such hypervisor interfaces (which I suppose
xc_reserved_device_memory_map turns into) is to first call it with NULL
to get the required size and then allocate a suitable buffer and call a
second time.

Ofentimes, RMRR should be rare with one or two entries, even zero.

It's not clear to me what number you are saying is the norm here.

In my broadwell platform we just have two entries.

Even if some N is common today what guarantee is there that N won't grow
or shrink with the next generation of systems?

As I understand RMRR may be legacy, and this should go away.

What is RMRR?

There are some devices the BIOS controls, for e.g USB devices to perform
PS2 emulation. The regions of memory used for these devices are marked
reserved in the e820 map. When we turn on DMA translation, DMA to those
regions will fail. Hence BIOS uses RMRR to specify these regions along
with devices that need to access these regions. OS is expected to setup
unity mappings for these regions for these devices to access these regions.

  So I
think its reasonable to start posting one entry since this can cover
such a scenario the platform really owns one entry.

Making the call twice is not terribly expensive (nor is this a hot path)
and it allows you to avoid the reallocation and recall and the twisty
error handling structure which that implies.

As I understand when we call one given hypercall, we may know that
possible numbers to issue that. Then we can get the appropriate number
via the returned value if that is not enough. I think its better than we
always issue twice hypercall unconditionally :)

But if you persist in this fixed twice-call mechanism, I can try to
rework out this implementation :)

And actually, I think current implementation is already as you expect. Please check patch #3,

@@ -4842,6 +4843,55 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         return rc;

+    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;
+        struct xen_mem_reserved_device_memory rmrr_map;
+        struct acpi_rmrr_unit *rmrr;
+        if ( !acpi_rmrr_unit_entries )
+                return -ENOENT;
+        if ( copy_from_guest(&map, arg, 1) )
+            return -EFAULT;
+        if ( map.nr_entries < acpi_rmrr_unit_entries )
+        {
+            map.nr_entries = acpi_rmrr_unit_entries;
+            if ( copy_to_guest(arg, &map, 1) )
+                return -EFAULT;
+            return -ENOBUFS;
+        }

And turn back, here I just set map.nr_entries = 1, not '0' as you image.

+    /* Assume we have one entry if not enough we'll expand.*/
+    uint32_t nr_entries = 1;
+    /* We should check if mmio range is out of RMRR mapping. */
+    if ( (map = malloc(nr_entries *
+                       sizeof(xen_mem_reserved_device_memory_t))) == NULL )

So you really hope I should set map.nr_entries = 0 firstly?


Xen-devel mailing list



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