Re: [Xen-devel] [PATCH 3/6] x86/pvh: allow PVH Dom0 to use the debug IO port console

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Oct 12, 2018 at 09:55:10AM -0600, Jan Beulich wrote:
> >>> On 09.10.18 at 11:42, <roger.pau@xxxxxxxxxx> wrote:
> > --- a/docs/misc/xen-command-line.markdown
> > +++ b/docs/misc/xen-command-line.markdown
> > @@ -681,6 +681,17 @@ Flag that makes a dom0 boot in PVHv2 mode.
> >  Flag that makes a dom0 use shadow paging. Only works when "pvh" is
> >  enabled.
> >  
> > +> `debug-ioport`
> > +
> > +> Default: `false`
> > +
> > +Flag that enables the HVM debug console for a PVH Dom0. Xen will trap 
> > accesses
> > +to IO port 0xe9 so that Dom0 kernel can print output using this IO port 
> > before
> > +setting up the hypercall page.
> > +
> > +Note this option is not enabled by default because it might clash with 
> > hardware
> > +on the system using IO port 0xe9 and should only be used for debug 
> > purposes.
> 
> You also want to extend the "List of" at the top of the option.
> 
> > --- a/xen/arch/x86/dom0_build.c
> > +++ b/xen/arch/x86/dom0_build.c
> > @@ -212,12 +212,16 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain 
> > *dom0)
> >  bool __initdata opt_dom0_shadow;
> >  #endif
> >  bool __initdata dom0_pvh;
> > +#ifdef CONFIG_HVM
> > +bool __initdata opt_dom0_debug_ioport;
> > +#endif
> 
> And dom0_pvh does not go into this same conditional?

dom0_pvh is used by __start_xen without any guards, so while I agree
that it should be guarded with CONFIG_HVM it will require some
unrelated changes.

> > @@ -236,6 +240,10 @@ static int __init parse_dom0_param(const char *s)
> >  #ifdef CONFIG_SHADOW_PAGING
> >          else if ( (val = parse_boolean("shadow", s, ss)) >= 0 )
> >              opt_dom0_shadow = val;
> > +#endif
> > +#ifdef CONFIG_HVM
> > +        else if ( (val = parse_boolean("debug-ioport", s, ss)) >= 0 )
> > +            opt_dom0_debug_ioport = val;
> >  #endif
> >          else
> >              rc = -EINVAL;
> > @@ -433,6 +441,11 @@ int __init dom0_setup_permissions(struct domain *d)
> >          rc |= ioports_deny_access(d, pmtmr_ioport, pmtmr_ioport + 3);
> >      /* PCI configuration space (NB. 0xcf8 has special treatment). */
> >      rc |= ioports_deny_access(d, 0xcfc, 0xcff);
> > +#ifdef CONFIG_HVM
> > +    if ( is_hvm_domain(d) && opt_dom0_debug_ioport )
> > +        /* HVM debug console IO port. */
> > +        rc |= ioports_deny_access(d, 0xe9, 0xe9);
> 
> I think we finally need a manifest constant for this port number.

I've added a patch to introduce the constant define.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel