[v7,7/9] common/domain: add a domain context record for shared_info...

Commit Message

Paul Durrant Aug. 18, 2020, 10:30 a.m. UTC

From: Paul Durrant <[e-mail]>

... and update xen-domctx to dump some information describing the record.

NOTE: Processing of the content during restore is currently limited to
      PV domains, and matches processing of the PV-only SHARED_INFO record
      done by libxc. All content is, however, saved such that restore
      processing can be modified in future without requiring a new record
      format.

Signed-off-by: Paul Durrant <[e-mail]>
---
Cc: Ian Jackson <[e-mail]>
Cc: Wei Liu <[e-mail]>
Cc: Andrew Cooper <[e-mail]>
Cc: George Dunlap <[e-mail]>
Cc: Jan Beulich <[e-mail]>
Cc: Julien Grall <[e-mail]>
Cc: Stefano Stabellini <[e-mail]>

v7:
 - Only restore vcpu_info and arch sub-structures for PV domains, to match
   processing of SHARED_INFO in xc_sr_restore_x86_pv.c
 - Use additional option to domain_load_end() to ignore the record for
   HVM domains

v6:
 - Only save compat_shared_info buffer if has_32bit_shinfo is set
 - Validate flags field in load handler

v5:
 - Addressed comments from Julien

v4:
 - Addressed comments from Jan

v3:
 - Actually dump some of the content of shared_info

v2:
 - Drop the header change to define a 'Xen' page size and instead use a
   variable length struct now that the framework makes this is feasible
 - Guard use of 'has_32bit_shinfo' in common code with CONFIG_COMPAT
---
 tools/misc/xen-domctx.c   |  78 ++++++++++++++++++++++++++
 xen/common/domain.c       | 114 ++++++++++++++++++++++++++++++++++++++
 xen/include/public/save.h |  13 ++++-
 3 files changed, 204 insertions(+), 1 deletion(-)

Comments

Jan Beulich Aug. 26, 2020, 1:57 p.m. UTC | #1

On 18.08.2020 12:30, Paul Durrant wrote:
> v7:
>  - Only restore vcpu_info and arch sub-structures for PV domains, to match
>    processing of SHARED_INFO in xc_sr_restore_x86_pv.c

Since you point out this original logic, ...

