[PATCH] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support

[PATCH] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support

[Juergen Gross]

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>
---
 tools/libs/guest/xg_sr_common_x86_pv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libs/guest/xg_sr_common_x86_pv.c b/tools/libs/guest/xg_sr_common_x86_pv.c
index cd33406aab..ad20461e2e 100644
--- a/tools/libs/guest/xg_sr_common_x86_pv.c
+++ b/tools/libs/guest/xg_sr_common_x86_pv.c
@@ -154,6 +154,7 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
     ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
 #else
     /* 64 bit toolstacks need to ask Xen specially for it */
+    if ( ctx->x86.pv.levels == 3 )
     {
         struct xen_machphys_mfn_list xmml = {
             .max_extents = 1,
-- 
2.26.2

Re: [PATCH] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support

[Jan Beulich]

On 07.06.2021 11:04, Juergen Gross wrote:
> 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>

As this will do for the single present consumer of the field
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Nevertheless I wonder ...

> --- a/tools/libs/guest/xg_sr_common_x86_pv.c
> +++ b/tools/libs/guest/xg_sr_common_x86_pv.c
> @@ -154,6 +154,7 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
>      ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>  #else
>      /* 64 bit toolstacks need to ask Xen specially for it */
> +    if ( ctx->x86.pv.levels == 3 )
>      {
>          struct xen_machphys_mfn_list xmml = {
>              .max_extents = 1,

... whether the field wouldn't better get set to an always-invalid
value in all other cases (either by pre-setting or by adding an
"else" here), e.g. ~0.

Jan

Re: [PATCH] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support

[Andrew Cooper]

On 07/06/2021 10:04, Juergen Gross wrote:
> 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>
> ---
>  tools/libs/guest/xg_sr_common_x86_pv.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/libs/guest/xg_sr_common_x86_pv.c b/tools/libs/guest/xg_sr_common_x86_pv.c
> index cd33406aab..ad20461e2e 100644
> --- a/tools/libs/guest/xg_sr_common_x86_pv.c
> +++ b/tools/libs/guest/xg_sr_common_x86_pv.c
> @@ -154,6 +154,7 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
>      ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>  #else
>      /* 64 bit toolstacks need to ask Xen specially for it */
> +    if ( ctx->x86.pv.levels == 3 )
>      {
>          struct xen_machphys_mfn_list xmml = {
>              .max_extents = 1,

This wants to encompass the whole ifdef block, to avoid having differing
behaviour between compile widths.

Also the comment next to compat_m2p_mfn0 wants adjusting to say "only
set for 32bit PV guests".

~Andrew

Re: [PATCH] tools/libs/guest: fix save and restore of pv domains after 32-bit de-support

[Juergen Gross]

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

On 07.06.21 13:01, Andrew Cooper wrote:
> On 07/06/2021 10:04, Juergen Gross wrote:
>> 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>
>> ---
>>   tools/libs/guest/xg_sr_common_x86_pv.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/libs/guest/xg_sr_common_x86_pv.c b/tools/libs/guest/xg_sr_common_x86_pv.c
>> index cd33406aab..ad20461e2e 100644
>> --- a/tools/libs/guest/xg_sr_common_x86_pv.c
>> +++ b/tools/libs/guest/xg_sr_common_x86_pv.c
>> @@ -154,6 +154,7 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
>>       ctx->x86.pv.compat_m2p_mfn0 = entries[0].mfn;
>>   #else
>>       /* 64 bit toolstacks need to ask Xen specially for it */
>> +    if ( ctx->x86.pv.levels == 3 )
>>       {
>>           struct xen_machphys_mfn_list xmml = {
>>               .max_extents = 1,
> 
> This wants to encompass the whole ifdef block, to avoid having differing
> behaviour between compile widths.
> 
> Also the comment next to compat_m2p_mfn0 wants adjusting to say "only
> set for 32bit PV guests".

Okay, together with Jan's suggestion this makes sense.


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 --]