Hi Andrew,
On 08/15/2018 02:08 PM, Andrew Cooper wrote:
On 15/08/18 14:04, Julien Grall wrote:
Hi Andrew,
On 08/13/2018 11:01 AM, Andrew Cooper wrote:
... rather than setting the limits up after domain_create() has
completed.
This removes all the gnttab infrastructure for calculating the number
of dom0
grant frames, opting instead to require the dom0 construction code to
pass a
sane value in via the configuration.
In practice, this now means that there is never a partially
constructed grant
table for a reference-able domain.
Signed-off-by: Andrew Cooper <[email protected]>
---
CC: Jan Beulich <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Julien Grall <[email protected]>
CC: Wei Liu <[email protected]>
v2:
* Split/rearrange to avoid the post-domain-create error path.
---
xen/arch/arm/domain_build.c | 3 ++-
xen/arch/arm/setup.c | 12 ++++++++++++
xen/arch/x86/setup.c | 3 +++
xen/common/domain.c | 3 ++-
xen/common/grant_table.c | 16 +++-------------
xen/include/asm-arm/grant_table.h | 12 ------------
xen/include/asm-x86/grant_table.h | 5 -----
xen/include/xen/grant_table.h | 6 ++----
8 files changed, 24 insertions(+), 36 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1351572..737e0f3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2079,7 +2079,8 @@ static void __init find_gnttab_region(struct
domain *d,
* enough space for a large grant table
*/
kinfo->gnttab_start = __pa(_stext);
- kinfo->gnttab_size = gnttab_dom0_frames() << PAGE_SHIFT;
+ kinfo->gnttab_size = min_t(unsigned int, opt_max_grant_frames,
+ PFN_DOWN(_etext - _stext)) <<
PAGE_SHIFT;
I agree with Jan's comment on v1 that there is a risk someone will
update the size here but ...
#ifdef CONFIG_ARM_32
/*
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 45f3841..3d3b30c 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -20,6 +20,7 @@
#include <xen/compile.h>
#include <xen/device_tree.h>
#include <xen/domain_page.h>
+#include <xen/grant_table.h>
#include <xen/types.h>
#include <xen/string.h>
#include <xen/serial.h>
@@ -693,6 +694,17 @@ void __init start_xen(unsigned long
boot_phys_offset,
struct domain *dom0;
struct xen_domctl_createdomain dom0_cfg = {
.max_evtchn_port = -1,
+
+ /*
+ * 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
+ */
+ .max_grant_frames = min_t(unsigned int, opt_max_grant_frames,
+ PFN_DOWN(_etext - _stext)),
... not here. So I would prefer if we either keep an helper to find
the size of pass that size around to domain_build. Do we store the
size in the domain information?
I have to admit that I'm somewhat perplexed by ARM's
find_gnttab_region(), and I'm not sure why it exists.
Dom0 is using the host memory layout that may differ between platforms.
So there is not a region address that would fit everyone.
This function is here to find at boot a suitable region in the layout
where the OS can map the grant-table. The result will be written in the
firmware table.
The value is available from d->grant_table.max_grant_frames but ISTR
finding that the order of construction meant that it wasn't available
when needed (although this was all from code inspection, so I could very
easily be wrong).
I think it should be fine for Dom0 as find_gnttab_region is called from
construct_dom0 and d->grant_table.max_grant_frames would be set before
via domain_create().
Assuming d->grant_table.max_grant_frames can only be 0 before
initialization, I would potentially add a
BUG_ON(!d->grant_table.max_grant_frames) to make sure this always stay
like that.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
