Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Wei,

On 07/07/2016 06:27 PM, Wei Liu wrote:
> On Mon, Jul 04, 2016 at 01:45:45PM +0200, Sergej Proskurin wrote:
>> The current implementation allows to set the parameter HVM_PARAM_ALTP2M.
>> This parameter allows further usage of altp2m on ARM. For this, we
>> define an additional altp2m field for PV domains as part of the
>> libxl_domain_type struct. This field can be set only on ARM systems
>> through the "altp2m" switch in the domain's configuration file (i.e.
>> set altp2m=1).
>>
>> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
>> ---
>> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>> ---
>>  tools/libxl/libxl_create.c  |  1 +
>>  tools/libxl/libxl_dom.c     | 14 ++++++++++++++
>>  tools/libxl/libxl_types.idl |  1 +
>>  tools/libxl/xl_cmdimpl.c    |  5 +++++
>>  4 files changed, 21 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index 1b99472..40b5f61 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>>              b_info->cmdline = b_info->u.pv.cmdline;
>>              b_info->u.pv.cmdline = NULL;
>>          }
>> +        libxl_defbool_setdefault(&b_info->u.pv.altp2m, false);
>>          break;
>>      default:
>>          LOG(ERROR, "invalid domain type %s in create info",
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index ec29060..ab023a2 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -277,6 +277,16 @@ err:
>>  }
>>  #endif
>>  
>> +#if defined(__arm__) || defined(__aarch64__)
>> +static void pv_set_conf_params(xc_interface *handle, uint32_t domid,
>> +                               libxl_domain_build_info *const info)
>> +{
>> +    if ( libxl_defbool_val(info->u.pv.altp2m) )
> 
> Coding style.
> 

I am not sure which part does not fulfill Xen's coding style. Could you
be more specific please? Thank you.

>> +        xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M,
>> +                         libxl_defbool_val(info->u.pv.altp2m));
>> +}
>> +#endif
>> +
>>  static void hvm_set_conf_params(xc_interface *handle, uint32_t domid,
>>                                  libxl_domain_build_info *const info)
>>  {
>> @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>>              return rc;
>>  #endif
>>      }
>> +#if defined(__arm__) || defined(__aarch64__)
>> +    else /* info->type == LIBXL_DOMAIN_TYPE_PV */
>> +        pv_set_conf_params(ctx->xch, domid, info);
>> +#endif
>>  
>>      rc = libxl__arch_domain_create(gc, d_config, domid);
>>  
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index ef614be..0a164f9 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>>                                        ("features", string, {'const': True}),
>>                                        # Use host's E820 for PCI passthrough.
>>                                        ("e820_host", libxl_defbool),
>> +                                      ("altp2m", libxl_defbool),
> 
> Why is this placed in PV instead of arch_arm?
> 

The current implementation mirrors the x86's altp2m, where the altp2m
field represents a property of HVM guests in b_info->u.hvm.altp2m. Since
guests on ARM are marked as PV, I have placed the field altp2m into
b_info->u.pv.altp2m.

However, if you believe that it would make more sense to place altp2m
into b_info->arch_arm.altp2m, I will try to adapt the affected fields.

> You would also need a LIBXL_HAVE macro in libxl.h for the new field.
> 

There is already a LIBXL_HAVE_ALTP2M macro defined in libxl.h. Or did
you mean using an explicit one for ARM?

>>                                        ])),
>>                   ("invalid", None),
>>                   ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 6459eec..12c6e48 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -1718,6 +1718,11 @@ static void parse_config_data(const char 
>> *config_source,
>>              exit(1);
>>          }
>>  
>> +#if defined(__arm__) || defined(__aarch64__)
>> +        /* Enable altp2m for PV guests solely on ARM */
>> +        xlu_cfg_get_defbool(config, "altp2m", &b_info->u.pv.altp2m, 0);
>> +#endif
>> +
>>          break;
>>      }
>>      default:
>> -- 
>> 2.8.3
>>

Thank you.

Best regards,
~Sergej

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel