Re: [Xen-devel] [PATCH v5 3/9] xen/x86: split Dom0 build into PV and PVHv2

[Roger Pau Monne]

On Thu, Jan 26, 2017 at 04:43:37AM -0700, Jan Beulich wrote:
> >>> On 19.01.17 at 18:29,  wrote:
> > --- a/xen/arch/x86/domain_build.c
> > +++ b/xen/arch/x86/domain_build.c
> > @@ -191,11 +191,40 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain 
> > *dom0)
> >  }
> >  
> >  #ifdef CONFIG_SHADOW_PAGING
> > -static bool_t __initdata opt_dom0_shadow;
> > +bool __initdata opt_dom0_shadow;
> >  boolean_param("dom0_shadow", opt_dom0_shadow);
> >  #else
> > -#define opt_dom0_shadow 0
> > +#define opt_dom0_shadow false
> 
> No need to duplicate the #define from the header here.
> 
> > @@ -1008,12 +1037,8 @@ int __init construct_dom0(
> >  paddr_t mpt_alloc;
> >  
> >  /* Sanity! */
> > -BUG_ON(d->domain_id != 0);
> > -BUG_ON(d->vcpu[0] == NULL);
> >  BUG_ON(v->is_initialised);
> 
> What about this last one?

Hm, not sure why I didn't move that last time, IIRC it triggered for PVHv2
vCPU, but that no longer seems to be the case, so I've moved it. Both fixed,
thanks.

Roger.

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

Re: [Xen-devel] [PATCH v5 3/9] xen/x86: split Dom0 build into PV and PVHv2

[Jan Beulich]

>>> On 19.01.17 at 18:29,  wrote:
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -191,11 +191,40 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain 
> *dom0)
>  }
>  
>  #ifdef CONFIG_SHADOW_PAGING
> -static bool_t __initdata opt_dom0_shadow;
> +bool __initdata opt_dom0_shadow;
>  boolean_param("dom0_shadow", opt_dom0_shadow);
>  #else
> -#define opt_dom0_shadow 0
> +#define opt_dom0_shadow false

No need to duplicate the #define from the header here.

