Re: [Xen-devel] [PATCH v3] x86: clarify shadow paging Dom0 support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/03/17 14:34, Wei Liu wrote:
> Classic PV shadow paging Dom0 has been broken for years, and can't
> possibly be configured after 4045953.
>
> PVH shadow paging Dom0 should still be possible.
>
> Change the code and documentation to clarify that.
>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> v3:
> 1. don't panic
>
> v2:
> 1. simplify need_paging
> 2. move panic to construct_dom0
>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  docs/misc/xen-command-line.markdown |  8 ++------
>  xen/arch/x86/dom0_build.c           | 11 ++++++++---
>  xen/arch/x86/pv/dom0_build.c        |  5 -----
>  3 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index bdbdb8a53b..9eb85d68b5 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -652,11 +652,6 @@ restrictions set up here. Note that the values to be 
> specified here are
>  ACPI PXM ones, not Xen internal node numbers. `relaxed` sets up vCPU
>  affinities to prefer but be not limited to the specified node(s).
>  
> -### dom0\_shadow
> -> `= <boolean>`
> -
> -This option is deprecated, please use `dom0=shadow` instead.
> -
>  ### dom0\_vcpus\_pin
>  > `= <boolean>`
>  
> @@ -679,7 +674,8 @@ Flag that makes a dom0 boot in PVHv2 mode.
>  
>  > Default: `false`
>  
> -Flag that makes a dom0 use shadow paging.
> +Flag that makes a dom0 use shadow paging. Only works when "pvh" is
> +enabled.
>  
>  ### dtuart (ARM)
>  > `= path [:options]`
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 20221b5e32..8597d43b11 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -174,7 +174,6 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
>  
>  #ifdef CONFIG_SHADOW_PAGING
>  bool __initdata opt_dom0_shadow;
> -boolean_param("dom0_shadow", opt_dom0_shadow);
>  #endif
>  bool __initdata dom0_pvh;
>  
> @@ -252,8 +251,8 @@ unsigned long __init dom0_compute_nr_pages(
>              avail -= max_pdx >> s;
>      }
>  
> -    need_paging = is_hvm_domain(d) ? !iommu_hap_pt_share || 
> !paging_mode_hap(d)
> -                                   : opt_dom0_shadow;
> +    need_paging = is_hvm_domain(d) &&
> +        (!iommu_hap_pt_share || !paging_mode_hap(d));
>      for ( ; ; need_paging = 0 )
>      {
>          nr_pages = dom0_nrpages;
> @@ -456,6 +455,12 @@ int __init construct_dom0(struct domain *d, const 
> module_t *image,
>  
>      process_pending_softirqs();
>  
> +    if ( opt_dom0_shadow && !dom0_pvh )
> +    {
> +        opt_dom0_shadow = false;
> +        printk("Shadow Dom0 requires PVH. Option ignored.");
> +    }

This needs to be inside a #ifdef CONFIG_SHADOW_PAGING to avoid breaking
the build.

With the other corrections as well, Reviewed-by: Andrew Cooper
<andrew.cooper@xxxxxxxxxx>

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