[v7,1/6] x86/domain: remove the 'oos_off' flag

Commit Message

Paul Durrant Aug. 30, 2019, 8:29 a.m. UTC

The flag is not needed since the domain 'options' can now be tested
directly.

Signed-off-by: Paul Durrant <[e-mail]>
Reviewed-by: Jan Beulich <[e-mail]>
---
Cc: Tim Deegan <[e-mail]>
Cc: George Dunlap <[e-mail]>
Cc: Andrew Cooper <[e-mail]>
Cc: Wei Liu <[e-mail]>
Cc: "Roger Pau Monné" <[e-mail]>

v3:
 - Force 'oos_off' to be set for PV guests (to avoid call to
   is_hvm_domain() except in ASSERT)
 - Dropped Tim's A-b because of the change

v2:
 - Move some of the hunks from patch #3
 - Also update the definition of shadow_domain_init() in none.c
---
 xen/arch/x86/mm/paging.c        |  2 +-
 xen/arch/x86/mm/shadow/common.c |  7 ++++---
 xen/arch/x86/mm/shadow/none.c   |  2 +-
 xen/common/domain.c             | 16 ++++++++++++----
 xen/include/asm-x86/domain.h    |  1 -
 xen/include/asm-x86/shadow.h    |  2 +-
 6 files changed, 19 insertions(+), 11 deletions(-)

Comments

Jan Beulich Sept. 2, 2019, 12:33 p.m. UTC | #1

On 30.08.2019 10:29, Paul Durrant wrote:
> The flag is not needed since the domain 'options' can now be tested
> directly.
> 
> Signed-off-by: Paul Durrant <[e-mail]>
> Reviewed-by: Jan Beulich <[e-mail]>
> ---
> Cc: Tim Deegan <[e-mail]>
> Cc: George Dunlap <[e-mail]>
> Cc: Andrew Cooper <[e-mail]>
> Cc: Wei Liu <[e-mail]>
> Cc: "Roger Pau Monné" <[e-mail]>
> 
> v3:
>  - Force 'oos_off' to be set for PV guests (to avoid call to
>    is_hvm_domain() except in ASSERT)
>  - Dropped Tim's A-b because of the change

