Re: [PATCH v2 06/12] xen/gnttab: Pass max_{grant, maptrack}_frames into grant_table_create()

["Roger Pau Monné" <roger.pau@citrix.com>]

On Mon, Aug 13, 2018 at 11:01:03AM +0100, 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 <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> 
> 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;
>  
>  #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)),

You have this calculation here and in the chunk above. Maybe you want
to keep gnttab_dom0_max (or something similar with the min included)
in order to avoid open coding this calculation twice?

The rest LGTM:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel