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

Re: [Xen-devel] [PATCH v2 for-4.10] x86/mm: Make PV linear pagetables optional



>>> On 18.10.17 at 12:51, <george.dunlap@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -37,6 +37,26 @@ source "arch/Kconfig"
>  config PV
>       def_bool y
>  
> +config PV_LINEAR_PT
> +       bool "Support for PV linear pagetables"
> +       depends on PV
> +       default y
> +       ---help---
> +         Linear pagetables (also called "recursive pagetables") refers
> +         to the practice of a guest operating system having pagetable
> +         entries pointing to other pagetables of the same level (i.e.,
> +         allowing L2 PTEs to point to other L2 pages).  Some operating
> +         systems use it as a simple way to consisently map the current
> +         process's pagetables into its own virtual address space.
> +
> +         Linux and MiniOS don't use this technique.  NetBSD and Novell
> +         Netware do; there may be other custom operating systems which
> +         do.  If you're certain you don't plan on having PV guests
> +         which use this feature, turning it off can reduce the attack
> +         surface.
> +
> +         If unsure, say Y.
> +
>  config HVM
>       def_bool y

Note how the options in context use tab indentation. Granted
there are other examples of space indentation in this file, but
at least they're using 8 spaces (except of course of the help
text), while you're using 7.

> @@ -2320,6 +2353,7 @@ static int _put_page_type(struct page_info *page, bool 
> preemptible,
>                  break;
>              }
>  
> +#ifdef CONFIG_PV_LINEAR_PT
>              if ( ptpg && PGT_type_equal(x, ptpg->u.inuse.type_info) )
>              {
>                  /*
> @@ -2334,6 +2368,9 @@ static int _put_page_type(struct page_info *page, bool 
> preemptible,
>                  ASSERT(ptpg->linear_pt_count > 0);
>                  ptpg = NULL;
>              }
> +#else /* CONFIG_PV_LINEAR_PT */
> +            BUG_ON(ptpg && PGT_type_equal(x, ptpg->u.inuse.type_info));
> +#endif

Along the lines of my most recent reply to v1 (which I realize I
did send only after v2 had arrived), I'm not really certain about
the usefulness of the preprocessor conditionals - I'd prefer if
we went without them, but I can live with them if you strongly
think they're better than the alternative. If you keep them,
please convert the BUG_ON() to ASSERT() though, to be in
line with the #ifdef side.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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