> @@ -1008,12 +1037,8 @@ int __init construct_dom0(
>  paddr_t mpt_alloc;
>  
>  /* Sanity! */
> -BUG_ON(d->domain_id != 0);
> -BUG_ON(d->vcpu[0] == NULL);
>  BUG_ON(v->is_initialised);

What about this last one?

Jan


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

Re: [Xen-devel] [PATCH v5 3/9] xen/x86: split Dom0 build into PV and PVHv2

[Andrew Cooper]

On 23/01/17 12:58, Roger Pau Monne wrote:
> On Fri, Jan 20, 2017 at 07:03:10PM +, Andrew Cooper wrote:
>> On 19/01/17 17:29, Roger Pau Monne wrote:
>>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>>> index 0ccef1d..f52f269 100644
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -1545,6 +1545,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>>  if ( opt_dom0pvh )
>>>  domcr_flags |= DOMCRF_pvh | DOMCRF_hap;
>>>  
>>> +if ( dom0_pvh )
>>> +{
>>> +panic("Building a PVHv2 Dom0 is not yet supported.");
>> This would be cleaner immediately before the return 0 of
>> construct_dom0_pvh(), would it not?
>>
>> Otherwise, Reviewed-by: Andrew Cooper 
> I've moved the panic to the end of construct_dom0_pvh, and marked dom0_shadow
> as deprecated in the document, would you like me to keep the R-B?

Yeah - sounds fine.

~Andrew

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

Re: [Xen-devel] [PATCH v5 3/9] xen/x86: split Dom0 build into PV and PVHv2

[Roger Pau Monne]

On Fri, Jan 20, 2017 at 07:03:10PM +, Andrew Cooper wrote:
> On 19/01/17 17:29, Roger Pau Monne wrote:
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 0ccef1d..f52f269 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -1545,6 +1545,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> >  if ( opt_dom0pvh )
> >  domcr_flags |= DOMCRF_pvh | DOMCRF_hap;
> >  
> > +if ( dom0_pvh )
> > +{
> > +panic("Building a PVHv2 Dom0 is not yet supported.");
> 
> This would be cleaner immediately before the return 0 of
> construct_dom0_pvh(), would it not?
> 
> Otherwise, Reviewed-by: Andrew Cooper 

I've moved the panic to the end of construct_dom0_pvh, and marked dom0_shadow
as deprecated in the document, would you like me to keep the R-B?

Roger.

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

Re: [Xen-devel] [PATCH v5 3/9] xen/x86: split Dom0 build into PV and PVHv2

[Andrew Cooper]

On 20/01/17 19:13, Boris Ostrovsky wrote:
>> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
>> index 243df96..4d555b1 100644
>> --- a/xen/arch/x86/domain_build.c
>> +++ b/xen/arch/x86/domain_build.c
>> @@ -191,11 +191,40 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain 
>> *dom0)
>>  }
>>  
>>  #ifdef CONFIG_SHADOW_PAGING
>> -static bool_t __initdata opt_dom0_shadow;
>> +bool __initdata opt_dom0_shadow;
>>  boolean_param("dom0_shadow", opt_dom0_shadow);
> This (boolean_param) is not needed any longer.

Yes it is, for backwards compatibility.

Which is a good point.  The docs change should be modified still refer
to it, mark it as deprecated, and point at dom0=shadow.

>
>
>
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 0ccef1d..f52f269 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1545,6 +1545,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>  if ( opt_dom0pvh )
>>  domcr_flags |= DOMCRF_pvh | DOMCRF_hap;
>>  
>> +if ( dom0_pvh )
>> +{
>> +panic("Building a PVHv2 Dom0 is not yet supported.");
>> +domcr_flags |= DOMCRF_hvm |
>> +   ((hvm_funcs.hap_supported && !opt_dom0_shadow) ?
>> + DOMCRF_hap : 0);
>
> This probably has been discussed already --- are we going to support
> shadow paging for PVH?

ISTR agreeing that it should stay.  Not point excluding something which
already works and is already supported in other cases.

~Andrew

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

Re: [Xen-devel] [PATCH v5 3/9] xen/x86: split Dom0 build into PV and PVHv2

[Boris Ostrovsky]

> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> index 243df96..4d555b1 100644
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -191,11 +191,40 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain 
> *dom0)
>  }
>  
>  #ifdef CONFIG_SHADOW_PAGING
> -static bool_t __initdata opt_dom0_shadow;
> +bool __initdata opt_dom0_shadow;
>  boolean_param("dom0_shadow", opt_dom0_shadow);

This (boolean_param) is not needed any longer.



> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 0ccef1d..f52f269 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1545,6 +1545,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  if ( opt_dom0pvh )
>  domcr_flags |= DOMCRF_pvh | DOMCRF_hap;
>  
> +if ( dom0_pvh )
> +{
> +panic("Building a PVHv2 Dom0 is not yet supported.");
> +domcr_flags |= DOMCRF_hvm |
> +   ((hvm_funcs.hap_supported && !opt_dom0_shadow) ?
> + DOMCRF_hap : 0);


This probably has been discussed already --- are we going to support
shadow paging for PVH?

-boris




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

Re: [Xen-devel] [PATCH v5 3/9] xen/x86: split Dom0 build into PV and PVHv2

[Andrew Cooper]

On 19/01/17 17:29, Roger Pau Monne wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 0ccef1d..f52f269 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1545,6 +1545,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  if ( opt_dom0pvh )
>  domcr_flags |= DOMCRF_pvh | DOMCRF_hap;
>  
> +if ( dom0_pvh )
> +{
> +panic("Building a PVHv2 Dom0 is not yet supported.");

This would be cleaner immediately before the return 0 of
construct_dom0_pvh(), would it not?

Otherwise, Reviewed-by: Andrew Cooper 


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