| Message ID | 1500296815-10243-5-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SBSA UART emulation support in Xen | expand |
Hi Bhupinder, Sorry I am jumping a bit late in the discussion here. On 17/07/17 14:06, Bhupinder Thakur wrote: > An option is provided in libxl to enable/disable sbsa vuart while s/sbsa/SBSA/ > creating a guest domain. > > Libxl now suppots a generic vuart console and sbsa uart is a specific type. s/suppots/supports/ s/sbsa/SBSA/ > In future support can be added for multiple vuart of different types. > > User can enable sbsa vuart by adding the following line in the guest ditto. > configuration file: > > vuart = "sbsa_uart" > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > Acked-by: Stefano Stabellini <sstabellini@kernel.org> > Acked-by: Wei Liu <wei.liu2@citrix.com> > --- > CC: Ian Jackson <ian.jackson@eu.citrix.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien.grall@arm.com> > > Changes since v4: > - Renamed "pl011" to "sbsa_uart". > > Changes since v3: > - Added a new config option CONFIG_VUART_CONSOLE to enable/disable vuart console > support. > - Moved libxl_vuart_type to arch-arm part of libxl_domain_build_info > - Updated xl command help to mention new console type - vuart. > > Changes since v2: > - Defined vuart option as an enum instead of a string. > - Removed the domain creation flag defined for vuart and the related code > to pass on the information while domain creation. Now vpl011 is initialized > independent of domain creation through new DOMCTL APIs. > > tools/libxl/libxl.h | 6 ++++++ > tools/libxl/libxl_console.c | 3 +++ > tools/libxl/libxl_dom.c | 1 + > tools/libxl/libxl_internal.h | 3 +++ > tools/libxl/libxl_types.idl | 7 +++++++ > tools/xl/xl_cmdtable.c | 2 +- > tools/xl/xl_console.c | 5 ++++- > tools/xl/xl_parse.c | 8 ++++++++ > 8 files changed, 33 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 7cf0f31..892ed35 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -306,6 +306,12 @@ > #define LIBXL_HAVE_BUILDINFO_HVM_ACPI_LAPTOP_SLATE 1 > > /* > + * LIBXL_HAVE_VUART indicates that xenconsole/client supports > + * virtual uart. I am not sure why you mention about xenconsole/client supporting VUART. It does not really matter here, someone may use another backend for the PV console here. What matters is the existence or arm.vuart. > + */ > +#define LIBXL_HAVE_VUART 1 Here you give the impression the virtual UART is supported for all the architectures but ... [...] > @@ -581,6 +587,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ > > > ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), > + ("vuart", libxl_vuart_type), ... here it is ARM specific. I am not convinced that we should tie vuart to ARM only. I cannot see why x86 would not be able to use it in the future. Any opinions? > ])), > # Alternate p2m is not bound to any architecture or guest type, as it is > # supported by x86 HVM and ARM support is planned. > diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c > index 30eb93c..9f91651 100644 > --- a/tools/xl/xl_cmdtable.c > +++ b/tools/xl/xl_cmdtable.c > @@ -133,7 +133,7 @@ struct cmd_spec cmd_table[] = { > &main_console, 0, 0, > "Attach to domain's console", > "[options] <Domain>\n" > - "-t <type> console type, pv or serial\n" > + "-t <type> console type, pv , serial or vuart\n" > "-n <number> console number" > }, > { "vncviewer", > diff --git a/tools/xl/xl_console.c b/tools/xl/xl_console.c > index 0508dda..4e65d73 100644 > --- a/tools/xl/xl_console.c > +++ b/tools/xl/xl_console.c > @@ -27,6 +27,7 @@ int main_console(int argc, char **argv) > uint32_t domid; > int opt = 0, num = 0; > libxl_console_type type = 0; > + char *console_names = "pv, serial, vuart"; > > SWITCH_FOREACH_OPT(opt, "n:t:", NULL, "console", 1) { > case 't': > @@ -34,8 +35,10 @@ 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, "vuart")) > + type = LIBXL_CONSOLE_TYPE_VUART; > else { > - fprintf(stderr, "console type supported are: pv, serial\n"); > + fprintf(stderr, "console type supported are: %s\n", console_names); > return EXIT_FAILURE; > } > break; > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c > index 5c2bf17..71588de 100644 > --- a/tools/xl/xl_parse.c > +++ b/tools/xl/xl_parse.c > @@ -918,6 +918,14 @@ void parse_config_data(const char *config_source, > if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0)) > b_info->max_vcpus = l; > > + if (!xlu_cfg_get_string(config, "vuart", &buf, 0)) { > + if (libxl_vuart_type_from_string(buf, &b_info->arch_arm.vuart)) { > + fprintf(stderr, "ERROR: invalid value \"%s\" for \"vuart\"\n", > + buf); > + exit(1); > + } > + } > + > parse_vnuma_config(config, b_info); > > /* Set max_memkb to target_memkb and max_vcpus to avail_vcpus if > Cheers,
CC x86 maintainers On Tue, Jul 18, 2017 at 12:19:19PM +0100, Julien Grall wrote: > > > > ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), > > + ("vuart", libxl_vuart_type), > > ... here it is ARM specific. I am not convinced that we should tie vuart to > ARM only. I cannot see why x86 would not be able to use it in the future. > Any opinions? I don't know. I asked Bhupinder to put it here because it looked arm specific to me. I will let x86 maintainers to decide whether they want such thing.
Hi, On 18 July 2017 at 17:00, Wei Liu <wei.liu2@citrix.com> wrote: > CC x86 maintainers > > On Tue, Jul 18, 2017 at 12:19:19PM +0100, Julien Grall wrote: >> > >> > ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), >> > + ("vuart", libxl_vuart_type), >> >> ... here it is ARM specific. I am not convinced that we should tie vuart to >> ARM only. I cannot see why x86 would not be able to use it in the future. >> Any opinions? > > I don't know. I asked Bhupinder to put it here because it looked arm > specific to me. I will let x86 maintainers to decide whether they want > such thing. What is the decision on this? I believe that since most of the vuart code added in libxl is arch agnostic, it should be fine to keep the libxl_vuart_type as a generic type. Regards, Bhupinder
On Tue, Jul 25, 2017 at 11:08:24PM +0530, Bhupinder Thakur wrote: > Hi, > > On 18 July 2017 at 17:00, Wei Liu <wei.liu2@citrix.com> wrote: > > CC x86 maintainers > > > > On Tue, Jul 18, 2017 at 12:19:19PM +0100, Julien Grall wrote: > >> > > >> > ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), > >> > + ("vuart", libxl_vuart_type), > >> > >> ... here it is ARM specific. I am not convinced that we should tie vuart to > >> ARM only. I cannot see why x86 would not be able to use it in the future. > >> Any opinions? > > > > I don't know. I asked Bhupinder to put it here because it looked arm > > specific to me. I will let x86 maintainers to decide whether they want > > such thing. > > What is the decision on this? > Unfortunately this email probably slipped through the crack for Andrew and Jan. I've prodded Andrew on IRC so he might chime in. > I believe that since most of the vuart code added in libxl is arch > agnostic, it should be fine to keep the libxl_vuart_type as a generic > type. > What about the actual emulation code? Is that arch-agnostic? If not, I personally don't see a chance of having vuart emulation for x86 in the near future and I'm inclined to keep the code as-is. There is always the option to lift the struct to common code in the future.
Hi Wei, On 07/28/2017 02:49 PM, Wei Liu wrote: > On Tue, Jul 25, 2017 at 11:08:24PM +0530, Bhupinder Thakur wrote: >> Hi, >> >> On 18 July 2017 at 17:00, Wei Liu <wei.liu2@citrix.com> wrote: >>> CC x86 maintainers >>> >>> On Tue, Jul 18, 2017 at 12:19:19PM +0100, Julien Grall wrote: >>>>> >>>>> ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), >>>>> + ("vuart", libxl_vuart_type), >>>> >>>> ... here it is ARM specific. I am not convinced that we should tie vuart to >>>> ARM only. I cannot see why x86 would not be able to use it in the future. >>>> Any opinions? >>> >>> I don't know. I asked Bhupinder to put it here because it looked arm >>> specific to me. I will let x86 maintainers to decide whether they want >>> such thing. >> >> What is the decision on this? >> > > Unfortunately this email probably slipped through the crack for Andrew > and Jan. > > I've prodded Andrew on IRC so he might chime in. > >> I believe that since most of the vuart code added in libxl is arch >> agnostic, it should be fine to keep the libxl_vuart_type as a generic >> type. >> > > What about the actual emulation code? Is that arch-agnostic? If not, I > personally don't see a chance of having vuart emulation for x86 in the > near future and I'm inclined to keep the code as-is. > > There is always the option to lift the struct to common code in the > future. Lifting the struct to common will imply to add compatibility code, right? Cheers,
On Fri, Jul 28, 2017 at 03:14:31PM +0100, Julien Grall wrote: > Hi Wei, > > On 07/28/2017 02:49 PM, Wei Liu wrote: > > On Tue, Jul 25, 2017 at 11:08:24PM +0530, Bhupinder Thakur wrote: > > > Hi, > > > > > > On 18 July 2017 at 17:00, Wei Liu <wei.liu2@citrix.com> wrote: > > > > CC x86 maintainers > > > > > > > > On Tue, Jul 18, 2017 at 12:19:19PM +0100, Julien Grall wrote: > > > > > > > > > > > > ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), > > > > > > + ("vuart", libxl_vuart_type), > > > > > > > > > > ... here it is ARM specific. I am not convinced that we should tie vuart to > > > > > ARM only. I cannot see why x86 would not be able to use it in the future. > > > > > Any opinions? > > > > > > > > I don't know. I asked Bhupinder to put it here because it looked arm > > > > specific to me. I will let x86 maintainers to decide whether they want > > > > such thing. > > > > > > What is the decision on this? > > > > > > > Unfortunately this email probably slipped through the crack for Andrew > > and Jan. > > > > I've prodded Andrew on IRC so he might chime in. > > > > > I believe that since most of the vuart code added in libxl is arch > > > agnostic, it should be fine to keep the libxl_vuart_type as a generic > > > type. > > > > > > > What about the actual emulation code? Is that arch-agnostic? If not, I > > personally don't see a chance of having vuart emulation for x86 in the > > near future and I'm inclined to keep the code as-is. > > > > There is always the option to lift the struct to common code in the > > future. > > Lifting the struct to common will imply to add compatibility code, right? Yes.
On 07/28/2017 03:42 PM, Wei Liu wrote: > On Fri, Jul 28, 2017 at 03:14:31PM +0100, Julien Grall wrote: >> Hi Wei, >> >> On 07/28/2017 02:49 PM, Wei Liu wrote: >>> On Tue, Jul 25, 2017 at 11:08:24PM +0530, Bhupinder Thakur wrote: >>>> Hi, >>>> >>>> On 18 July 2017 at 17:00, Wei Liu <wei.liu2@citrix.com> wrote: >>>>> CC x86 maintainers >>>>> >>>>> On Tue, Jul 18, 2017 at 12:19:19PM +0100, Julien Grall wrote: >>>>>>> >>>>>>> ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), >>>>>>> + ("vuart", libxl_vuart_type), >>>>>> >>>>>> ... here it is ARM specific. I am not convinced that we should tie vuart to >>>>>> ARM only. I cannot see why x86 would not be able to use it in the future. >>>>>> Any opinions? >>>>> >>>>> I don't know. I asked Bhupinder to put it here because it looked arm >>>>> specific to me. I will let x86 maintainers to decide whether they want >>>>> such thing. >>>> >>>> What is the decision on this? >>>> >>> >>> Unfortunately this email probably slipped through the crack for Andrew >>> and Jan. >>> >>> I've prodded Andrew on IRC so he might chime in. >>> >>>> I believe that since most of the vuart code added in libxl is arch >>>> agnostic, it should be fine to keep the libxl_vuart_type as a generic >>>> type. >>>> >>> >>> What about the actual emulation code? Is that arch-agnostic? If not, I >>> personally don't see a chance of having vuart emulation for x86 in the >>> near future and I'm inclined to keep the code as-is. >>> >>> There is always the option to lift the struct to common code in the >>> future. >> >> Lifting the struct to common will imply to add compatibility code, right? > > Yes. I would try to avoid that. But you are the maintainer, so it is your call :). Cheers,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 7cf0f31..892ed35 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -306,6 +306,12 @@ #define LIBXL_HAVE_BUILDINFO_HVM_ACPI_LAPTOP_SLATE 1 /* + * LIBXL_HAVE_VUART indicates that xenconsole/client supports + * virtual uart. + */ +#define LIBXL_HAVE_VUART 1 + +/* * libxl ABI compatibility * * The only guarantee which libxl makes regarding ABI compatibility diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c index 446e766..853be15 100644 --- a/tools/libxl/libxl_console.c +++ b/tools/libxl/libxl_console.c @@ -67,6 +67,9 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, case LIBXL_CONSOLE_TYPE_SERIAL: cons_type_s = "serial"; break; + case LIBXL_CONSOLE_TYPE_VUART: + cons_type_s = "vuart"; + break; default: goto out; } diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index f54fd49..e0f0d78 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -803,6 +803,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, if (xc_dom_translated(dom)) { state->console_mfn = dom->console_pfn; state->store_mfn = dom->xenstore_pfn; + state->vuart_gfn = dom->vuart_gfn; } else { state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index afe6652..d0d50c3 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1139,6 +1139,9 @@ typedef struct { uint32_t num_vmemranges; xc_domain_configuration_t config; + + xen_pfn_t vuart_gfn; + evtchn_port_t vuart_port; } libxl__domain_build_state; _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid, diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 8a9849c..728cc56 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, "VUART"), ]) libxl_disk_format = Enumeration("disk_format", [ @@ -240,6 +241,11 @@ libxl_checkpointed_stream = Enumeration("checkpointed_stream", [ (2, "COLO"), ]) +libxl_vuart_type = Enumeration("vuart_type", [ + (0, "unknown"), + (1, "sbsa_uart"), + ]) + # # Complex libxl types # @@ -581,6 +587,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), + ("vuart", libxl_vuart_type), ])), # Alternate p2m is not bound to any architecture or guest type, as it is # supported by x86 HVM and ARM support is planned. diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c index 30eb93c..9f91651 100644 --- a/tools/xl/xl_cmdtable.c +++ b/tools/xl/xl_cmdtable.c @@ -133,7 +133,7 @@ struct cmd_spec cmd_table[] = { &main_console, 0, 0, "Attach to domain's console", "[options] <Domain>\n" - "-t <type> console type, pv or serial\n" + "-t <type> console type, pv , serial or vuart\n" "-n <number> console number" }, { "vncviewer", diff --git a/tools/xl/xl_console.c b/tools/xl/xl_console.c index 0508dda..4e65d73 100644 --- a/tools/xl/xl_console.c +++ b/tools/xl/xl_console.c @@ -27,6 +27,7 @@ int main_console(int argc, char **argv) uint32_t domid; int opt = 0, num = 0; libxl_console_type type = 0; + char *console_names = "pv, serial, vuart"; SWITCH_FOREACH_OPT(opt, "n:t:", NULL, "console", 1) { case 't': @@ -34,8 +35,10 @@ 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, "vuart")) + type = LIBXL_CONSOLE_TYPE_VUART; else { - fprintf(stderr, "console type supported are: pv, serial\n"); + fprintf(stderr, "console type supported are: %s\n", console_names); return EXIT_FAILURE; } break; diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 5c2bf17..71588de 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -918,6 +918,14 @@ void parse_config_data(const char *config_source, if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0)) b_info->max_vcpus = l; + if (!xlu_cfg_get_string(config, "vuart", &buf, 0)) { + if (libxl_vuart_type_from_string(buf, &b_info->arch_arm.vuart)) { + fprintf(stderr, "ERROR: invalid value \"%s\" for \"vuart\"\n", + buf); + exit(1); + } + } + parse_vnuma_config(config, b_info); /* Set max_memkb to target_memkb and max_vcpus to avail_vcpus if