x86: move vgc_flags to struct pv_vcpu
diff mbox series

Message ID ce92465a-8a54-e8b3-035f-46b695704169@suse.com
State New
Headers show
Series
  • x86: move vgc_flags to struct pv_vcpu
Related show

Commit Message

Jan Beulich Dec. 20, 2019, 1:55 p.m. UTC
There's been effectively no use of the field for HVM.

Also shrink the field to unsigned int, even if this doesn't immediately
yield any space benefit for the structure itself. The resulting 32-bit
padding slot can eventually be used for some other field. The change in
size makes accesses slightly more efficient though, as no REX.W prefix
is going to be needed anymore on the respective insns.

Mirror the HVM side change here (dropping of setting the field to
VGCF_online) also to Arm, on the assumption that it was cloned like
this originally. VGCF_online really should simply and consistently be
the guest view of the inverse of VPF_down, and hence needs representing
only in the get/set vCPU context interfaces.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Andrew Cooper Dec. 20, 2019, 2:39 p.m. UTC | #1
On 20/12/2019 13:55, Jan Beulich wrote:
> There's been effectively no use of the field for HVM.
>
> Also shrink the field to unsigned int, even if this doesn't immediately
> yield any space benefit for the structure itself. The resulting 32-bit
> padding slot can eventually be used for some other field. The change in
> size makes accesses slightly more efficient though, as no REX.W prefix
> is going to be needed anymore on the respective insns.
>
> Mirror the HVM side change here (dropping of setting the field to
> VGCF_online) also to Arm, on the assumption that it was cloned like
> this originally. VGCF_online really should simply and consistently be
> the guest view of the inverse of VPF_down, and hence needs representing
> only in the get/set vCPU context interfaces.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Julien Grall Dec. 23, 2019, 5:33 p.m. UTC | #2
Hi Jan,

On 20/12/2019 14:55, Jan Beulich wrote:
> There's been effectively no use of the field for HVM.
> 
> Also shrink the field to unsigned int, even if this doesn't immediately
> yield any space benefit for the structure itself. The resulting 32-bit
> padding slot can eventually be used for some other field. The change in
> size makes accesses slightly more efficient though, as no REX.W prefix
> is going to be needed anymore on the respective insns.
> 
> Mirror the HVM side change here (dropping of setting the field to
> VGCF_online) also to Arm, on the assumption that it was cloned like
> this originally. VGCF_online really should simply and consistently be
> the guest view of the inverse of VPF_down, and hence needs representing
> only in the get/set vCPU context interfaces.

But vPSCI is just a wrapper to get/set vCPU context interfaces. Your 
changes below will clearly break bring-up of secondary vCPUs on Arm.

This is because arch_set_guest_info() will rely on this flag to 
clear/set VPF_down in the pause flags.

So I think the line in arm/vpsci.c should be left alone.

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -78,7 +78,6 @@ static int do_common_cpu_on(register_t t
>           ctxt->user_regs.x0 = context_id;
>       }
>   #endif
> -    ctxt->flags = VGCF_online; >
>       domain_lock(d);
>       rc = arch_set_info_guest(v, ctxt);

Cheers,
Jan Beulich Dec. 27, 2019, 8:17 a.m. UTC | #3
On 23.12.2019 18:33, Julien Grall wrote:
> Hi Jan,
> 
> On 20/12/2019 14:55, Jan Beulich wrote:
>> There's been effectively no use of the field for HVM.
>>
>> Also shrink the field to unsigned int, even if this doesn't immediately
>> yield any space benefit for the structure itself. The resulting 32-bit
>> padding slot can eventually be used for some other field. The change in
>> size makes accesses slightly more efficient though, as no REX.W prefix
>> is going to be needed anymore on the respective insns.
>>
>> Mirror the HVM side change here (dropping of setting the field to
>> VGCF_online) also to Arm, on the assumption that it was cloned like
>> this originally. VGCF_online really should simply and consistently be
>> the guest view of the inverse of VPF_down, and hence needs representing
>> only in the get/set vCPU context interfaces.
> 
> But vPSCI is just a wrapper to get/set vCPU context interfaces. Your 
> changes below will clearly break bring-up of secondary vCPUs on Arm.
> 
> This is because arch_set_guest_info() will rely on this flag to 
> clear/set VPF_down in the pause flags.
> 
> So I think the line in arm/vpsci.c should be left alone.

Oh, I see - I didn't notice this (ab)use despite ...

