Re: [Xen-devel] [PATCH 04/10] xen/arm: vpl011: Provide a knob in libxl to enable/disable pl011 emulation

[Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>]

Hi Wei,

>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index a612d1f..fe7f795 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -105,6 +105,7 @@ libxl_console_type = Enumeration("console_type", [
>>      (0, "UNKNOWN"),
>>      (1, "SERIAL"),
>>      (2, "PV"),
>> +    (3, "VCON"),
>>      ])
>>
>
> You need to add a LIBXL_HAVE macro to libxl.h.

I will add this macro. I am trying to understand the use of these
macros. Are the applications which are building against a particular
libxl version supposed to check this flag before using this feature?

>
>>  libxl_disk_format = Enumeration("disk_format", [
>> @@ -460,6 +461,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>>      ("disable_migrate", libxl_defbool),
>>      ("cpuid",           libxl_cpuid_policy_list),
>>      ("blkdev_start",    string),
>> +    ("enable_pl011",    libxl_defbool),
>>
>>      ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")),
>>
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 358757f..4f4d4e6 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -1433,6 +1433,8 @@ static void parse_config_data(const char 
>> *config_source,
>>      if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
>>          b_info->max_vcpus = l;
>>
>> +    xlu_cfg_get_defbool(config, "pl011", &b_info->enable_pl011, 0);
>> +
>
> I'm not very keen on having "pl011" because it is ARM specific.
I will rename it to vconsole, which I am using through out the libxl code now.

>
> Is there a case in which you don't want this vconsole? Should we always
> enable it in toolstack?
There was a review comment from Julien that pl011 emulation should not
be enabled by default for domU. So based on this flag the pl011
emulation will be enabled/disabled at the time of domain creation.

>
>>      parse_vnuma_config(config, b_info);
>>
>>      /* Set max_memkb to target_memkb and max_vcpus to avail_vcpus if
>> @@ -3788,6 +3790,8 @@ int main_console(int argc, char **argv)
>>              type = LIBXL_CONSOLE_TYPE_PV;
>>          else if (!strcmp(optarg, "serial"))
>>              type = LIBXL_CONSOLE_TYPE_SERIAL;
>> +        else if (!strcmp(optarg, "vcon"))
>> +            type = LIBXL_CONSOLE_TYPE_VCON;
>>          else {
>>              fprintf(stderr, "console type supported are: pv, serial\n");
>>              return EXIT_FAILURE;
>
> You also need to patch manpage etc.
>
> It appears that your tree is not up to date -- xl was split out from
> libxl at some point.
I will rebase my patches on the latest master branch.

Regards,
Bhupinder

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