[RFC] xen/arm: Find automatically the gnttab region for DOM0

[RFC] xen/arm: Find automatically the gnttab region for DOM0

[Julien Grall]

Currently, the grant table region is hardcoded per-platform. When a new
board is coming up, we have to check the spec in order to find a space
in the memory layout free. Depending on the platform it may be tedious.

A good candidate for the gnttab region is the one used by Xen binary as
some part will never be mapped to the DOM0 address, MMIO are mapped 1:1
and the RAM will be either:
    - direct mapped: 1:1 mapping is used => no problem
    - non direct mapped: Xen always relocates himself as high as possible
    (limited to 4GB on ARM32) and the RAM bank are filled from the first
    one. It's very unlikely that the gnttab region will overlap with the
    RAM. Although for safety a check may be necessary when we will reenable
    the option.

Futhermore, there is plenty of space to contain a big gnttab, the default
size is 32 frame (i.e 32KB) but it can be changed via a command option.

We can only use text section as it will always reside in memory, never be
mapped and the size is big enough (~300KB on ARM64).

Note that on ARM64, the grant table region may be after 4GB (Xen is
relocated to the highest address) using DOM0 32 bit with short page table
may not work. Although, I don't think this is a big deal as device may not
work and/or the RAM is too high due to the 1:1 mapping.

This patch also drop the platforms thunderx and xilinx-zynqmp which became
dummy by dropping the hardcoding DOM0 grant table region.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
    This patch depends on the patch to use ULL instead of UL for GB [1].

    [1] http://lists.xen.org/archives/html/xen-devel/2015-05/msg02972.html
---
 xen/arch/arm/domain_build.c            | 46 ++++++++++++++++++++++++++++------
 xen/arch/arm/kernel.h                  |  4 +++
 xen/arch/arm/platform.c                | 14 -----------
 xen/arch/arm/platforms/Makefile        |  2 --
 xen/arch/arm/platforms/midway.c        |  3 ---
 xen/arch/arm/platforms/omap5.c         |  6 -----
 xen/arch/arm/platforms/rcar2.c         |  3 ---
 xen/arch/arm/platforms/seattle.c       |  3 ---
 xen/arch/arm/platforms/sunxi.c         |  3 ---
 xen/arch/arm/platforms/thunderx.c      | 41 ------------------------------
 xen/arch/arm/platforms/vexpress.c      |  2 --
 xen/arch/arm/platforms/xgene-storm.c   |  3 ---
 xen/arch/arm/platforms/xilinx-zynqmp.c | 41 ------------------------------
 xen/include/asm-arm/platform.h         |  7 ------
 14 files changed, 42 insertions(+), 136 deletions(-)
 delete mode 100644 xen/arch/arm/platforms/thunderx.c
 delete mode 100644 xen/arch/arm/platforms/xilinx-zynqmp.c

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1e545fe..6393886 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -21,6 +21,7 @@
 
 #include <asm/gic.h>
 #include <xen/irq.h>
+#include <xen/grant_table.h>
 #include "kernel.h"
 
 static unsigned int __initdata opt_dom0_max_vcpus;
@@ -605,8 +606,8 @@ static int make_memory_node(const struct domain *d,
     return res;
 }
 
-static int make_hypervisor_node(struct domain *d,
-                                void *fdt, const struct dt_device_node *parent)
+static int make_hypervisor_node(const struct kernel_info *kinfo,
+                                const struct dt_device_node *parent)
 {
     const char compat[] =
         "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
@@ -615,9 +616,10 @@ static int make_hypervisor_node(struct domain *d,
     gic_interrupt_t intr;
     __be32 *cells;
     int res;
+    /* Convenience alias */
     int addrcells = dt_n_addr_cells(parent);
     int sizecells = dt_n_size_cells(parent);
-    paddr_t gnttab_start, gnttab_size;
+    void *fdt = kinfo->fdt;
 
     DPRINT("Create hypervisor node\n");
 
@@ -639,12 +641,9 @@ static int make_hypervisor_node(struct domain *d,
     if ( res )
         return res;
 
-    platform_dom0_gnttab(&gnttab_start, &gnttab_size);
-    DPRINT("  Grant table range: %#"PRIpaddr"-%#"PRIpaddr"\n",
-           gnttab_start, gnttab_start + gnttab_size);
     /* reg 0 is grant table space */
     cells = &reg[0];
-    dt_set_range(&cells, parent, gnttab_start, gnttab_size);
+    dt_set_range(&cells, parent, kinfo->gnttab_start, kinfo->gnttab_size);
     res = fdt_property(fdt, "reg", reg,
                        dt_cells_to_size(addrcells + sizecells));
     if ( res )
@@ -1189,7 +1188,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
 
     if ( node == dt_host )
     {
-        res = make_hypervisor_node(d, kinfo->fdt, node);
+        res = make_hypervisor_node(kinfo, node);
         if ( res )
             return res;
 
@@ -1365,6 +1364,36 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
         panic("Cannot fix up \"interrupts\" property of the hypervisor node");
 }
 
+static void __init find_gnttab_region(struct domain *d,
+                                      struct kernel_info *kinfo)
+{
+    /*
+     * The region used by Xen on the memory will never be mapped in DOM0
+     * memory layout. Therefore it can be used for the grant table.
+     *
+     * Only use the text section as it's always present and will contain
+     * enough space for a large grant table
+     */
+    kinfo->gnttab_start = __pa(_stext);
+    kinfo->gnttab_size = (_etext - _stext) & PAGE_MASK;
+
+    /* Make sure the grant table will fit in the region */
+    if ( (kinfo->gnttab_size >> PAGE_SHIFT) < max_grant_frames )
+        panic("Cannot find a space for the grant table region\n");
+
+#ifdef CONFIG_ARM_32
+    /*
+     * The gnttab region must be under 4GB in order to work with DOM0
+     * using short page table.
+     * In pratice it's always the case, but be safe
+     */
+    BUG_ON((kinfo->gnttab_start + kinfo->gnttab_size) > GB(4));
+#endif
+
+    printk("Grant table range: %#"PRIpaddr"-%#"PRIpaddr"\n",
+           kinfo->gnttab_start, kinfo->gnttab_start + kinfo->gnttab_size);
+}
+
 int construct_dom0(struct domain *d)
 {
     struct kernel_info kinfo = {};
@@ -1402,6 +1431,7 @@ int construct_dom0(struct domain *d)
 #endif
 
     allocate_memory(d, &kinfo);
+    find_gnttab_region(d, &kinfo);
 
     rc = prepare_dtb(d, &kinfo);
     if ( rc < 0 )
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 0050dfb..c1b07d4 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -22,6 +22,10 @@ struct kernel_info {
     /* kernel entry point */
     paddr_t entry;
 
+    /* grant table region */
+    paddr_t gnttab_start;
+    paddr_t gnttab_size;
+
     /* boot blob load addresses */
     const struct bootmodule *kernel_bootmodule, *initrd_bootmodule;
     paddr_t dtb_paddr;
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index 70c1388..0af6d57 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -147,20 +147,6 @@ bool_t platform_device_is_blacklisted(const struct dt_device_node *node)
     return (dt_match_node(blacklist, node) != NULL);
 }
 
-void platform_dom0_gnttab(paddr_t *start, paddr_t *size)
-{
-    if ( platform && platform->dom0_gnttab_size )
-    {
-        *start = platform->dom0_gnttab_start;
-        *size = platform->dom0_gnttab_size;
-    }
-    else
-    {
-        *start = 0xb0000000;
-        *size = 0x20000;
-    }
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index e7f2a43..e173fec 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -6,6 +6,4 @@ obj-$(CONFIG_ARM_32) += omap5.o
 obj-$(CONFIG_ARM_32) += sunxi.o
 obj-$(CONFIG_ARM_32) += rcar2.o
 obj-$(CONFIG_ARM_64) += seattle.o
-obj-$(CONFIG_ARM_64) += thunderx.o
 obj-$(CONFIG_ARM_64) += xgene-storm.o
-obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
index 42f7697..b221279 100644
--- a/xen/arch/arm/platforms/midway.c
+++ b/xen/arch/arm/platforms/midway.c
@@ -51,9 +51,6 @@ static const char * const midway_dt_compat[] __initconst =
 PLATFORM_START(midway, "CALXEDA MIDWAY")
     .compatible = midway_dt_compat,
     .reset = midway_reset,
-
-    .dom0_gnttab_start = 0xff800000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index e7bf30d..a49ba62 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -161,9 +161,6 @@ PLATFORM_START(omap5, "TI OMAP5")
     .specific_mapping = omap5_specific_mapping,
     .smp_init = omap5_smp_init,
     .cpu_up = cpu_up_send_sgi,
-
-    .dom0_gnttab_start = 0x4b000000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 PLATFORM_START(dra7, "TI DRA7")
@@ -171,9 +168,6 @@ PLATFORM_START(dra7, "TI DRA7")
     .init_time = omap5_init_time,
     .cpu_up = cpu_up_send_sgi,
     .smp_init = omap5_smp_init,
-
-    .dom0_gnttab_start = 0x4b000000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/rcar2.c b/xen/arch/arm/platforms/rcar2.c
index aef544c..bb25751 100644
--- a/xen/arch/arm/platforms/rcar2.c
+++ b/xen/arch/arm/platforms/rcar2.c
@@ -56,9 +56,6 @@ PLATFORM_START(rcar2, "Renesas R-Car Gen2")
     .compatible = rcar2_dt_compat,
     .cpu_up = cpu_up_send_sgi,
     .smp_init = rcar2_smp_init,
-
-    .dom0_gnttab_start = 0xc0000000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
index 6cc5362..86dce91 100644
--- a/xen/arch/arm/platforms/seattle.c
+++ b/xen/arch/arm/platforms/seattle.c
@@ -45,9 +45,6 @@ PLATFORM_START(seattle, "SEATTLE")
     .compatible = seattle_dt_compat,
     .reset      = seattle_system_reset,
     .poweroff   = seattle_system_off,
-
-    .dom0_gnttab_start = 0xe1700000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/sunxi.c b/xen/arch/arm/platforms/sunxi.c
index 89d8290..0ba7b3d 100644
--- a/xen/arch/arm/platforms/sunxi.c
+++ b/xen/arch/arm/platforms/sunxi.c
@@ -69,9 +69,6 @@ PLATFORM_START(sunxi, "Allwinner A20")
     .compatible = sunxi_dt_compat,
     .blacklist_dev = sunxi_blacklist_dev,
     .reset = sunxi_reset,
-
-    .dom0_gnttab_start = 0x01d00000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/thunderx.c b/xen/arch/arm/platforms/thunderx.c
deleted file mode 100644
index be6f24f..0000000
--- a/xen/arch/arm/platforms/thunderx.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * xen/arch/arm/platforms/thunderx.c
- *
- * Cavium Thunder specific settings
- *
- * Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
- * Copyright (c) 2015 Cavium Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <asm/platform.h>
-
-static const char * const thunderx_dt_compat[] __initconst =
-{
-    "cavium,thunder-88xx",
-    NULL
-};
-
-PLATFORM_START(thunderx, "THUNDERX")
-    .compatible = thunderx_dt_compat,
-    .dom0_gnttab_start = 0x40000000000,
-    .dom0_gnttab_size = 0x20000,
-PLATFORM_END
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/arm/platforms/vexpress.c b/xen/arch/arm/platforms/vexpress.c
index ce66935..8e6a4ea 100644
--- a/xen/arch/arm/platforms/vexpress.c
+++ b/xen/arch/arm/platforms/vexpress.c
@@ -176,8 +176,6 @@ PLATFORM_START(vexpress, "VERSATILE EXPRESS")
 #endif
     .reset = vexpress_reset,
     .blacklist_dev = vexpress_blacklist_dev,
-    .dom0_gnttab_start = 0x10000000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 1362bea..26754b5 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -266,9 +266,6 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM")
     .reset = xgene_storm_reset,
     .quirks = xgene_storm_quirks,
     .specific_mapping = xgene_storm_specific_mapping,
-
-    .dom0_gnttab_start = 0x1f800000,
-    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c b/xen/arch/arm/platforms/xilinx-zynqmp.c
deleted file mode 100644
index d03fb36..0000000
--- a/xen/arch/arm/platforms/xilinx-zynqmp.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * xen/arch/arm/platforms/xilinx-zynqmp.c
- *
- * Xilinx ZynqMP setup
- *
- * Copyright (c) 2015 Xilinx Inc.
- * Written by Edgar E. Iglesias <edgar.iglesias@xilinx.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <asm/platform.h>
-
-static const char * const zynqmp_dt_compat[] __initconst =
-{
-    "xlnx,zynqmp",
-    NULL
-};
-
-PLATFORM_START(xgene_storm, "Xilinx ZynqMP")
-    .compatible = zynqmp_dt_compat,
-    .dom0_gnttab_start = 0xf0000000,
-    .dom0_gnttab_size = 0x20000,
-PLATFORM_END
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index 746e126..f6736ca 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -37,12 +37,6 @@ struct platform_desc {
      * List of devices which must not pass-through to a guest
      */
     const struct dt_device_match *blacklist_dev;
-    /*
-     * The location of a region of physical address space which dom0
-     * can use for grant table mappings. If size is zero defaults to
-     * 0xb0000000-0xb0020000.
-     */
-    paddr_t dom0_gnttab_start, dom0_gnttab_size;
 };
 
 /*
@@ -63,7 +57,6 @@ void platform_poweroff(void);
 bool_t platform_has_quirk(uint32_t quirk);
 bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
 unsigned int platform_dom0_evtchn_ppi(void);
-void platform_dom0_gnttab(paddr_t *start, paddr_t *size);
 
 #define PLATFORM_START(_name, _namestr)                         \
 static const struct platform_desc  __plat_desc_##_name __used   \
-- 
2.1.4

Re: [RFC] xen/arm: Find automatically the gnttab region for DOM0

[Julien Grall]

Hi,

On 22/05/2015 01:38, Julien Grall wrote:
> Currently, the grant table region is hardcoded per-platform. When a new
> board is coming up, we have to check the spec in order to find a space
> in the memory layout free. Depending on the platform it may be tedious.
>
> A good candidate for the gnttab region is the one used by Xen binary as
> some part will never be mapped to the DOM0 address, MMIO are mapped 1:1
> and the RAM will be either:
>      - direct mapped: 1:1 mapping is used => no problem
>      - non direct mapped: Xen always relocates himself as high as possible
>      (limited to 4GB on ARM32) and the RAM bank are filled from the first
>      one. It's very unlikely that the gnttab region will overlap with the
>      RAM. Although for safety a check may be necessary when we will reenable
>      the option.
>
> Futhermore, there is plenty of space to contain a big gnttab, the default
> size is 32 frame (i.e 32KB) but it can be changed via a command option.

I miscalculate the size here. It should be 32 * 4KB = 128KB.

Regards,

-- 
Julien Grall

Re: [RFC] xen/arm: Find automatically the gnttab region for DOM0

[Ian Campbell]

On Fri, 2015-05-22 at 00:38 +0100, Julien Grall wrote:
> Note that on ARM64, the grant table region may be after 4GB (Xen is
> relocated to the highest address) using DOM0 32 bit with short page table
> may not work. Although, I don't think this is a big deal as device may not
> work and/or the RAM is too high due to the 1:1 mapping.

I agree.

> Currently, the grant table region is hardcoded per-platform. When a new
> board is coming up, we have to check the spec in order to find a space
> in the memory layout free. Depending on the platform it may be tedious.
> 
> A good candidate for the gnttab region is the one used by Xen binary as
> some part will never be mapped to the DOM0 address, MMIO are mapped 1:1
> and the RAM will be either:
>     - direct mapped: 1:1 mapping is used => no problem
>     - non direct mapped: Xen always relocates himself as high as possible
>     (limited to 4GB on ARM32) and the RAM bank are filled from the first
>     one. It's very unlikely that the gnttab region will overlap with the
>     RAM. Although for safety a check may be necessary when we will reenable
>     the option.
> 
> Futhermore, there is plenty of space to contain a big gnttab, the default

"Furthermore"

> @@ -1365,6 +1364,36 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
>          panic("Cannot fix up \"interrupts\" property of the hypervisor node");
>  }
>  
> +static void __init find_gnttab_region(struct domain *d,
> +                                      struct kernel_info *kinfo)
> +{
> +    /*
> +     * The region used by Xen on the memory will never be mapped in DOM0
> +     * memory layout. Therefore it can be used for the grant table.
> +     *
> +     * Only use the text section as it's always present and will contain
> +     * enough space for a large grant table
> +     */

Does the fact that XEN_PADDR_ALIGN == 2MB allow us to go larger? I
suspect not because we don't round up the size of the pseudo-bootmodule,
and in any case it'll have init sections in it.

So I agree that the text section is a reasonable choice.

> +    kinfo->gnttab_start = __pa(_stext);
> +    kinfo->gnttab_size = (_etext - _stext) & PAGE_MASK;
> +
> +    /* Make sure the grant table will fit in the region */
> +    if ( (kinfo->gnttab_size >> PAGE_SHIFT) < max_grant_frames )
> +        panic("Cannot find a space for the grant table region\n");
> +
> +#ifdef CONFIG_ARM_32
> +    /*
> +     * The gnttab region must be under 4GB in order to work with DOM0
> +     * using short page table.
> +     * In pratice it's always the case, but be safe

"practice"

I'd include "because Xen is always located below 4GB" in the final
sentence, to explain why in practice this is the case.

Ian.

Re: [RFC] xen/arm: Find automatically the gnttab region for DOM0

[Julien Grall]

Hi Ian,

On 17/06/15 11:26, Ian Campbell wrote:
> On Fri, 2015-05-22 at 00:38 +0100, Julien Grall wrote:
>> Note that on ARM64, the grant table region may be after 4GB (Xen is
>> relocated to the highest address) using DOM0 32 bit with short page table
>> may not work. Although, I don't think this is a big deal as device may not
>> work and/or the RAM is too high due to the 1:1 mapping.
> 
> I agree.
> 
>> Currently, the grant table region is hardcoded per-platform. When a new
>> board is coming up, we have to check the spec in order to find a space
>> in the memory layout free. Depending on the platform it may be tedious.
>>
>> A good candidate for the gnttab region is the one used by Xen binary as
>> some part will never be mapped to the DOM0 address, MMIO are mapped 1:1
>> and the RAM will be either:
>>     - direct mapped: 1:1 mapping is used => no problem
>>     - non direct mapped: Xen always relocates himself as high as possible
>>     (limited to 4GB on ARM32) and the RAM bank are filled from the first
>>     one. It's very unlikely that the gnttab region will overlap with the
>>     RAM. Although for safety a check may be necessary when we will reenable
>>     the option.
>>
>> Futhermore, there is plenty of space to contain a big gnttab, the default
> 
> "Furthermore"

My usual mistake :/

>> @@ -1365,6 +1364,36 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
>>          panic("Cannot fix up \"interrupts\" property of the hypervisor node");
>>  }
>>  
>> +static void __init find_gnttab_region(struct domain *d,
>> +                                      struct kernel_info *kinfo)
>> +{
>> +    /*
>> +     * The region used by Xen on the memory will never be mapped in DOM0
>> +     * memory layout. Therefore it can be used for the grant table.
>> +     *
>> +     * Only use the text section as it's always present and will contain
>> +     * enough space for a large grant table
>> +     */
> 
> Does the fact that XEN_PADDR_ALIGN == 2MB allow us to go larger? I
> suspect not because we don't round up the size of the pseudo-bootmodule,
> and in any case it'll have init sections in it.
> 
> So I agree that the text section is a reasonable choice.

The init section is located in the middle of the binary. We may be able
to get a larger region by including the other section (data, arch,...).
Although, I don't think it worth to do it now, the default size of the
grant is 128KB and the text section is 300KB.

> 
>> +    kinfo->gnttab_start = __pa(_stext);
>> +    kinfo->gnttab_size = (_etext - _stext) & PAGE_MASK;
>> +
>> +    /* Make sure the grant table will fit in the region */
>> +    if ( (kinfo->gnttab_size >> PAGE_SHIFT) < max_grant_frames )
>> +        panic("Cannot find a space for the grant table region\n");
>> +
>> +#ifdef CONFIG_ARM_32
>> +    /*
>> +     * The gnttab region must be under 4GB in order to work with DOM0
>> +     * using short page table.
>> +     * In pratice it's always the case, but be safe
> 
> "practice"
> 
> I'd include "because Xen is always located below 4GB" in the final
> sentence, to explain why in practice this is the case.

Ok. I will resend a new version.

Regards,

-- 
Julien Grall