I've been debating with myself whether to not wait any longer for
Tim to re-instate his ack, but now that I've looked again ...

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -313,11 +313,19 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>          return -EINVAL;
>      }
>  
> -    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
> -         (config->flags & XEN_DOMCTL_CDF_hap) )
> +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
>      {
> -        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> -        return -EINVAL;
> +        if ( config->flags & XEN_DOMCTL_CDF_hap )
> +        {
> +            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> +            return -EINVAL;
> +        }
> +
> +        /*
> +         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
> +         * for HVM guests.
> +         */
> +        config->flags |= XEN_DOMCTL_CDF_oos_off;

... I wonder whether this last part wouldn't better belong into
x86's arch_sanitise_domain_config(). Arm, to the contrary, should
force/require the bit to be uniformly off.

Jan

Paul Durrant Sept. 2, 2019, 1:06 p.m. UTC | #2

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> Sent: 02 September 2019 13:34
> To: Paul Durrant <[e-mail]>
> Cc: [e-mail]; Andrew Cooper <[e-mail]>; Roger Pau Monne
> <[e-mail]>; George Dunlap <[e-mail]>; Tim (Xen.org) <[e-mail]>; Wei Liu
> <[e-mail]>
> Subject: Re: [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag
> 
> On 30.08.2019 10:29, Paul Durrant wrote:
> > The flag is not needed since the domain 'options' can now be tested
> > directly.
> >
> > Signed-off-by: Paul Durrant <[e-mail]>
> > Reviewed-by: Jan Beulich <[e-mail]>
> > ---
> > Cc: Tim Deegan <[e-mail]>
> > Cc: George Dunlap <[e-mail]>
> > Cc: Andrew Cooper <[e-mail]>
> > Cc: Wei Liu <[e-mail]>
> > Cc: "Roger Pau Monné" <[e-mail]>
> >
> > v3:
> >  - Force 'oos_off' to be set for PV guests (to avoid call to
> >    is_hvm_domain() except in ASSERT)
> >  - Dropped Tim's A-b because of the change
> 
> I've been debating with myself whether to not wait any longer for
> Tim to re-instate his ack, but now that I've looked again ...
> 
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -313,11 +313,19 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> >          return -EINVAL;
> >      }
> >
> > -    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
> > -         (config->flags & XEN_DOMCTL_CDF_hap) )
> > +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
> >      {
> > -        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> > -        return -EINVAL;
> > +        if ( config->flags & XEN_DOMCTL_CDF_hap )
> > +        {
> > +            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> > +            return -EINVAL;
> > +        }
> > +
> > +        /*
> > +         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
> > +         * for HVM guests.
> > +         */
> > +        config->flags |= XEN_DOMCTL_CDF_oos_off;
> 
> ... I wonder whether this last part wouldn't better belong into
> x86's arch_sanitise_domain_config(). Arm, to the contrary, should
> force/require the bit to be uniformly off.
> 

I'm sure I had a reason for doing it like this but it's sufficiently long ago now that I've forgotten what it was*. Would it be ok to take the code as-is and I'll investigate whether this can be tidied up?

[ * I suspect it was concern over breaking existing tool-stacks by requiring them to set a flag that previously they did not need to, but I'm not sure that was the only reason ]

  Paul




> Jan

Jan Beulich Sept. 2, 2019, 1:45 p.m. UTC | #3

On 02.09.2019 15:06, Paul Durrant wrote:
>> From: Jan Beulich <[e-mail]>
>> Sent: 02 September 2019 13:34
>>
>> On 30.08.2019 10:29, Paul Durrant wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -313,11 +313,19 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>>>          return -EINVAL;
>>>      }
>>>
>>> -    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
>>> -         (config->flags & XEN_DOMCTL_CDF_hap) )
>>> +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
>>>      {
>>> -        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
>>> -        return -EINVAL;
>>> +        if ( config->flags & XEN_DOMCTL_CDF_hap )
>>> +        {
>>> +            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
>>> +            return -EINVAL;
>>> +        }
>>> +
>>> +        /*
>>> +         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
>>> +         * for HVM guests.
>>> +         */
>>> +        config->flags |= XEN_DOMCTL_CDF_oos_off;
>>
>> ... I wonder whether this last part wouldn't better belong into
>> x86's arch_sanitise_domain_config(). Arm, to the contrary, should
>> force/require the bit to be uniformly off.
>>
> 
> I'm sure I had a reason for doing it like this but it's sufficiently long
> ago now that I've forgotten what it was*. Would it be ok to take the code
> as-is and I'll investigate whether this can be tidied up?

Well, with this pending question I'm less inclined to stop waiting for
the outstanding acks.

> [ * I suspect it was concern over breaking existing tool-stacks by
> requiring them to set a flag that previously they did not need to, but
> I'm not sure that was the only reason ]

Seems rather unlikely to me, as there wouldn't be any difference (from
tool stack perspective) if the adjustment was done by per-arch code.

Jan

Paul Durrant Sept. 2, 2019, 1:59 p.m. UTC | #4

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> Sent: 02 September 2019 14:46
> To: Paul Durrant <[e-mail]>
> Cc: Andrew Cooper <[e-mail]>; George Dunlap <[e-mail]>; Roger Pau
> Monne <[e-mail]>; [e-mail]; Tim (Xen.org) <[e-mail]>; WeiLiu
> <[e-mail]>
> Subject: Re: [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag
> 
> On 02.09.2019 15:06, Paul Durrant wrote:
> >> From: Jan Beulich <[e-mail]>
> >> Sent: 02 September 2019 13:34
> >>
> >> On 30.08.2019 10:29, Paul Durrant wrote:
> >>> --- a/xen/common/domain.c
> >>> +++ b/xen/common/domain.c
> >>> @@ -313,11 +313,19 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> >>>          return -EINVAL;
> >>>      }
> >>>
> >>> -    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
> >>> -         (config->flags & XEN_DOMCTL_CDF_hap) )
> >>> +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
> >>>      {
> >>> -        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> >>> -        return -EINVAL;
> >>> +        if ( config->flags & XEN_DOMCTL_CDF_hap )
> >>> +        {
> >>> +            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> >>> +            return -EINVAL;
> >>> +        }
> >>> +
> >>> +        /*
> >>> +         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
> >>> +         * for HVM guests.
> >>> +         */
> >>> +        config->flags |= XEN_DOMCTL_CDF_oos_off;
> >>
> >> ... I wonder whether this last part wouldn't better belong into
> >> x86's arch_sanitise_domain_config(). Arm, to the contrary, should
> >> force/require the bit to be uniformly off.
> >>
> >
> > I'm sure I had a reason for doing it like this but it's sufficiently long
> > ago now that I've forgotten what it was*. Would it be ok to take the code
> > as-is and I'll investigate whether this can be tidied up?
> 
> Well, with this pending question I'm less inclined to stop waiting for
> the outstanding acks.
> 
> > [ * I suspect it was concern over breaking existing tool-stacks by
> > requiring them to set a flag that previously they did not need to, but
> > I'm not sure that was the only reason ]
> 
> Seems rather unlikely to me, as there wouldn't be any difference (from
> tool stack perspective) if the adjustment was done by per-arch code.
> 

Ok, if you feel strongly about it I'll move the hunk.

  Paul

> Jan

Jan Beulich Sept. 2, 2019, 2:11 p.m. UTC | #5

On 02.09.2019 15:59, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <[e-mail]>
>> Sent: 02 September 2019 14:46
>> To: Paul Durrant <[e-mail]>
>> Cc: Andrew Cooper <[e-mail]>; George Dunlap <[e-mail]>; Roger Pau
>> Monne <[e-mail]>; [e-mail]; Tim (Xen.org) <[e-mail]>; WeiLiu
>> <[e-mail]>
>> Subject: Re: [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag
>>
>> On 02.09.2019 15:06, Paul Durrant wrote:
>>>> From: Jan Beulich <[e-mail]>
>>>> Sent: 02 September 2019 13:34
>>>>
>>>> On 30.08.2019 10:29, Paul Durrant wrote:
>>>>> --- a/xen/common/domain.c
>>>>> +++ b/xen/common/domain.c
>>>>> @@ -313,11 +313,19 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>>>>>          return -EINVAL;
>>>>>      }
>>>>>
>>>>> -    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
>>>>> -         (config->flags & XEN_DOMCTL_CDF_hap) )
>>>>> +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
>>>>>      {
>>>>> -        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
>>>>> -        return -EINVAL;
>>>>> +        if ( config->flags & XEN_DOMCTL_CDF_hap )
>>>>> +        {
>>>>> +            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
>>>>> +            return -EINVAL;
>>>>> +        }
>>>>> +
>>>>> +        /*
>>>>> +         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
>>>>> +         * for HVM guests.
>>>>> +         */
>>>>> +        config->flags |= XEN_DOMCTL_CDF_oos_off;
>>>>
>>>> ... I wonder whether this last part wouldn't better belong into
>>>> x86's arch_sanitise_domain_config(). Arm, to the contrary, should
>>>> force/require the bit to be uniformly off.
>>>>
>>>
>>> I'm sure I had a reason for doing it like this but it's sufficiently long
>>> ago now that I've forgotten what it was*. Would it be ok to take the code
>>> as-is and I'll investigate whether this can be tidied up?
>>
>> Well, with this pending question I'm less inclined to stop waiting for
>> the outstanding acks.
>>
>>> [ * I suspect it was concern over breaking existing tool-stacks by
>>> requiring them to set a flag that previously they did not need to, but
>>> I'm not sure that was the only reason ]
>>
>> Seems rather unlikely to me, as there wouldn't be any difference (from
>> tool stack perspective) if the adjustment was done by per-arch code.
> 
> Ok, if you feel strongly about it I'll move the hunk.

Well, wait - not the hunk. The HAP part should remain in common code
imo. The OOS part wants doing differently in x86 and Arm code.

Jan

Paul Durrant Sept. 2, 2019, 2:21 p.m. UTC | #6

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> Sent: 02 September 2019 15:12
> To: Paul Durrant <[e-mail]>
> Cc: Andrew Cooper <[e-mail]>; George Dunlap <[e-mail]>; Roger Pau
> Monne <[e-mail]>; [e-mail]; Tim (Xen.org) <[e-mail]>; WeiLiu
> <[e-mail]>
> Subject: Re: [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag
> 
> On 02.09.2019 15:59, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <[e-mail]>
> >> Sent: 02 September 2019 14:46
> >> To: Paul Durrant <[e-mail]>
> >> Cc: Andrew Cooper <[e-mail]>; George Dunlap <[e-mail]>; Roger Pau
> >> Monne <[e-mail]>; [e-mail]; Tim (Xen.org) <[e-mail]>; WeiLiu
> >> <[e-mail]>
> >> Subject: Re: [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag
> >>
> >> On 02.09.2019 15:06, Paul Durrant wrote:
> >>>> From: Jan Beulich <[e-mail]>
> >>>> Sent: 02 September 2019 13:34
> >>>>
> >>>> On 30.08.2019 10:29, Paul Durrant wrote:
> >>>>> --- a/xen/common/domain.c
> >>>>> +++ b/xen/common/domain.c
> >>>>> @@ -313,11 +313,19 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> >>>>>          return -EINVAL;
> >>>>>      }
> >>>>>
> >>>>> -    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
> >>>>> -         (config->flags & XEN_DOMCTL_CDF_hap) )
> >>>>> +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
> >>>>>      {
> >>>>> -        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> >>>>> -        return -EINVAL;
> >>>>> +        if ( config->flags & XEN_DOMCTL_CDF_hap )
> >>>>> +        {
> >>>>> +            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> >>>>> +            return -EINVAL;
> >>>>> +        }
> >>>>> +
> >>>>> +        /*
> >>>>> +         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
> >>>>> +         * for HVM guests.
> >>>>> +         */
> >>>>> +        config->flags |= XEN_DOMCTL_CDF_oos_off;
> >>>>
> >>>> ... I wonder whether this last part wouldn't better belong into
> >>>> x86's arch_sanitise_domain_config(). Arm, to the contrary, should
> >>>> force/require the bit to be uniformly off.
> >>>>
> >>>
> >>> I'm sure I had a reason for doing it like this but it's sufficiently long
> >>> ago now that I've forgotten what it was*. Would it be ok to take the code
> >>> as-is and I'll investigate whether this can be tidied up?
> >>
> >> Well, with this pending question I'm less inclined to stop waiting for
> >> the outstanding acks.
> >>
> >>> [ * I suspect it was concern over breaking existing tool-stacks by
> >>> requiring them to set a flag that previously they did not need to, but
> >>> I'm not sure that was the only reason ]
> >>
> >> Seems rather unlikely to me, as there wouldn't be any difference (from
> >> tool stack perspective) if the adjustment was done by per-arch code.
> >
> > Ok, if you feel strongly about it I'll move the hunk.
> 
> Well, wait - not the hunk. The HAP part should remain in common code
> imo. The OOS part wants doing differently in x86 and Arm code.
> 

Yes, the hap part stays put. The 'oos_off' part moves to x86 and arm can be left alone because it already rejects flags != (hvm | hap).

  Paul

> Jan

Jan Beulich Sept. 2, 2019, 2:53 p.m. UTC | #7

On 02.09.2019 16:21, Paul Durrant wrote:
> Yes, the hap part stays put. The 'oos_off' part moves to x86 and arm can
> be left alone because it already rejects flags != (hvm | hap).

But it may better reject the OOS flag _despite_ having only HVM guests,
as long as there's no shadow mode there in the first place.

Jan

Paul Durrant Sept. 2, 2019, 3:13 p.m. UTC | #8

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> Sent: 02 September 2019 15:54
> To: Paul Durrant <[e-mail]>
> Cc: Andrew Cooper <[e-mail]>; George Dunlap <[e-mail]>; Roger Pau
> Monne <[e-mail]>; [e-mail]; Tim (Xen.org) <[e-mail]>; WeiLiu
> <[e-mail]>
> Subject: Re: [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag
> 
> On 02.09.2019 16:21, Paul Durrant wrote:
> > Yes, the hap part stays put. The 'oos_off' part moves to x86 and arm can
> > be left alone because it already rejects flags != (hvm | hap).
> 
> But it may better reject the OOS flag _despite_ having only HVM guests,
> as long as there's no shadow mode there in the first place.
> 

The flag will be rejected. As I said (in abbreviated form), the test in the ARM code is:

if (config->flags != (XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap) )
	error...

So, any attempt to set XEN_DOMCTL_CDF_oos_off will already cause an error.

  Paul

> Jan

Jan Beulich Sept. 2, 2019, 3:33 p.m. UTC | #9

On 02.09.2019 17:13, Paul Durrant wrote:
>> From: Jan Beulich <[e-mail]>
>> Sent: 02 September 2019 15:54
>>
>> On 02.09.2019 16:21, Paul Durrant wrote:
>>> Yes, the hap part stays put. The 'oos_off' part moves to x86 and arm can
>>> be left alone because it already rejects flags != (hvm | hap).
>>
>> But it may better reject the OOS flag _despite_ having only HVM guests,
>> as long as there's no shadow mode there in the first place.
>>
> 
> The flag will be rejected. As I said (in abbreviated form), the test in the ARM code is:
> 
> if (config->flags != (XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap) )
> 	error...
> 
> So, any attempt to set XEN_DOMCTL_CDF_oos_off will already cause an error.

Oh, I'm sorry, I didn't recall that they accept only a single value
here. I'm sorry for the noise then.

Jan

Patch

diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 097a27f608..69aa228e46 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -653,7 +653,7 @@  int paging_domain_init(struct domain *d)
     if ( hap_enabled(d) )
         hap_domain_init(d);
     else
-        rc = shadow_domain_init(d, d->options);
+        rc = shadow_domain_init(d);
 
     return rc;
 }
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index c0d4a27287..9463794059 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -46,7 +46,7 @@  static void sh_clean_dirty_bitmap(struct domain *);
 
 /* Set up the shadow-specific parts of a domain struct at start of day.
  * Called for every domain from arch_domain_create() */
-int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
+int shadow_domain_init(struct domain *d)
 {
     static const struct log_dirty_ops sh_ops = {
         .enable  = sh_enable_log_dirty,
@@ -62,7 +62,6 @@  int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
     d->arch.paging.shadow.oos_active = 0;
-    d->arch.paging.shadow.oos_off = domcr_flags & XEN_DOMCTL_CDF_oos_off;
 #endif
     d->arch.paging.shadow.pagetable_dying_op = 0;
 
@@ -2528,11 +2527,13 @@  static void sh_update_paging_modes(struct vcpu *v)
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
     /* We need to check that all the vcpus have paging enabled to
      * unsync PTs. */
-    if ( is_hvm_domain(d) && !d->arch.paging.shadow.oos_off )
+    if ( !(d->options & XEN_DOMCTL_CDF_oos_off) )
     {
         int pe = 1;
         struct vcpu *vptr;
 
+        ASSERT(is_hvm_domain(d));
+
         for_each_vcpu(d, vptr)
         {
             if ( !hvm_paging_enabled(vptr) )
diff --git a/xen/arch/x86/mm/shadow/none.c b/xen/arch/x86/mm/shadow/none.c
index a70888bd98..2fddf4274c 100644
--- a/xen/arch/x86/mm/shadow/none.c
+++ b/xen/arch/x86/mm/shadow/none.c
@@ -18,7 +18,7 @@  static void _clean_dirty_bitmap(struct domain *d)
     ASSERT(is_pv_domain(d));
 }
 
-int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
+int shadow_domain_init(struct domain *d)
 {
     static const struct log_dirty_ops sh_none_ops = {
         .enable  = _enable_log_dirty,
diff --git a/xen/common/domain.c b/xen/common/domain.c
index e9d2c613e0..eb13807af9 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -313,11 +313,19 @@  static int sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
-    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
-         (config->flags & XEN_DOMCTL_CDF_hap) )
+    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
     {
-        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
-        return -EINVAL;
+        if ( config->flags & XEN_DOMCTL_CDF_hap )
+        {
+            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
+            return -EINVAL;
+        }
+
+        /*
+         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
+         * for HVM guests.
+         */
+        config->flags |= XEN_DOMCTL_CDF_oos_off;
     }
 
     return arch_sanitise_domain_config(config);
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 9f3afd12bc..7cebfa4fb9 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -115,7 +115,6 @@  struct shadow_domain {
 
     /* OOS */
     bool_t oos_active;
-    bool_t oos_off;
 
     /* Has this domain ever used HVMOP_pagetable_dying? */
     bool_t pagetable_dying_op;
diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index f29f0f652b..8ebb89c027 100644
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -49,7 +49,7 @@ 
 
 /* Set up the shadow-specific parts of a domain struct at start of day.
  * Called from paging_domain_init(). */
-int shadow_domain_init(struct domain *d, unsigned int domcr_flags);
+int shadow_domain_init(struct domain *d);
 
 /* Setup the shadow-specific parts of a vcpu struct. It is called by
  * paging_vcpu_init() in paging.c */