xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support
@ 2021-06-07 13:00 Juergen Gross
  2021-06-07 13:04 ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Juergen Gross @ 2021-06-07 13:00 UTC (permalink / raw)
  To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu

After 32-bit PV-guests have been security de-supported when not running
under PV-shim, the hypervisor will no longer be configured to support
those domains per default when not being built as PV-shim.

Unfortunately libxenguest will fail saving or restoring a PV domain
due to this restriction, as it is trying to get the compat MFN list
even for 64 bit guests.

Fix that by obtaining the compat MFN list only for 32-bit PV guests.

Fixes: 1a0f2fe2297d122a08fe ("SUPPORT.md: Un-shimmed 32-bit PV guests are no longer supported")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- set compat MFN to "invalid" instead of net setting it at all (Jan Beulich)
- don't set compat MFN for 64-bit guests even if running as 32-bit
  domain (Andrew Cooper)
---
 tools/libs/guest/xg_sr_common.h        |  2 +-
 tools/libs/guest/xg_sr_common_x86_pv.c | 37 +++++++++++++++-----------
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/tools/libs/guest/xg_sr_common.h b/tools/libs/guest/xg_sr_common.h
index cc3ad1c394..e2994e18ac 100644
--- a/tools/libs/guest/xg_sr_common.h
+++ b/tools/libs/guest/xg_sr_common.h
@@ -325,7 +325,7 @@ struct xc_sr_context
                 xen_pfn_t max_mfn;
                 /* Read-only machine to phys map */
                 xen_pfn_t *m2p;
-                /* first mfn of the compat m2p (Only needed for 32bit PV guests) */
+                /* first mfn of the compat m2p (Only set for 32bit PV guests) */
                 xen_pfn_t compat_m2p_mfn0;
                 /* Number of m2p frames mapped */
                 unsigned long nr_m2p_frames;
diff --git a/tools/libs/guest/xg_sr_common_x86_pv.c b/tools/libs/guest/xg_sr_common_x86_pv.c
index cd33406aab..f339ea4a79 100644
--- a/tools/libs/guest/xg_sr_common_x86_pv.c
+++ b/tools/libs/guest/xg_sr_common_x86_pv.c
@@ -149,27 +149,32 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
 
     ctx->x86.pv.nr_m2p_frames = (M2P_CHUNK_SIZE >> PAGE_SHIFT) * m2p_chunks;
 
+    if ( ctx->x86.pv.levels == 3 )
+    {
 #ifdef __i386__
-    /* 32 bit toolstacks automatically get the compat m2p */
-    ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
+        /* 32 bit toolstacks automatically get the compat m2p */
+        ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
 #else
-    /* 64 bit toolstacks need to ask Xen specially for it */
-    {
-        struct xen_machphys_mfn_list xmml = {
-            .max_extents = 1,
-            .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
-        };
-
-        rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
-                          &xmml, sizeof(xmml));
-        if ( rc || xmml.nr_extents != 1 )
+        /* 64 bit toolstacks need to ask Xen specially for it */
         {
-            PERROR("Failed to get compat mfn list from Xen");
-            rc = -1;
-            goto err;
+            struct xen_machphys_mfn_list xmml = {
+                .max_extents = 1,
+                .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
+            };
+
+            rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
+                              &xmml, sizeof(xmml));
+            if ( rc || xmml.nr_extents != 1 )
+            {
+                PERROR("Failed to get compat mfn list from Xen");
+                rc = -1;
+                goto err;
+            }
         }
-    }
 #endif
+    }
+    else
+        ctx->x86.pv.compat_m2p_mfn0 = INVALID_MFN;
 
     /* All Done */
     rc = 0;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support
  2021-06-07 13:00 [PATCH v2] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support Juergen Gross
@ 2021-06-07 13:04 ` Jan Beulich
  2021-06-07 13:59   ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2021-06-07 13:04 UTC (permalink / raw)
  To: Juergen Gross; +Cc: Ian Jackson, Wei Liu, xen-devel

On 07.06.2021 15:00, Juergen Gross wrote:
> --- a/tools/libs/guest/xg_sr_common_x86_pv.c
> +++ b/tools/libs/guest/xg_sr_common_x86_pv.c
> @@ -149,27 +149,32 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
>  
>      ctx->x86.pv.nr_m2p_frames = (M2P_CHUNK_SIZE >> PAGE_SHIFT) * m2p_chunks;
>  
> +    if ( ctx->x86.pv.levels == 3 )
> +    {

With this opening brace you no longer need ...

>  #ifdef __i386__
> -    /* 32 bit toolstacks automatically get the compat m2p */
> -    ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
> +        /* 32 bit toolstacks automatically get the compat m2p */
> +        ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>  #else
> -    /* 64 bit toolstacks need to ask Xen specially for it */
> -    {

... this one, and hence you could avoid re-indenting ...

> -        struct xen_machphys_mfn_list xmml = {
> -            .max_extents = 1,
> -            .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
> -        };
> -
> -        rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
> -                          &xmml, sizeof(xmml));
> -        if ( rc || xmml.nr_extents != 1 )
> +        /* 64 bit toolstacks need to ask Xen specially for it */
>          {
> -            PERROR("Failed to get compat mfn list from Xen");
> -            rc = -1;
> -            goto err;
> +            struct xen_machphys_mfn_list xmml = {
> +                .max_extents = 1,
> +                .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
> +            };
> +
> +            rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
> +                              &xmml, sizeof(xmml));
> +            if ( rc || xmml.nr_extents != 1 )
> +            {
> +                PERROR("Failed to get compat mfn list from Xen");
> +                rc = -1;
> +                goto err;
> +            }

... all of this. Preferably with such reduced code churn,
still/again:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support
  2021-06-07 13:04 ` Jan Beulich