> +static int load_shared_info(struct domain *d, struct domain_context *c)
> +{
> +    struct domain_shared_info_context ctxt;
> +    size_t hdr_size = offsetof(typeof(ctxt), buffer);
> +    unsigned int i;
> +    int rc;
> +
> +    rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, &i);
> +    if ( rc )
> +        return rc;
> +
> +    if ( i ) /* expect only a single instance */
> +        return -ENXIO;
> +
> +    rc = domain_load_data(c, &ctxt, hdr_size);
> +    if ( rc )
> +        return rc;
> +
> +    if ( ctxt.buffer_size > sizeof(shared_info_t) ||
> +         (ctxt.flags & ~DOMAIN_SAVE_32BIT_SHINFO) )
> +        return -EINVAL;
> +
> +    if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
> +    {
> +#ifdef CONFIG_COMPAT
> +        has_32bit_shinfo(d) = true;
> +#else
> +        return -EINVAL;
> +#endif
> +    }
> +
> +    if ( is_pv_domain(d) )
> +    {
> +        shared_info_t *shinfo = xmalloc(shared_info_t);
> +
> +        rc = domain_load_data(c, shinfo, sizeof(*shinfo));
> +        if ( rc )
> +        {
> +            xfree(shinfo);
> +            return rc;
> +        }
> +
> +#ifdef CONFIG_COMPAT
> +        if ( has_32bit_shinfo(d) )
> +        {
> +            memcpy(&d->shared_info->compat.vcpu_info,
> +                   &shinfo->compat.vcpu_info,
> +                   sizeof(d->shared_info->compat.vcpu_info));
> +            memcpy(&d->shared_info->compat.arch,
> +                   &shinfo->compat.arch,
> +                   sizeof(d->shared_info->compat.vcpu_info));
> +        }
> +        else
> +        {
> +            memcpy(&d->shared_info->native.vcpu_info,
> +                   &shinfo->native.vcpu_info,
> +                   sizeof(d->shared_info->native.vcpu_info));
> +            memcpy(&d->shared_info->native.arch,
> +                   &shinfo->native.arch,
> +                   sizeof(d->shared_info->native.arch));
> +        }
> +#else
> +        memcpy(&d->shared_info->vcpu_info,
> +               &shinfo->vcpu_info,
> +               sizeof(d->shared_info->vcpu_info));
> +        memcpy(&d->shared_info->arch,
> +               &shinfo->arch,
> +               sizeof(d->shared_info->shared));
> +#endif

... where does the rest of that logic (resetting of
arch.pfn_to_mfn_frame_list_list, evtchn_pending, evtchn_mask, and
evtchn_pending_sel) get done? Or why is it not needed anymore?

Jan

Paul Durrant Sept. 4, 2020, 5:29 p.m. UTC | #2

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> Sent: 26 August 2020 14:58
> To: Paul Durrant <[e-mail]>
> Cc: [e-mail]; Paul Durrant <[e-mail]>; Ian Jackson
> <[e-mail]>; Wei Liu <[e-mail]>; Andrew Cooper <[e-mail]>; George
> Dunlap <[e-mail]>; Julien Grall <[e-mail]>; Stefano Stabellini
> <[e-mail]>
> Subject: Re: [PATCH v7 7/9] common/domain: add a domain context record for shared_info...
> 
> On 18.08.2020 12:30, Paul Durrant wrote:
> > v7:
> >  - Only restore vcpu_info and arch sub-structures for PV domains, to match
> >    processing of SHARED_INFO in xc_sr_restore_x86_pv.c
> 
> Since you point out this original logic, ...
> 
> > +static int load_shared_info(struct domain *d, struct domain_context *c)
> > +{
> > +    struct domain_shared_info_context ctxt;
> > +    size_t hdr_size = offsetof(typeof(ctxt), buffer);
> > +    unsigned int i;
> > +    int rc;
> > +
> > +    rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, &i);
> > +    if ( rc )
> > +        return rc;
> > +
> > +    if ( i ) /* expect only a single instance */
> > +        return -ENXIO;
> > +
> > +    rc = domain_load_data(c, &ctxt, hdr_size);
> > +    if ( rc )
> > +        return rc;
> > +
> > +    if ( ctxt.buffer_size > sizeof(shared_info_t) ||
> > +         (ctxt.flags & ~DOMAIN_SAVE_32BIT_SHINFO) )
> > +        return -EINVAL;
> > +
> > +    if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
> > +    {
> > +#ifdef CONFIG_COMPAT
> > +        has_32bit_shinfo(d) = true;
> > +#else
> > +        return -EINVAL;
> > +#endif
> > +    }
> > +
> > +    if ( is_pv_domain(d) )
> > +    {
> > +        shared_info_t *shinfo = xmalloc(shared_info_t);
> > +
> > +        rc = domain_load_data(c, shinfo, sizeof(*shinfo));
> > +        if ( rc )
> > +        {
> > +            xfree(shinfo);
> > +            return rc;
> > +        }
> > +
> > +#ifdef CONFIG_COMPAT
> > +        if ( has_32bit_shinfo(d) )
> > +        {
> > +            memcpy(&d->shared_info->compat.vcpu_info,
> > +                   &shinfo->compat.vcpu_info,
> > +                   sizeof(d->shared_info->compat.vcpu_info));
> > +            memcpy(&d->shared_info->compat.arch,
> > +                   &shinfo->compat.arch,
> > +                   sizeof(d->shared_info->compat.vcpu_info));
> > +        }
> > +        else
> > +        {
> > +            memcpy(&d->shared_info->native.vcpu_info,
> > +                   &shinfo->native.vcpu_info,
> > +                   sizeof(d->shared_info->native.vcpu_info));
> > +            memcpy(&d->shared_info->native.arch,
> > +                   &shinfo->native.arch,
> > +                   sizeof(d->shared_info->native.arch));
> > +        }
> > +#else
> > +        memcpy(&d->shared_info->vcpu_info,
> > +               &shinfo->vcpu_info,
> > +               sizeof(d->shared_info->vcpu_info));
> > +        memcpy(&d->shared_info->arch,
> > +               &shinfo->arch,
> > +               sizeof(d->shared_info->shared));
> > +#endif
> 
> ... where does the rest of that logic (resetting of
> arch.pfn_to_mfn_frame_list_list, evtchn_pending, evtchn_mask, and
> evtchn_pending_sel) get done? Or why is it not needed anymore?

The resetting logic is still in xc_sr_restore_x86_pv.c (see patch #6). It's going to need to stay there anyway to deal with older streams so I made it common to both cases; it seems slightly separate from restoring the shared info.

  Paul

> 
> Jan

Jan Beulich Sept. 7, 2020, 7:01 a.m. UTC | #3

On 04.09.2020 19:29, Paul Durrant wrote:
>> From: Jan Beulich <[e-mail]>
>> Sent: 26 August 2020 14:58
>>
>> On 18.08.2020 12:30, Paul Durrant wrote:
>>> v7:
>>>  - Only restore vcpu_info and arch sub-structures for PV domains, to match
>>>    processing of SHARED_INFO in xc_sr_restore_x86_pv.c
>>
>> Since you point out this original logic, ...
>>
>>> +static int load_shared_info(struct domain *d, struct domain_context *c)
>>> +{
>>> +    struct domain_shared_info_context ctxt;
>>> +    size_t hdr_size = offsetof(typeof(ctxt), buffer);
>>> +    unsigned int i;
>>> +    int rc;
>>> +
>>> +    rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, &i);
>>> +    if ( rc )
>>> +        return rc;
>>> +
>>> +    if ( i ) /* expect only a single instance */
>>> +        return -ENXIO;
>>> +
>>> +    rc = domain_load_data(c, &ctxt, hdr_size);
>>> +    if ( rc )
>>> +        return rc;
>>> +
>>> +    if ( ctxt.buffer_size > sizeof(shared_info_t) ||
>>> +         (ctxt.flags & ~DOMAIN_SAVE_32BIT_SHINFO) )
>>> +        return -EINVAL;
>>> +
>>> +    if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
>>> +    {
>>> +#ifdef CONFIG_COMPAT
>>> +        has_32bit_shinfo(d) = true;
>>> +#else
>>> +        return -EINVAL;
>>> +#endif
>>> +    }
>>> +
>>> +    if ( is_pv_domain(d) )
>>> +    {
>>> +        shared_info_t *shinfo = xmalloc(shared_info_t);
>>> +
>>> +        rc = domain_load_data(c, shinfo, sizeof(*shinfo));
>>> +        if ( rc )
>>> +        {
>>> +            xfree(shinfo);
>>> +            return rc;
>>> +        }
>>> +
>>> +#ifdef CONFIG_COMPAT
>>> +        if ( has_32bit_shinfo(d) )
>>> +        {
>>> +            memcpy(&d->shared_info->compat.vcpu_info,
>>> +                   &shinfo->compat.vcpu_info,
>>> +                   sizeof(d->shared_info->compat.vcpu_info));
>>> +            memcpy(&d->shared_info->compat.arch,
>>> +                   &shinfo->compat.arch,
>>> +                   sizeof(d->shared_info->compat.vcpu_info));
>>> +        }
>>> +        else
>>> +        {
>>> +            memcpy(&d->shared_info->native.vcpu_info,
>>> +                   &shinfo->native.vcpu_info,
>>> +                   sizeof(d->shared_info->native.vcpu_info));
>>> +            memcpy(&d->shared_info->native.arch,
>>> +                   &shinfo->native.arch,
>>> +                   sizeof(d->shared_info->native.arch));
>>> +        }
>>> +#else
>>> +        memcpy(&d->shared_info->vcpu_info,
>>> +               &shinfo->vcpu_info,
>>> +               sizeof(d->shared_info->vcpu_info));
>>> +        memcpy(&d->shared_info->arch,
>>> +               &shinfo->arch,
>>> +               sizeof(d->shared_info->shared));
>>> +#endif
>>
>> ... where does the rest of that logic (resetting of
>> arch.pfn_to_mfn_frame_list_list, evtchn_pending, evtchn_mask, and
>> evtchn_pending_sel) get done? Or why is it not needed anymore?
> 
> The resetting logic is still in xc_sr_restore_x86_pv.c (see patch #6).
> It's going to need to stay there anyway to deal with older streams so
> I made it common to both cases; it seems slightly separate from
> restoring the shared info.

I guess I at least don't fully agree: The resetting is part of restoring,
as it effectively determines which parts are restored and which parts
are simply set (not truly reset, but I agree the perception may change
depending on whose position you take). Hence at the very least this
aspect wants clearly spelling out in the description, I think. But
really I'd prefer if for old streams libxc took care of (all of) it,
and if for new streams all logic lived in the hypervisor.

Jan

Paul Durrant Sept. 7, 2020, 7:11 a.m. UTC | #4

> -----Original Message-----
> From: Jan Beulich <[e-mail]>
> Sent: 07 September 2020 08:01
> To: [e-mail]
> Cc: [e-mail]; Durrant, Paul <[e-mail]>; 'Ian Jackson'
> <[e-mail]>; 'Wei Liu' <[e-mail]>; 'Andrew Cooper' <[e-mail]>;
> 'George Dunlap' <[e-mail]>; 'Julien Grall' <[e-mail]>; 'Stefano Stabellini'
> <[e-mail]>
> Subject: RE: [EXTERNAL] [PATCH v7 7/9] common/domain: add a domain context record for shared_info...
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 04.09.2020 19:29, Paul Durrant wrote:
> >> From: Jan Beulich <[e-mail]>
> >> Sent: 26 August 2020 14:58
> >>
> >> On 18.08.2020 12:30, Paul Durrant wrote:
> >>> v7:
> >>>  - Only restore vcpu_info and arch sub-structures for PV domains, to match
> >>>    processing of SHARED_INFO in xc_sr_restore_x86_pv.c
> >>
> >> Since you point out this original logic, ...
> >>
> >>> +static int load_shared_info(struct domain *d, struct domain_context *c)
> >>> +{
> >>> +    struct domain_shared_info_context ctxt;
> >>> +    size_t hdr_size = offsetof(typeof(ctxt), buffer);
> >>> +    unsigned int i;
> >>> +    int rc;
> >>> +
> >>> +    rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, &i);
> >>> +    if ( rc )
> >>> +        return rc;
> >>> +
> >>> +    if ( i ) /* expect only a single instance */
> >>> +        return -ENXIO;
> >>> +
> >>> +    rc = domain_load_data(c, &ctxt, hdr_size);
> >>> +    if ( rc )
> >>> +        return rc;
> >>> +
> >>> +    if ( ctxt.buffer_size > sizeof(shared_info_t) ||
> >>> +         (ctxt.flags & ~DOMAIN_SAVE_32BIT_SHINFO) )
> >>> +        return -EINVAL;
> >>> +
> >>> +    if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
> >>> +    {
> >>> +#ifdef CONFIG_COMPAT
> >>> +        has_32bit_shinfo(d) = true;
> >>> +#else
> >>> +        return -EINVAL;
> >>> +#endif
> >>> +    }
> >>> +
> >>> +    if ( is_pv_domain(d) )
> >>> +    {
> >>> +        shared_info_t *shinfo = xmalloc(shared_info_t);
> >>> +
> >>> +        rc = domain_load_data(c, shinfo, sizeof(*shinfo));
> >>> +        if ( rc )
> >>> +        {
> >>> +            xfree(shinfo);
> >>> +            return rc;
> >>> +        }
> >>> +
> >>> +#ifdef CONFIG_COMPAT
> >>> +        if ( has_32bit_shinfo(d) )
> >>> +        {
> >>> +            memcpy(&d->shared_info->compat.vcpu_info,
> >>> +                   &shinfo->compat.vcpu_info,
> >>> +                   sizeof(d->shared_info->compat.vcpu_info));
> >>> +            memcpy(&d->shared_info->compat.arch,
> >>> +                   &shinfo->compat.arch,
> >>> +                   sizeof(d->shared_info->compat.vcpu_info));
> >>> +        }
> >>> +        else
> >>> +        {
> >>> +            memcpy(&d->shared_info->native.vcpu_info,
> >>> +                   &shinfo->native.vcpu_info,
> >>> +                   sizeof(d->shared_info->native.vcpu_info));
> >>> +            memcpy(&d->shared_info->native.arch,
> >>> +                   &shinfo->native.arch,
> >>> +                   sizeof(d->shared_info->native.arch));
> >>> +        }
> >>> +#else
> >>> +        memcpy(&d->shared_info->vcpu_info,
> >>> +               &shinfo->vcpu_info,
> >>> +               sizeof(d->shared_info->vcpu_info));
> >>> +        memcpy(&d->shared_info->arch,
> >>> +               &shinfo->arch,
> >>> +               sizeof(d->shared_info->shared));
> >>> +#endif
> >>
> >> ... where does the rest of that logic (resetting of
> >> arch.pfn_to_mfn_frame_list_list, evtchn_pending, evtchn_mask, and
> >> evtchn_pending_sel) get done? Or why is it not needed anymore?
> >
> > The resetting logic is still in xc_sr_restore_x86_pv.c (see patch #6).
> > It's going to need to stay there anyway to deal with older streams so
> > I made it common to both cases; it seems slightly separate from
> > restoring the shared info.
> 
> I guess I at least don't fully agree: The resetting is part of restoring,
> as it effectively determines which parts are restored and which parts
> are simply set (not truly reset, but I agree the perception may change
> depending on whose position you take). Hence at the very least this
> aspect wants clearly spelling out in the description, I think. But
> really I'd prefer if for old streams libxc took care of (all of) it,
> and if for new streams all logic lived in the hypervisor.
> 

Andrew, do you have an opinion either way?

  Paul

> Jan

Durrant, Paul Sept. 15, 2020, 2:20 p.m. UTC | #5

> -----Original Message-----
> From: Paul Durrant <[e-mail]>
> Sent: 04 September 2020 18:30
> To: 'Jan Beulich' <[e-mail]>
> Cc: [e-mail]; Durrant, Paul <[e-mail]>; 'Ian Jackson'
> <[e-mail]>; 'Wei Liu' <[e-mail]>; 'Andrew Cooper' <[e-mail]>;
> 'George Dunlap' <[e-mail]>; 'Julien Grall' <[e-mail]>; 'Stefano Stabellini'
> <[e-mail]>
> Subject: RE: [EXTERNAL] [PATCH v7 7/9] common/domain: add a domain context record for shared_info...
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> > -----Original Message-----
> > From: Jan Beulich <[e-mail]>
> > Sent: 26 August 2020 14:58
> > To: Paul Durrant <[e-mail]>
> > Cc: [e-mail]; Paul Durrant <[e-mail]>; Ian Jackson
> > <[e-mail]>; Wei Liu <[e-mail]>; Andrew Cooper <[e-mail]>; George
> > Dunlap <[e-mail]>; Julien Grall <[e-mail]>; Stefano Stabellini
> > <[e-mail]>
> > Subject: Re: [PATCH v7 7/9] common/domain: add a domain context record for shared_info...
> >
> > On 18.08.2020 12:30, Paul Durrant wrote:
> > > v7:
> > >  - Only restore vcpu_info and arch sub-structures for PV domains, to match
> > >    processing of SHARED_INFO in xc_sr_restore_x86_pv.c
> >
> > Since you point out this original logic, ...
> >
> > > +static int load_shared_info(struct domain *d, struct domain_context *c)
> > > +{
> > > +    struct domain_shared_info_context ctxt;
> > > +    size_t hdr_size = offsetof(typeof(ctxt), buffer);
> > > +    unsigned int i;
> > > +    int rc;
> > > +
> > > +    rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, &i);
> > > +    if ( rc )
> > > +        return rc;
> > > +
> > > +    if ( i ) /* expect only a single instance */
> > > +        return -ENXIO;
> > > +
> > > +    rc = domain_load_data(c, &ctxt, hdr_size);
> > > +    if ( rc )
> > > +        return rc;
> > > +
> > > +    if ( ctxt.buffer_size > sizeof(shared_info_t) ||
> > > +         (ctxt.flags & ~DOMAIN_SAVE_32BIT_SHINFO) )
> > > +        return -EINVAL;
> > > +
> > > +    if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
> > > +    {
> > > +#ifdef CONFIG_COMPAT
> > > +        has_32bit_shinfo(d) = true;
> > > +#else
> > > +        return -EINVAL;
> > > +#endif
> > > +    }
> > > +
> > > +    if ( is_pv_domain(d) )
> > > +    {
> > > +        shared_info_t *shinfo = xmalloc(shared_info_t);
> > > +
> > > +        rc = domain_load_data(c, shinfo, sizeof(*shinfo));
> > > +        if ( rc )
> > > +        {
> > > +            xfree(shinfo);
> > > +            return rc;
> > > +        }
> > > +
> > > +#ifdef CONFIG_COMPAT
> > > +        if ( has_32bit_shinfo(d) )
> > > +        {
> > > +            memcpy(&d->shared_info->compat.vcpu_info,
> > > +                   &shinfo->compat.vcpu_info,
> > > +                   sizeof(d->shared_info->compat.vcpu_info));
> > > +            memcpy(&d->shared_info->compat.arch,
> > > +                   &shinfo->compat.arch,
> > > +                   sizeof(d->shared_info->compat.vcpu_info));
> > > +        }
> > > +        else
> > > +        {
> > > +            memcpy(&d->shared_info->native.vcpu_info,
> > > +                   &shinfo->native.vcpu_info,
> > > +                   sizeof(d->shared_info->native.vcpu_info));
> > > +            memcpy(&d->shared_info->native.arch,
> > > +                   &shinfo->native.arch,
> > > +                   sizeof(d->shared_info->native.arch));
> > > +        }
> > > +#else
> > > +        memcpy(&d->shared_info->vcpu_info,
> > > +               &shinfo->vcpu_info,
> > > +               sizeof(d->shared_info->vcpu_info));
> > > +        memcpy(&d->shared_info->arch,
> > > +               &shinfo->arch,
> > > +               sizeof(d->shared_info->shared));
> > > +#endif
> >
> > ... where does the rest of that logic (resetting of
> > arch.pfn_to_mfn_frame_list_list, evtchn_pending, evtchn_mask, and
> > evtchn_pending_sel) get done? Or why is it not needed anymore?
> 
> The resetting logic is still in xc_sr_restore_x86_pv.c (see patch #6). It's going to need to stay
> there anyway to deal with older streams so I made it common to both cases; it seems slightly separate
> from restoring the shared info.
> 

Actually I've changed my mind on this one; I'll do the zeroing here and leave the old SHARED_INFO record restore code alone.

  Paul

>   Paul
> 
> >
> > Jan

Patch

diff --git a/tools/misc/xen-domctx.c b/tools/misc/xen-domctx.c
index 243325dfce..6ead7ea89d 100644
--- a/tools/misc/xen-domctx.c
+++ b/tools/misc/xen-domctx.c
@@ -31,6 +31,7 @@ 
 #include <errno.h>
 
 #include <xenctrl.h>
+#include <xen-tools/libs.h>
 #include <xen/xen.h>
 #include <xen/domctl.h>
 #include <xen/save.h>
@@ -61,6 +62,82 @@  static void dump_header(void)
 
 }
 
+static void print_binary(const char *prefix, const void *val, size_t size,
+                         const char *suffix)
+{
+    printf("%s", prefix);
+
+    while ( size-- )
+    {
+        uint8_t octet = *(const uint8_t *)val++;
+        unsigned int i;
+
+        for ( i = 0; i < 8; i++ )
+        {
+            printf("%u", octet & 1);
+            octet >>= 1;
+        }
+    }
+
+    printf("%s", suffix);
+}
+
+static void dump_shared_info(void)
+{
+    DOMAIN_SAVE_TYPE(SHARED_INFO) *s;
+    bool has_32bit_shinfo;
+    shared_info_any_t *info;
+    unsigned int i, n;
+
+    GET_PTR(s);
+    has_32bit_shinfo = s->flags & DOMAIN_SAVE_32BIT_SHINFO;
+
+    printf("    SHARED_INFO: has_32bit_shinfo: %s buffer_size: %u\n",
+           has_32bit_shinfo ? "true" : "false", s->buffer_size);
+
+    info = (shared_info_any_t *)s->buffer;
+
+#define GET_FIELD_PTR(_f)            \
+    (has_32bit_shinfo ?              \
+     (const void *)&(info->x32._f) : \
+     (const void *)&(info->x64._f))
+#define GET_FIELD_SIZE(_f) \
+    (has_32bit_shinfo ? sizeof(info->x32._f) : sizeof(info->x64._f))
+#define GET_FIELD(_f) \
+    (has_32bit_shinfo ? info->x32._f : info->x64._f)
+
+    n = has_32bit_shinfo ?
+        ARRAY_SIZE(info->x32.evtchn_pending) :
+        ARRAY_SIZE(info->x64.evtchn_pending);
+
+    for ( i = 0; i < n; i++ )
+    {
+        const char *prefix = !i ?
+            "                 evtchn_pending: " :
+            "                                 ";
+
+        print_binary(prefix, GET_FIELD_PTR(evtchn_pending[0]),
+                 GET_FIELD_SIZE(evtchn_pending[0]), "\n");
+    }
+
+    for ( i = 0; i < n; i++ )
+    {
+        const char *prefix = !i ?
+            "                    evtchn_mask: " :
+            "                                 ";
+
+        print_binary(prefix, GET_FIELD_PTR(evtchn_mask[0]),
+                 GET_FIELD_SIZE(evtchn_mask[0]), "\n");
+    }
+
+    printf("                 wc: version: %u sec: %u nsec: %u\n",
+           GET_FIELD(wc_version), GET_FIELD(wc_sec), GET_FIELD(wc_nsec));
+
+#undef GET_FIELD
+#undef GET_FIELD_SIZE
+#undef GET_FIELD_PTR
+}
+
 static void dump_end(void)
 {
     DOMAIN_SAVE_TYPE(END) *e;
@@ -173,6 +250,7 @@  int main(int argc, char **argv)
             switch (desc->typecode)
             {
             case DOMAIN_SAVE_CODE(HEADER): dump_header(); break;
+            case DOMAIN_SAVE_CODE(SHARED_INFO): dump_shared_info(); break;
             case DOMAIN_SAVE_CODE(END): dump_end(); break;
             default:
                 printf("Unknown type %u: skipping\n", desc->typecode);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index f0f9c62feb..9ac2158d6c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -33,6 +33,7 @@ 
 #include <xen/xenoprof.h>
 #include <xen/irq.h>
 #include <xen/argo.h>
+#include <xen/save.h>
 #include <asm/debugger.h>
 #include <asm/p2m.h>
 #include <asm/processor.h>
@@ -1659,6 +1660,119 @@  int continue_hypercall_on_cpu(
     return 0;
 }
 
+static int save_shared_info(const struct domain *d, struct domain_context *c,
+                            bool dry_run)
+{
+    struct domain_shared_info_context ctxt = {
+#ifdef CONFIG_COMPAT
+        .flags = has_32bit_shinfo(d) ? DOMAIN_SAVE_32BIT_SHINFO : 0,
+        .buffer_size = has_32bit_shinfo(d) ?
+                       sizeof(struct compat_shared_info) :
+                       sizeof(struct shared_info),
+#else
+        .buffer_size = sizeof(struct shared_info),
+#endif
+    };
+    size_t hdr_size = offsetof(typeof(ctxt), buffer);
+    int rc;
+
+    rc = DOMAIN_SAVE_BEGIN(SHARED_INFO, c, 0);
+    if ( rc )
+        return rc;
+
+    rc = domain_save_data(c, &ctxt, hdr_size);
+    if ( rc )
+        return rc;
+
+    rc = domain_save_data(c, d->shared_info, ctxt.buffer_size);
+    if ( rc )
+        return rc;
+
+    return domain_save_end(c);
+}
+
+static int load_shared_info(struct domain *d, struct domain_context *c)
+{
+    struct domain_shared_info_context ctxt;
+    size_t hdr_size = offsetof(typeof(ctxt), buffer);
+    unsigned int i;
+    int rc;
+
+    rc = DOMAIN_LOAD_BEGIN(SHARED_INFO, c, &i);
+    if ( rc )
+        return rc;
+
+    if ( i ) /* expect only a single instance */
+        return -ENXIO;
+
+    rc = domain_load_data(c, &ctxt, hdr_size);
+    if ( rc )
+        return rc;
+
+    if ( ctxt.buffer_size > sizeof(shared_info_t) ||
+         (ctxt.flags & ~DOMAIN_SAVE_32BIT_SHINFO) )
+        return -EINVAL;
+
+    if ( ctxt.flags & DOMAIN_SAVE_32BIT_SHINFO )
+    {
+#ifdef CONFIG_COMPAT
+        has_32bit_shinfo(d) = true;
+#else
+        return -EINVAL;
+#endif
+    }
+
+    if ( is_pv_domain(d) )
+    {
+        shared_info_t *shinfo = xmalloc(shared_info_t);
+
+        rc = domain_load_data(c, shinfo, sizeof(*shinfo));
+        if ( rc )
+        {
+            xfree(shinfo);
+            return rc;
+        }
+
+#ifdef CONFIG_COMPAT
+        if ( has_32bit_shinfo(d) )
+        {
+            memcpy(&d->shared_info->compat.vcpu_info,
+                   &shinfo->compat.vcpu_info,
+                   sizeof(d->shared_info->compat.vcpu_info));
+            memcpy(&d->shared_info->compat.arch,
+                   &shinfo->compat.arch,
+                   sizeof(d->shared_info->compat.vcpu_info));
+        }
+        else
+        {
+            memcpy(&d->shared_info->native.vcpu_info,
+                   &shinfo->native.vcpu_info,
+                   sizeof(d->shared_info->native.vcpu_info));
+            memcpy(&d->shared_info->native.arch,
+                   &shinfo->native.arch,
+                   sizeof(d->shared_info->native.arch));
+        }
+#else
+        memcpy(&d->shared_info->vcpu_info,
+               &shinfo->vcpu_info,
+               sizeof(d->shared_info->vcpu_info));
+        memcpy(&d->shared_info->arch,
+               &shinfo->arch,
+               sizeof(d->shared_info->shared));
+#endif
+
+        xfree(shinfo);
+
+        rc = domain_load_end(c, false);
+    }
+    else
+        rc = domain_load_end(c, true);
+
+    return rc;
+}
+
+DOMAIN_REGISTER_SAVE_LOAD(SHARED_INFO, save_shared_info, load_shared_info);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/public/save.h b/xen/include/public/save.h
index 551dbbddb8..0e855a4b97 100644
--- a/xen/include/public/save.h
+++ b/xen/include/public/save.h
@@ -82,7 +82,18 @@  struct domain_save_header {
 };
 DECLARE_DOMAIN_SAVE_TYPE(HEADER, 1, struct domain_save_header);
 
-#define DOMAIN_SAVE_CODE_MAX 1
+struct domain_shared_info_context {
+    uint32_t flags;
+
+#define DOMAIN_SAVE_32BIT_SHINFO 0x00000001
+
+    uint32_t buffer_size;
+    uint8_t buffer[XEN_FLEX_ARRAY_DIM]; /* Implementation specific size */
+};
+
+DECLARE_DOMAIN_SAVE_TYPE(SHARED_INFO, 2, struct domain_shared_info_context);
+
+#define DOMAIN_SAVE_CODE_MAX 2
 
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */