Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-24 Thread Jürgen Groß

On 17.05.24 15:33, Roger Pau Monne wrote:

Enabling it using an HVM param is fragile, and complicates the logic when
deciding whether options that interact with altp2m can also be enabled.

Leave the HVM param value for consumption by the guest, but prevent it from
being set.  Enabling is now done using and additional altp2m specific field in
xen_domctl_createdomain.

Note that albeit only currently implemented in x86, altp2m could be implemented
in other architectures, hence why the field is added to xen_domctl_createdomain
instead of xen_arch_domainconfig.

Signed-off-by: Roger Pau Monné 


Reviewed-by: Juergen Gross  # tools/libs/


Juergen



Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-23 Thread Stefano Stabellini
On Thu, 23 May 2024, Roger Pau Monné wrote:
> On Fri, May 17, 2024 at 03:33:51PM +0200, Roger Pau Monne wrote:
> > Enabling it using an HVM param is fragile, and complicates the logic when
> > deciding whether options that interact with altp2m can also be enabled.
> > 
> > Leave the HVM param value for consumption by the guest, but prevent it from
> > being set.  Enabling is now done using and additional altp2m specific field 
> > in
> > xen_domctl_createdomain.
> > 
> > Note that albeit only currently implemented in x86, altp2m could be 
> > implemented
> > in other architectures, hence why the field is added to 
> > xen_domctl_createdomain
> > instead of xen_arch_domainconfig.
> > 
> > Signed-off-by: Roger Pau Monné 
> > ---
> > Changes since v2:
> >  - Introduce a new altp2m field in xen_domctl_createdomain.
> > 
> > Changes since v1:
> >  - New in this version.
> > ---
> >  tools/libs/light/libxl_create.c | 23 ++-
> >  tools/libs/light/libxl_x86.c| 26 --
> >  tools/ocaml/libs/xc/xenctrl_stubs.c |  2 +-
> >  xen/arch/arm/domain.c   |  6 ++
> 
> Could I get an Ack from one of the Arm maintainers for the trivial Arm
> change?

Acked-by: Stefano Stabellini 

Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-23 Thread Roger Pau Monné
On Fri, May 17, 2024 at 03:33:51PM +0200, Roger Pau Monne wrote:
> Enabling it using an HVM param is fragile, and complicates the logic when
> deciding whether options that interact with altp2m can also be enabled.
> 
> Leave the HVM param value for consumption by the guest, but prevent it from
> being set.  Enabling is now done using and additional altp2m specific field in
> xen_domctl_createdomain.
> 
> Note that albeit only currently implemented in x86, altp2m could be 
> implemented
> in other architectures, hence why the field is added to 
> xen_domctl_createdomain
> instead of xen_arch_domainconfig.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Changes since v2:
>  - Introduce a new altp2m field in xen_domctl_createdomain.
> 
> Changes since v1:
>  - New in this version.
> ---
>  tools/libs/light/libxl_create.c | 23 ++-
>  tools/libs/light/libxl_x86.c| 26 --
>  tools/ocaml/libs/xc/xenctrl_stubs.c |  2 +-
>  xen/arch/arm/domain.c   |  6 ++

Could I get an Ack from one of the Arm maintainers for the trivial Arm
change?

Thanks, Roger.



Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-23 Thread Jan Beulich
On 22.05.2024 18:21, Roger Pau Monné wrote:
> On Wed, May 22, 2024 at 03:34:29PM +0200, Jan Beulich wrote:
>> On 22.05.2024 15:16, Roger Pau Monné wrote:
>>> On Tue, May 21, 2024 at 12:30:32PM +0200, Jan Beulich wrote:
 On 17.05.2024 15:33, Roger Pau Monne wrote:
> Enabling it using an HVM param is fragile, and complicates the logic when
> deciding whether options that interact with altp2m can also be enabled.
>
> Leave the HVM param value for consumption by the guest, but prevent it 
> from
> being set.  Enabling is now done using and additional altp2m specific 
> field in
> xen_domctl_createdomain.
>
> Note that albeit only currently implemented in x86, altp2m could be 
> implemented
> in other architectures, hence why the field is added to 
> xen_domctl_createdomain
> instead of xen_arch_domainconfig.
>
> Signed-off-by: Roger Pau Monné 

 Reviewed-by: Jan Beulich  # hypervisor
 albeit with one question:

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -637,6 +637,8 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  bool hap = config->flags & XEN_DOMCTL_CDF_hap;
>  bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
>  unsigned int max_vcpus;
> +unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
> + XEN_DOMCTL_ALTP2M_mode_mask);
>  
>  if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
>  {
> @@ -715,6 +717,26 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  return -EINVAL;
>  }
>  
> +if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
> +{
> +dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
> +config->flags);
> +return -EINVAL;
> +}
> +
> +if ( altp2m_mode && nested_virt )
> +{
> +dprintk(XENLOG_INFO,
> +"Nested virt and altp2m are not supported together\n");
> +return -EINVAL;
> +}
> +
> +if ( altp2m_mode && !hap )
> +{
> +dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
> +return -EINVAL;
> +}

 Should this last one perhaps be further extended to permit altp2m with EPT
 only?
>>>
>>> Hm, yes, that would be more accurate as:
>>>
>>> if ( altp2m_mode && (!hap || !hvm_altp2m_supported()) )
>>
>> Wouldn't
>>
>>if ( altp2m_mode && !hvm_altp2m_supported() )
>>
>> suffice? hvm_funcs.caps.altp2m is not supposed to be set when no HAP,
>> as long as HAP continues to be a pre-condition?
> 
> No, `hap` here signals whether the domain is using HAP, and we need to
> take this int account, otherwise we would allow enabling altp2m for
> domains using shadow.

Oh, right. But then the original for is good enough HAP-wise, as a request
to use HAP when HAP isn't available is deal with elsewhere. The
!hvm_altp2m_supported() is still wanted imo (for there potentially being
other restrictions), but then in a separate check, not resulting in a HAP-
specific log message. I'll commit the patch in its original form, and that
further addition can then be an incremental change.

Jan



Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-22 Thread Roger Pau Monné
On Wed, May 22, 2024 at 03:34:29PM +0200, Jan Beulich wrote:
> On 22.05.2024 15:16, Roger Pau Monné wrote:
> > On Tue, May 21, 2024 at 12:30:32PM +0200, Jan Beulich wrote:
> >> On 17.05.2024 15:33, Roger Pau Monne wrote:
> >>> Enabling it using an HVM param is fragile, and complicates the logic when
> >>> deciding whether options that interact with altp2m can also be enabled.
> >>>
> >>> Leave the HVM param value for consumption by the guest, but prevent it 
> >>> from
> >>> being set.  Enabling is now done using and additional altp2m specific 
> >>> field in
> >>> xen_domctl_createdomain.
> >>>
> >>> Note that albeit only currently implemented in x86, altp2m could be 
> >>> implemented
> >>> in other architectures, hence why the field is added to 
> >>> xen_domctl_createdomain
> >>> instead of xen_arch_domainconfig.
> >>>
> >>> Signed-off-by: Roger Pau Monné 
> >>
> >> Reviewed-by: Jan Beulich  # hypervisor
> >> albeit with one question:
> >>
> >>> --- a/xen/arch/x86/domain.c
> >>> +++ b/xen/arch/x86/domain.c
> >>> @@ -637,6 +637,8 @@ int arch_sanitise_domain_config(struct 
> >>> xen_domctl_createdomain *config)
> >>>  bool hap = config->flags & XEN_DOMCTL_CDF_hap;
> >>>  bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
> >>>  unsigned int max_vcpus;
> >>> +unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
> >>> + XEN_DOMCTL_ALTP2M_mode_mask);
> >>>  
> >>>  if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
> >>>  {
> >>> @@ -715,6 +717,26 @@ int arch_sanitise_domain_config(struct 
> >>> xen_domctl_createdomain *config)
> >>>  return -EINVAL;
> >>>  }
> >>>  
> >>> +if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
> >>> +{
> >>> +dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
> >>> +config->flags);
> >>> +return -EINVAL;
> >>> +}
> >>> +
> >>> +if ( altp2m_mode && nested_virt )
> >>> +{
> >>> +dprintk(XENLOG_INFO,
> >>> +"Nested virt and altp2m are not supported together\n");
> >>> +return -EINVAL;
> >>> +}
> >>> +
> >>> +if ( altp2m_mode && !hap )
> >>> +{
> >>> +dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
> >>> +return -EINVAL;
> >>> +}
> >>
> >> Should this last one perhaps be further extended to permit altp2m with EPT
> >> only?
> > 
> > Hm, yes, that would be more accurate as:
> > 
> > if ( altp2m_mode && (!hap || !hvm_altp2m_supported()) )
> 
> Wouldn't
> 
>if ( altp2m_mode && !hvm_altp2m_supported() )
> 
> suffice? hvm_funcs.caps.altp2m is not supposed to be set when no HAP,
> as long as HAP continues to be a pre-condition?

No, `hap` here signals whether the domain is using HAP, and we need to
take this int account, otherwise we would allow enabling altp2m for
domains using shadow.

Thanks, Roger.



Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-22 Thread Jan Beulich
On 22.05.2024 15:16, Roger Pau Monné wrote:
> On Tue, May 21, 2024 at 12:30:32PM +0200, Jan Beulich wrote:
>> On 17.05.2024 15:33, Roger Pau Monne wrote:
>>> Enabling it using an HVM param is fragile, and complicates the logic when
>>> deciding whether options that interact with altp2m can also be enabled.
>>>
>>> Leave the HVM param value for consumption by the guest, but prevent it from
>>> being set.  Enabling is now done using and additional altp2m specific field 
>>> in
>>> xen_domctl_createdomain.
>>>
>>> Note that albeit only currently implemented in x86, altp2m could be 
>>> implemented
>>> in other architectures, hence why the field is added to 
>>> xen_domctl_createdomain
>>> instead of xen_arch_domainconfig.
>>>
>>> Signed-off-by: Roger Pau Monné 
>>
>> Reviewed-by: Jan Beulich  # hypervisor
>> albeit with one question:
>>
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -637,6 +637,8 @@ int arch_sanitise_domain_config(struct 
>>> xen_domctl_createdomain *config)
>>>  bool hap = config->flags & XEN_DOMCTL_CDF_hap;
>>>  bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
>>>  unsigned int max_vcpus;
>>> +unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
>>> + XEN_DOMCTL_ALTP2M_mode_mask);
>>>  
>>>  if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
>>>  {
>>> @@ -715,6 +717,26 @@ int arch_sanitise_domain_config(struct 
>>> xen_domctl_createdomain *config)
>>>  return -EINVAL;
>>>  }
>>>  
>>> +if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
>>> +{
>>> +dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
>>> +config->flags);
>>> +return -EINVAL;
>>> +}
>>> +
>>> +if ( altp2m_mode && nested_virt )
>>> +{
>>> +dprintk(XENLOG_INFO,
>>> +"Nested virt and altp2m are not supported together\n");
>>> +return -EINVAL;
>>> +}
>>> +
>>> +if ( altp2m_mode && !hap )
>>> +{
>>> +dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
>>> +return -EINVAL;
>>> +}
>>
>> Should this last one perhaps be further extended to permit altp2m with EPT
>> only?
> 
> Hm, yes, that would be more accurate as:
> 
> if ( altp2m_mode && (!hap || !hvm_altp2m_supported()) )

Wouldn't

   if ( altp2m_mode && !hvm_altp2m_supported() )

suffice? hvm_funcs.caps.altp2m is not supposed to be set when no HAP,
as long as HAP continues to be a pre-condition?

> Would you be fine adjusting at commit, or would you prefer me to send
> an updated version?

I'd be happy to fold in whatever we settle on.

Jan




Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-22 Thread Roger Pau Monné
On Tue, May 21, 2024 at 12:30:32PM +0200, Jan Beulich wrote:
> On 17.05.2024 15:33, Roger Pau Monne wrote:
> > Enabling it using an HVM param is fragile, and complicates the logic when
> > deciding whether options that interact with altp2m can also be enabled.
> > 
> > Leave the HVM param value for consumption by the guest, but prevent it from
> > being set.  Enabling is now done using and additional altp2m specific field 
> > in
> > xen_domctl_createdomain.
> > 
> > Note that albeit only currently implemented in x86, altp2m could be 
> > implemented
> > in other architectures, hence why the field is added to 
> > xen_domctl_createdomain
> > instead of xen_arch_domainconfig.
> > 
> > Signed-off-by: Roger Pau Monné 
> 
> Reviewed-by: Jan Beulich  # hypervisor
> albeit with one question:
> 
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -637,6 +637,8 @@ int arch_sanitise_domain_config(struct 
> > xen_domctl_createdomain *config)
> >  bool hap = config->flags & XEN_DOMCTL_CDF_hap;
> >  bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
> >  unsigned int max_vcpus;
> > +unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
> > + XEN_DOMCTL_ALTP2M_mode_mask);
> >  
> >  if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
> >  {
> > @@ -715,6 +717,26 @@ int arch_sanitise_domain_config(struct 
> > xen_domctl_createdomain *config)
> >  return -EINVAL;
> >  }
> >  
> > +if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
> > +{
> > +dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
> > +config->flags);
> > +return -EINVAL;
> > +}
> > +
> > +if ( altp2m_mode && nested_virt )
> > +{
> > +dprintk(XENLOG_INFO,
> > +"Nested virt and altp2m are not supported together\n");
> > +return -EINVAL;
> > +}
> > +
> > +if ( altp2m_mode && !hap )
> > +{
> > +dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
> > +return -EINVAL;
> > +}
> 
> Should this last one perhaps be further extended to permit altp2m with EPT
> only?

Hm, yes, that would be more accurate as:

if ( altp2m_mode && (!hap || !hvm_altp2m_supported()) )

Would you be fine adjusting at commit, or would you prefer me to send
an updated version?

Thanks, Roger.



Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-21 Thread Jan Beulich
On 17.05.2024 15:33, Roger Pau Monne wrote:
> Enabling it using an HVM param is fragile, and complicates the logic when
> deciding whether options that interact with altp2m can also be enabled.
> 
> Leave the HVM param value for consumption by the guest, but prevent it from
> being set.  Enabling is now done using and additional altp2m specific field in
> xen_domctl_createdomain.
> 
> Note that albeit only currently implemented in x86, altp2m could be 
> implemented
> in other architectures, hence why the field is added to 
> xen_domctl_createdomain
> instead of xen_arch_domainconfig.
> 
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Jan Beulich  # hypervisor
albeit with one question:

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -637,6 +637,8 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  bool hap = config->flags & XEN_DOMCTL_CDF_hap;
>  bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
>  unsigned int max_vcpus;
> +unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
> + XEN_DOMCTL_ALTP2M_mode_mask);
>  
>  if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
>  {
> @@ -715,6 +717,26 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  return -EINVAL;
>  }
>  
> +if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
> +{
> +dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
> +config->flags);
> +return -EINVAL;
> +}
> +
> +if ( altp2m_mode && nested_virt )
> +{
> +dprintk(XENLOG_INFO,
> +"Nested virt and altp2m are not supported together\n");
> +return -EINVAL;
> +}
> +
> +if ( altp2m_mode && !hap )
> +{
> +dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
> +return -EINVAL;
> +}

Should this last one perhaps be further extended to permit altp2m with EPT
only?

Jan