@ 2021-06-07 13:59   ` Andrew Cooper
  2021-06-07 14:25     ` Juergen Gross
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2021-06-07 13:59 UTC (permalink / raw)
  To: Juergen Gross; +Cc: Ian Jackson, Wei Liu, xen-devel, Jan Beulich

On 07/06/2021 14:04, Jan Beulich wrote:
> On 07.06.2021 15:00, Juergen Gross wrote:
>> --- a/tools/libs/guest/xg_sr_common_x86_pv.c
>> +++ b/tools/libs/guest/xg_sr_common_x86_pv.c
>> @@ -149,27 +149,32 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
>>  
>>      ctx->x86.pv.nr_m2p_frames = (M2P_CHUNK_SIZE >> PAGE_SHIFT) * m2p_chunks;
>>  
>> +    if ( ctx->x86.pv.levels == 3 )
>> +    {
> With this opening brace you no longer need ...
>
>>  #ifdef __i386__
>> -    /* 32 bit toolstacks automatically get the compat m2p */
>> -    ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>> +        /* 32 bit toolstacks automatically get the compat m2p */
>> +        ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>>  #else
>> -    /* 64 bit toolstacks need to ask Xen specially for it */
>> -    {
> ... this one, and hence you could avoid re-indenting ...
>
>> -        struct xen_machphys_mfn_list xmml = {
>> -            .max_extents = 1,
>> -            .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
>> -        };
>> -
>> -        rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
>> -                          &xmml, sizeof(xmml));
>> -        if ( rc || xmml.nr_extents != 1 )
>> +        /* 64 bit toolstacks need to ask Xen specially for it */
>>          {
>> -            PERROR("Failed to get compat mfn list from Xen");
>> -            rc = -1;
>> -            goto err;
>> +            struct xen_machphys_mfn_list xmml = {
>> +                .max_extents = 1,
>> +                .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
>> +            };
>> +
>> +            rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
>> +                              &xmml, sizeof(xmml));
>> +            if ( rc || xmml.nr_extents != 1 )
>> +            {
>> +                PERROR("Failed to get compat mfn list from Xen");
>> +                rc = -1;
>> +                goto err;
>> +            }
> ... all of this. Preferably with such reduced code churn,
> still/again:

I agree.  I can fix on commit, if you're happy with that.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support
  2021-06-07 13:59   ` Andrew Cooper
@ 2021-06-07 14:25     ` Juergen Gross
  0 siblings, 0 replies; 4+ messages in thread
From: Juergen Gross @ 2021-06-07 14:25 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Ian Jackson, Wei Liu, xen-devel, Jan Beulich


[-- Attachment #1.1.1: Type: text/plain, Size: 2321 bytes --]

On 07.06.21 15:59, Andrew Cooper wrote:
> On 07/06/2021 14:04, Jan Beulich wrote:
>> On 07.06.2021 15:00, Juergen Gross wrote:
>>> --- a/tools/libs/guest/xg_sr_common_x86_pv.c
>>> +++ b/tools/libs/guest/xg_sr_common_x86_pv.c
>>> @@ -149,27 +149,32 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
>>>   
>>>       ctx->x86.pv.nr_m2p_frames = (M2P_CHUNK_SIZE >> PAGE_SHIFT) * m2p_chunks;
>>>   
>>> +    if ( ctx->x86.pv.levels == 3 )
>>> +    {
>> With this opening brace you no longer need ...
>>
>>>   #ifdef __i386__
>>> -    /* 32 bit toolstacks automatically get the compat m2p */
>>> -    ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>>> +        /* 32 bit toolstacks automatically get the compat m2p */
>>> +        ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>>>   #else
>>> -    /* 64 bit toolstacks need to ask Xen specially for it */
>>> -    {
>> ... this one, and hence you could avoid re-indenting ...
>>
>>> -        struct xen_machphys_mfn_list xmml = {
>>> -            .max_extents = 1,
>>> -            .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
>>> -        };
>>> -
>>> -        rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
>>> -                          &xmml, sizeof(xmml));
>>> -        if ( rc || xmml.nr_extents != 1 )
>>> +        /* 64 bit toolstacks need to ask Xen specially for it */
>>>           {
>>> -            PERROR("Failed to get compat mfn list from Xen");
>>> -            rc = -1;
>>> -            goto err;
>>> +            struct xen_machphys_mfn_list xmml = {
>>> +                .max_extents = 1,
>>> +                .extent_start = { &ctx->x86.pv.compat_m2p_mfn0 },
>>> +            };
>>> +
>>> +            rc = do_memory_op(xch, XENMEM_machphys_compat_mfn_list,
>>> +                              &xmml, sizeof(xmml));
>>> +            if ( rc || xmml.nr_extents != 1 )
>>> +            {
>>> +                PERROR("Failed to get compat mfn list from Xen");
>>> +                rc = -1;
>>> +                goto err;
>>> +            }
>> ... all of this. Preferably with such reduced code churn,
>> still/again:
> 
> I agree.  I can fix on commit, if you're happy with that.

I'm fine with that.

> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 

Thanks,

Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-07 14:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-07 13:00 [PATCH v2] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support Juergen Gross
2021-06-07 13:04 ` Jan Beulich
2021-06-07 13:59   ` Andrew Cooper
2021-06-07 14:25     ` Juergen Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).