>> --- a/xen/arch/arm/vpsci.c
>> +++ b/xen/arch/arm/vpsci.c
>> @@ -78,7 +78,6 @@ static int do_common_cpu_on(register_t t
>>           ctxt->user_regs.x0 = context_id;
>>       }
>>   #endif
>> -    ctxt->flags = VGCF_online; >
>>       domain_lock(d);
>>       rc = arch_set_info_guest(v, ctxt);

... it actually being in context. Thanks for noticing.

Jan
Julien Grall Dec. 27, 2019, 11:27 a.m. UTC | #4
Hi Jan,

On Fri, 27 Dec 2019, 09:22 Jan Beulich, <JBeulich@suse.com> wrote:

> On 23.12.2019 18:33, Julien Grall wrote:
> > Hi Jan,
> >
> > On 20/12/2019 14:55, Jan Beulich wrote:
> >> There's been effectively no use of the field for HVM.
> >>
> >> Also shrink the field to unsigned int, even if this doesn't immediately
> >> yield any space benefit for the structure itself. The resulting 32-bit
> >> padding slot can eventually be used for some other field. The change in
> >> size makes accesses slightly more efficient though, as no REX.W prefix
> >> is going to be needed anymore on the respective insns.
> >>
> >> Mirror the HVM side change here (dropping of setting the field to
> >> VGCF_online) also to Arm, on the assumption that it was cloned like
> >> this originally. VGCF_online really should simply and consistently be
> >> the guest view of the inverse of VPF_down, and hence needs representing
> >> only in the get/set vCPU context interfaces.
> >
> > But vPSCI is just a wrapper to get/set vCPU context interfaces. Your
> > changes below will clearly break bring-up of secondary vCPUs on Arm.
> >
> > This is because arch_set_guest_info() will rely on this flag to
> > clear/set VPF_down in the pause flags.
> >
> > So I think the line in arm/vpsci.c should be left alone.
>
> Oh, I see - I didn't notice this (ab)use despite ...
>

Out of Interest, why do you think it is an abuse here and not in the
toolstack?

How do you suggest to improve it? I can add it in my improvement list for
Arm.

Cheers,
Jan Beulich Dec. 27, 2019, 12:14 p.m. UTC | #5
On 27.12.2019 12:27, Julien Grall wrote:
> Hi Jan,
> 
> On Fri, 27 Dec 2019, 09:22 Jan Beulich, <JBeulich@suse.com> wrote:
> 
>> On 23.12.2019 18:33, Julien Grall wrote:
>>> Hi Jan,
>>>
>>> On 20/12/2019 14:55, Jan Beulich wrote:
>>>> There's been effectively no use of the field for HVM.
>>>>
>>>> Also shrink the field to unsigned int, even if this doesn't immediately
>>>> yield any space benefit for the structure itself. The resulting 32-bit
>>>> padding slot can eventually be used for some other field. The change in
>>>> size makes accesses slightly more efficient though, as no REX.W prefix
>>>> is going to be needed anymore on the respective insns.
>>>>
>>>> Mirror the HVM side change here (dropping of setting the field to
>>>> VGCF_online) also to Arm, on the assumption that it was cloned like
>>>> this originally. VGCF_online really should simply and consistently be
>>>> the guest view of the inverse of VPF_down, and hence needs representing
>>>> only in the get/set vCPU context interfaces.
>>>
>>> But vPSCI is just a wrapper to get/set vCPU context interfaces. Your
>>> changes below will clearly break bring-up of secondary vCPUs on Arm.
>>>
>>> This is because arch_set_guest_info() will rely on this flag to
>>> clear/set VPF_down in the pause flags.
>>>
>>> So I think the line in arm/vpsci.c should be left alone.
>>
>> Oh, I see - I didn't notice this (ab)use despite ...
>>
> 
> Out of Interest, why do you think it is an abuse here and not in the
> toolstack?
> 
> How do you suggest to improve it? I can add it in my improvement list for
> Arm.

Oh, "abuse" was about the arch_set_guest_info() use, not the use of
the flag by the tool stack.

Jan
Julien Grall Jan. 3, 2020, 10:56 a.m. UTC | #6
Hi,

On 27/12/2019 12:14, Jan Beulich wrote:
> On 27.12.2019 12:27, Julien Grall wrote:
>> Hi Jan,
>>
>> On Fri, 27 Dec 2019, 09:22 Jan Beulich, <JBeulich@suse.com> wrote:
>>
>>> On 23.12.2019 18:33, Julien Grall wrote:
>>>> Hi Jan,
>>>>
>>>> On 20/12/2019 14:55, Jan Beulich wrote:
>>>>> There's been effectively no use of the field for HVM.
>>>>>
>>>>> Also shrink the field to unsigned int, even if this doesn't immediately
>>>>> yield any space benefit for the structure itself. The resulting 32-bit
>>>>> padding slot can eventually be used for some other field. The change in
>>>>> size makes accesses slightly more efficient though, as no REX.W prefix
>>>>> is going to be needed anymore on the respective insns.
>>>>>
>>>>> Mirror the HVM side change here (dropping of setting the field to
>>>>> VGCF_online) also to Arm, on the assumption that it was cloned like
>>>>> this originally. VGCF_online really should simply and consistently be
>>>>> the guest view of the inverse of VPF_down, and hence needs representing
>>>>> only in the get/set vCPU context interfaces.
>>>>
>>>> But vPSCI is just a wrapper to get/set vCPU context interfaces. Your
>>>> changes below will clearly break bring-up of secondary vCPUs on Arm.
>>>>
>>>> This is because arch_set_guest_info() will rely on this flag to
>>>> clear/set VPF_down in the pause flags.
>>>>
>>>> So I think the line in arm/vpsci.c should be left alone.
>>>
>>> Oh, I see - I didn't notice this (ab)use despite ...
>>>
>>
>> Out of Interest, why do you think it is an abuse here and not in the
>> toolstack?
>>
>> How do you suggest to improve it? I can add it in my improvement list for
>> Arm.
> 
> Oh, "abuse" was about the arch_set_guest_info() use, not the use of
> the flag by the tool stack.

I may have read incorrectly your e-mail, although I think my questions 
about why this is an abuse and how do you suggest to improve are still 
relevant.

Cheers,
Jan Beulich Jan. 3, 2020, 11:05 a.m. UTC | #7
On 03.01.2020 11:56, Julien Grall wrote:
> Hi,
> 
> On 27/12/2019 12:14, Jan Beulich wrote:
>> On 27.12.2019 12:27, Julien Grall wrote:
>>> Hi Jan,
>>>
>>> On Fri, 27 Dec 2019, 09:22 Jan Beulich, <JBeulich@suse.com> wrote:
>>>
>>>> On 23.12.2019 18:33, Julien Grall wrote:
>>>>> Hi Jan,
>>>>>
>>>>> On 20/12/2019 14:55, Jan Beulich wrote:
>>>>>> There's been effectively no use of the field for HVM.
>>>>>>
>>>>>> Also shrink the field to unsigned int, even if this doesn't immediately
>>>>>> yield any space benefit for the structure itself. The resulting 32-bit
>>>>>> padding slot can eventually be used for some other field. The change in
>>>>>> size makes accesses slightly more efficient though, as no REX.W prefix
>>>>>> is going to be needed anymore on the respective insns.
>>>>>>
>>>>>> Mirror the HVM side change here (dropping of setting the field to
>>>>>> VGCF_online) also to Arm, on the assumption that it was cloned like
>>>>>> this originally. VGCF_online really should simply and consistently be
>>>>>> the guest view of the inverse of VPF_down, and hence needs representing
>>>>>> only in the get/set vCPU context interfaces.
>>>>>
>>>>> But vPSCI is just a wrapper to get/set vCPU context interfaces. Your
>>>>> changes below will clearly break bring-up of secondary vCPUs on Arm.
>>>>>
>>>>> This is because arch_set_guest_info() will rely on this flag to
>>>>> clear/set VPF_down in the pause flags.
>>>>>
>>>>> So I think the line in arm/vpsci.c should be left alone.
>>>>
>>>> Oh, I see - I didn't notice this (ab)use despite ...
>>>>
>>>
>>> Out of Interest, why do you think it is an abuse here and not in the
>>> toolstack?
>>>
>>> How do you suggest to improve it? I can add it in my improvement list for
>>> Arm.
>>
>> Oh, "abuse" was about the arch_set_guest_info() use, not the use of
>> the flag by the tool stack.
> 
> I may have read incorrectly your e-mail, although I think my questions 
> about why this is an abuse and how do you suggest to improve are still 
> relevant.

arch_set_info_guest() is intended to be used for exactly one purpose
- vCPU context initialization via hypercall. With this, and _without_
me knowing anything about PSCI, it _looks_ to me to be an abuse. I'd
expect there to be something in x86 that could be used for
comparison, and whatever that is - it doesn't need a similar extra
use of arch_set_info_guest(). (As a result, I don't see how I could
reasonably give a concrete suggestion towards improvement. In fact I
may be entirely wrong with my feeling of this being an abuse in the
first place.)

Jan
Julien Grall Jan. 3, 2020, 11:19 a.m. UTC | #8
Hi,

On 03/01/2020 11:05, Jan Beulich wrote:
> On 03.01.2020 11:56, Julien Grall wrote:
>> Hi,
>>
>> On 27/12/2019 12:14, Jan Beulich wrote:
>>> On 27.12.2019 12:27, Julien Grall wrote:
>>>> Hi Jan,
>>>>
>>>> On Fri, 27 Dec 2019, 09:22 Jan Beulich, <JBeulich@suse.com> wrote:
>>>>
>>>>> On 23.12.2019 18:33, Julien Grall wrote:
>>>>>> Hi Jan,
>>>>>>
>>>>>> On 20/12/2019 14:55, Jan Beulich wrote:
>>>>>>> There's been effectively no use of the field for HVM.
>>>>>>>
>>>>>>> Also shrink the field to unsigned int, even if this doesn't immediately
>>>>>>> yield any space benefit for the structure itself. The resulting 32-bit
>>>>>>> padding slot can eventually be used for some other field. The change in
>>>>>>> size makes accesses slightly more efficient though, as no REX.W prefix
>>>>>>> is going to be needed anymore on the respective insns.
>>>>>>>
>>>>>>> Mirror the HVM side change here (dropping of setting the field to
>>>>>>> VGCF_online) also to Arm, on the assumption that it was cloned like
>>>>>>> this originally. VGCF_online really should simply and consistently be
>>>>>>> the guest view of the inverse of VPF_down, and hence needs representing
>>>>>>> only in the get/set vCPU context interfaces.
>>>>>>
>>>>>> But vPSCI is just a wrapper to get/set vCPU context interfaces. Your
>>>>>> changes below will clearly break bring-up of secondary vCPUs on Arm.
>>>>>>
>>>>>> This is because arch_set_guest_info() will rely on this flag to
>>>>>> clear/set VPF_down in the pause flags.
>>>>>>
>>>>>> So I think the line in arm/vpsci.c should be left alone.
>>>>>
>>>>> Oh, I see - I didn't notice this (ab)use despite ...
>>>>>
>>>>
>>>> Out of Interest, why do you think it is an abuse here and not in the
>>>> toolstack?
>>>>
>>>> How do you suggest to improve it? I can add it in my improvement list for
>>>> Arm.
>>>
>>> Oh, "abuse" was about the arch_set_guest_info() use, not the use of
>>> the flag by the tool stack.
>>
>> I may have read incorrectly your e-mail, although I think my questions
>> about why this is an abuse and how do you suggest to improve are still
>> relevant.
> 
> arch_set_info_guest() is intended to be used for exactly one purpose
> - vCPU context initialization via hypercall. With this, and _without_
> me knowing anything about PSCI, it _looks_ to me to be an abuse. 

PSCI (Power State Coordination Interface) is a generic way to manage the 
power on your plarform (e.g CPU bring-up).

The CPU_ON call will actually initialize the vCPU context and then start 
the vCPU.

Whilst, this is not a Xen specific interface, they are still hypercall 
based.

> I'd
> expect there to be something in x86 that could be used for
> comparison, and whatever that is - it doesn't need a similar extra
> use of arch_set_info_guest().

How do you manage secondary CPUs on HVM/PVH guest?

Cheers,
Jan Beulich Jan. 3, 2020, 11:31 a.m. UTC | #9
On 03.01.2020 12:19, Julien Grall wrote:
> How do you manage secondary CPUs on HVM/PVH guest?

Secondary CPUs have architectural state they start with, and
there's very little control an OS has over initial register
state: There's just an 8-bit value specifying (part of) the
address the CPU should start executing from. All other
registers get set to hard coded values. And that 8-bit value
is part of the IPI message the primary CPU sends to the AP
to be brought up (i.e. there's no hypercall involved here).

For PVH, a variant of the normal PV model of starting vCPU-s
gets used, i.e. via VCPUOP_initialise.

Jan
Julien Grall Jan. 3, 2020, 11:48 a.m. UTC | #10
Hi Jan,

Thank you for the information.

On 03/01/2020 11:31, Jan Beulich wrote:
> On 03.01.2020 12:19, Julien Grall wrote:
>> How do you manage secondary CPUs on HVM/PVH guest?
> 
> Secondary CPUs have architectural state they start with, and
> there's very little control an OS has over initial register
> state: There's just an 8-bit value specifying (part of) the
> address the CPU should start executing from. All other
> registers get set to hard coded values. And that 8-bit value
> is part of the IPI message the primary CPU sends to the AP
> to be brought up (i.e. there's no hypercall involved here).

Do you have any pointer to this code? Can a CPU be turned off afterwards 
and then boot again?

> 
> For PVH, a variant of the normal PV model of starting vCPU-s
> gets used, i.e. via VCPUOP_initialise.

In the case of PSCI, I think it is between the two. We are using a 
generic hypercall, yet most of the state is fixed.

But as the guest OS may run a CPU for a while, turning off and then boot 
again, we need to be able to set the state again. Hence, the 
arch_set_guest_info() is quite convenient to use to reset the CPU state.

Cheers,
Jan Beulich Jan. 3, 2020, 12:11 p.m. UTC | #11
On 03.01.2020 12:48, Julien Grall wrote:
> Hi Jan,
> 
> Thank you for the information.
> 
> On 03/01/2020 11:31, Jan Beulich wrote:
>> On 03.01.2020 12:19, Julien Grall wrote:
>>> How do you manage secondary CPUs on HVM/PVH guest?
>>
>> Secondary CPUs have architectural state they start with, and
>> there's very little control an OS has over initial register
>> state: There's just an 8-bit value specifying (part of) the
>> address the CPU should start executing from. All other
>> registers get set to hard coded values. And that 8-bit value
>> is part of the IPI message the primary CPU sends to the AP
>> to be brought up (i.e. there's no hypercall involved here).
> 
> Do you have any pointer to this code?

Perhaps best look at how Xen _uses_ this mechanism:
smpboot.c:wakeup_secondary_cpu().

> Can a CPU be turned off afterwards 
> and then boot again?

Yes, by sending it an INIT signal (as done by e.g.
wakeup_secondary_cpu()).

Jan

Patch
diff mbox series

--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -78,7 +78,6 @@  static int do_common_cpu_on(register_t t
         ctxt->user_regs.x0 = context_id;
     }
 #endif
-    ctxt->flags = VGCF_online;
 
     domain_lock(d);
     rc = arch_set_info_guest(v, ctxt);
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -896,6 +896,8 @@  int arch_set_info_guest(
         if ( ((c(ldt_base) & (PAGE_SIZE - 1)) != 0) ||
              (c(ldt_ents) > 8192) )
             return -EINVAL;
+
+        v->arch.pv.vgc_flags = flags;
     }
 
     v->arch.flags |= TF_kernel_mode;
@@ -908,8 +910,6 @@  int arch_set_info_guest(
          !is_hvm_domain(d) && !is_pv_32bit_domain(d) )
         v->arch.flags &= ~TF_kernel_mode;
 
-    v->arch.vgc_flags = flags;
-
     vcpu_setup_fpu(v, v->arch.xsave_area,
                    flags & VGCF_I387_VALID ? &c.nat->fpu_ctxt : NULL,
                    FCW_DEFAULT);
@@ -1488,7 +1488,7 @@  static void load_segments(struct vcpu *n
                 domain_crash(n->domain);
             }
 
-            if ( n->arch.vgc_flags & VGCF_failsafe_disables_events )
+            if ( n->arch.pv.vgc_flags & VGCF_failsafe_disables_events )
                 vcpu_info(n, evtchn_upcall_mask) = 1;
 
             regs->entry_vector |= TRAP_syscall;
@@ -1527,7 +1527,7 @@  static void load_segments(struct vcpu *n
             domain_crash(n->domain);
         }
 
-        if ( n->arch.vgc_flags & VGCF_failsafe_disables_events )
+        if ( n->arch.pv.vgc_flags & VGCF_failsafe_disables_events )
             vcpu_info(n, evtchn_upcall_mask) = 1;
 
         regs->entry_vector |= TRAP_syscall;
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1500,7 +1500,10 @@  void arch_get_info_guest(struct vcpu *v,
 #define c(fld) (!compat ? (c.nat->fld) : (c.cmp->fld))
 
     memcpy(&c.nat->fpu_ctxt, v->arch.fpu_ctxt, sizeof(c.nat->fpu_ctxt));
-    c(flags = v->arch.vgc_flags & ~(VGCF_i387_valid|VGCF_in_kernel));
+    if ( is_pv_domain(d) )
+        c(flags = v->arch.pv.vgc_flags & ~(VGCF_i387_valid|VGCF_in_kernel));
+    else
+        c(flags = 0);
     if ( v->fpu_initialised )
         c(flags |= VGCF_i387_valid);
     if ( !(v->pause_flags & VPF_down) )
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1151,8 +1151,6 @@  static int hvm_load_cpu_ctxt(struct doma
     v->arch.dr6   = ctxt.dr6;
     v->arch.dr7   = ctxt.dr7;
 
-    v->arch.vgc_flags = VGCF_online;
-
     /* Auxiliary processors should be woken immediately. */
     v->is_initialised = 1;
     clear_bit(_VPF_down, &v->pause_flags);
@@ -3864,8 +3862,6 @@  void hvm_vcpu_reset_state(struct vcpu *v
         v->arch.xsave_area->xsave_hdr.xstate_bv = 0;
     vcpu_setup_fpu(v, v->arch.xsave_area, NULL, FCW_RESET);
 
-    v->arch.vgc_flags = VGCF_online;
-
     arch_vcpu_regs_init(v);
     v->arch.user_regs.rip = ip;
 
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -82,17 +82,17 @@  static long register_guest_callback(stru
     case CALLBACKTYPE_failsafe:
         curr->arch.pv.failsafe_callback_eip = reg->address;
         if ( reg->flags & CALLBACKF_mask_events )
-            curr->arch.vgc_flags |= VGCF_failsafe_disables_events;
+            curr->arch.pv.vgc_flags |= VGCF_failsafe_disables_events;
         else
-            curr->arch.vgc_flags &= ~VGCF_failsafe_disables_events;
+            curr->arch.pv.vgc_flags &= ~VGCF_failsafe_disables_events;
         break;
 
     case CALLBACKTYPE_syscall:
         curr->arch.pv.syscall_callback_eip = reg->address;
         if ( reg->flags & CALLBACKF_mask_events )
-            curr->arch.vgc_flags |= VGCF_syscall_disables_events;
+            curr->arch.pv.vgc_flags |= VGCF_syscall_disables_events;
         else
-            curr->arch.vgc_flags &= ~VGCF_syscall_disables_events;
+            curr->arch.pv.vgc_flags &= ~VGCF_syscall_disables_events;
         break;
 
     case CALLBACKTYPE_syscall32:
@@ -226,9 +226,9 @@  static long compat_register_guest_callba
         curr->arch.pv.failsafe_callback_cs = reg->address.cs;
         curr->arch.pv.failsafe_callback_eip = reg->address.eip;
         if ( reg->flags & CALLBACKF_mask_events )
-            curr->arch.vgc_flags |= VGCF_failsafe_disables_events;
+            curr->arch.pv.vgc_flags |= VGCF_failsafe_disables_events;
         else
-            curr->arch.vgc_flags &= ~VGCF_failsafe_disables_events;
+            curr->arch.pv.vgc_flags &= ~VGCF_failsafe_disables_events;
         break;
 
     case CALLBACKTYPE_syscall32:
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -69,7 +69,7 @@  void __dummy__(void)
     OFFSET(VCPU_kernel_sp, struct vcpu, arch.pv.kernel_sp);
     OFFSET(VCPU_kernel_ss, struct vcpu, arch.pv.kernel_ss);
     OFFSET(VCPU_iopl, struct vcpu, arch.pv.iopl);
-    OFFSET(VCPU_guest_context_flags, struct vcpu, arch.vgc_flags);
+    OFFSET(VCPU_guest_context_flags, struct vcpu, arch.pv.vgc_flags);
     OFFSET(VCPU_cr3, struct vcpu, arch.cr3);
     OFFSET(VCPU_arch_msrs, struct vcpu, arch.msrs);
     OFFSET(VCPU_nmi_pending, struct vcpu, nmi_pending);
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -476,6 +476,8 @@  struct pv_vcpu
     /* map_domain_page() mapping cache. */
     struct mapcache_vcpu mapcache;
 
+    unsigned int vgc_flags;
+
     struct trap_info *trap_ctxt;
 
     unsigned long gdt_frames[FIRST_RESERVED_GDT_PAGE];
@@ -542,7 +544,6 @@  struct arch_vcpu
      */
 
     void              *fpu_ctxt;
-    unsigned long      vgc_flags;
     struct cpu_user_regs user_regs;
 
     /* Debug registers. */