Re: [PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-17 Thread Christian Lindig



> On 17 May 2024, at 14:33, Roger Pau Monne  wrote:
> 
> Enabling it using an HVM param is fragile, and complicates the logic when
> deciding whether options that interact with altp2m can also be enabled.
> 
> Leave the HVM param value for consumption by the guest, but prevent it from
> being set.  Enabling is now done using and additional altp2m specific field in
> xen_domctl_createdomain.
> 
> Note that albeit only currently implemented in x86, altp2m could be 
> implemented
> in other architectures, hence why the field is added to 
> xen_domctl_createdomain
> instead of xen_arch_domainconfig.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Changes since v2:
> - Introduce a new altp2m field in xen_domctl_createdomain.
> 
> Changes since v1:
> - New in this version.
> ---
> tools/libs/light/libxl_create.c | 23 ++-
> tools/libs/light/libxl_x86.c| 26 --
> tools/ocaml/libs/xc/xenctrl_stubs.c |  2 +-
> xen/arch/arm/domain.c   |  6 ++
> xen/arch/x86/domain.c   | 22 ++
> xen/arch/x86/hvm/hvm.c  | 23 ++-
> xen/include/public/domctl.h | 20 +++-
> xen/include/public/hvm/params.h |  9 ++---
> 8 files changed, 106 insertions(+), 25 deletions(-)

> 
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
> b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index 2b6d3c09dfa0..c6da9bb09137 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -257,7 +257,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value 
> wanted_domid, value co
> #if defined(__i386__) || defined(__x86_64__)
> 
> /* Quick & dirty check for ABI changes. */
> - BUILD_BUG_ON(sizeof(cfg) != 64);
> + BUILD_BUG_ON(sizeof(cfg) != 68);
> 

Acked-by: Christian Lindig 





[PATCH for-4.19 v3 2/3] xen: enable altp2m at create domain domctl

2024-05-17 Thread Roger Pau Monne
Enabling it using an HVM param is fragile, and complicates the logic when
deciding whether options that interact with altp2m can also be enabled.

Leave the HVM param value for consumption by the guest, but prevent it from
being set.  Enabling is now done using and additional altp2m specific field in
xen_domctl_createdomain.

Note that albeit only currently implemented in x86, altp2m could be implemented
in other architectures, hence why the field is added to xen_domctl_createdomain
instead of xen_arch_domainconfig.

Signed-off-by: Roger Pau Monné 
---
Changes since v2:
 - Introduce a new altp2m field in xen_domctl_createdomain.

Changes since v1:
 - New in this version.
---
 tools/libs/light/libxl_create.c | 23 ++-
 tools/libs/light/libxl_x86.c| 26 --
 tools/ocaml/libs/xc/xenctrl_stubs.c |  2 +-
 xen/arch/arm/domain.c   |  6 ++
 xen/arch/x86/domain.c   | 22 ++
 xen/arch/x86/hvm/hvm.c  | 23 ++-
 xen/include/public/domctl.h | 20 +++-
 xen/include/public/hvm/params.h |  9 ++---
 8 files changed, 106 insertions(+), 25 deletions(-)

diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 41252ec55370..edeadd57ef5a 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -372,7 +372,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
 libxl_defbool_setdefault(_info->u.hvm.viridian,   false);
 libxl_defbool_setdefault(_info->u.hvm.hpet,   true);
 libxl_defbool_setdefault(_info->u.hvm.vpt_align,  true);
-libxl_defbool_setdefault(_info->u.hvm.altp2m, false);
 libxl_defbool_setdefault(_info->u.hvm.usb,false);
 libxl_defbool_setdefault(_info->u.hvm.vkb_device, true);
 libxl_defbool_setdefault(_info->u.hvm.xen_platform_pci,   true);
@@ -678,6 +677,28 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config 
*d_config,
 if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
 create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
 
+LOG(DETAIL, "altp2m: %s", libxl_altp2m_mode_to_string(b_info->altp2m));
+switch(b_info->altp2m) {
+case LIBXL_ALTP2M_MODE_MIXED:
+create.altp2m_opts |=
+XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_mixed);
+break;
+
+case LIBXL_ALTP2M_MODE_EXTERNAL:
+create.altp2m_opts |=
+XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_external);
+break;
+
+case LIBXL_ALTP2M_MODE_LIMITED:
+create.altp2m_opts |=
+XEN_DOMCTL_ALTP2M_mode(XEN_DOMCTL_ALTP2M_limited);
+break;
+
+case LIBXL_ALTP2M_MODE_DISABLED:
+/* Nothing to do - altp2m disabled is signaled as mode == 0. */
+break;
+}
+
 /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
 libxl_uuid_copy(ctx, (libxl_uuid *), >uuid);
 
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index a50ec37eb3eb..60643d6f5376 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -407,19 +407,9 @@ static int hvm_set_conf_params(libxl__gc *gc, uint32_t 
domid,
 libxl_ctx *ctx = libxl__gc_owner(gc);
 xc_interface *xch = ctx->xch;
 int ret = ERROR_FAIL;
-unsigned int altp2m = info->altp2m;
 
 switch(info->type) {
 case LIBXL_DOMAIN_TYPE_HVM:
-/* The config parameter "altp2m" replaces the parameter "altp2mhvm". 
For
- * legacy reasons, both parameters are accepted on x86 HVM guests.
- *
- * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
- * Otherwise set altp2m based on the field info->altp2m. */
-if (info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
-libxl_defbool_val(info->u.hvm.altp2m))
-altp2m = libxl_defbool_val(info->u.hvm.altp2m);
-
 if (xc_hvm_param_set(xch, domid, HVM_PARAM_HPET_ENABLED,
  libxl_defbool_val(info->u.hvm.hpet))) {
 LOG(ERROR, "Couldn't set HVM_PARAM_HPET_ENABLED");
@@ -444,10 +434,6 @@ static int hvm_set_conf_params(libxl__gc *gc, uint32_t 
domid,
 LOG(ERROR, "Couldn't set HVM_PARAM_TIMER_MODE");
 goto out;
 }
-if (xc_hvm_param_set(xch, domid, HVM_PARAM_ALTP2M, altp2m)) {
-LOG(ERROR, "Couldn't set HVM_PARAM_ALTP2M");
-goto out;
-}
 break;
 
 default:
@@ -818,6 +804,18 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
 libxl_defbool_setdefault(_info->acpi, true);
 libxl_defbool_setdefault(_info->arch_x86.msr_relaxed, false);
 
+/*
+ * The config parameter "altp2m" replaces the parameter "altp2mhvm".
+ * For legacy reasons, both parameters are accepted on x86