[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RESEND 1/2] hvc_xen: support PV on HVM consoles



On Tue, 2011-06-07 at 17:28 +0100, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  drivers/tty/hvc/hvc_xen.c          |   75 
> ++++++++++++++++++++++++++++++------
>  include/xen/interface/hvm/params.h |    6 ++-
>  2 files changed, 68 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 52fdf60..9e27f7e 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -24,9 +24,12 @@
>  #include <linux/init.h>
>  #include <linux/types.h>
>  
> +#include <asm/io.h>
>  #include <asm/xen/hypervisor.h>
>  
>  #include <xen/xen.h>
> +#include <xen/interface/xen.h>
> +#include <xen/hvm.h>
>  #include <xen/page.h>
>  #include <xen/events.h>
>  #include <xen/interface/io/console.h>
> @@ -42,9 +45,13 @@ static int xencons_irq;
>  /* ------------------------------------------------------------------ */
>  
>  static unsigned long console_pfn = ~0ul;
> +static unsigned int console_evtchn = ~0;
> +static struct xencons_interface *xencons_if = NULL;
>  
>  static inline struct xencons_interface *xencons_interface(void)
>  {
> +     if (xencons_if != NULL)
> +             return xencons_if;
>       if (console_pfn == ~0ul)
>               return mfn_to_virt(xen_start_info->console.domU.mfn);
>       else
> @@ -54,7 +61,10 @@ static inline struct xencons_interface 
> *xencons_interface(void)
>  static inline void notify_daemon(void)
>  {
>       /* Use evtchn: this is called early, before irq is set up. */
> -     notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> +     if (console_evtchn == ~0ul)
> +             notify_remote_via_evtchn(xen_start_info->console.domU.evtchn);
> +     else
> +             notify_remote_via_evtchn(console_evtchn);

You already have
    console_evtchn = xen_start_info->console.domU.evtchn;
below in the init function, so is this test needed?

(there is roughly the same existing with console_pfn and
(xen_start_info->console.domU.mfn in xencons_interface).

>  }
>  
>  static int __write_console(const char *data, int len)
> @@ -157,23 +167,60 @@ static struct hv_ops dom0_hvc_ops = {
>       .notifier_hangup = notifier_hangup_irq,
>  };
>  
> +static int xen_hvm_console_init(void)
> +{
> +     int r;
> +     uint64_t v = 0;
> +     unsigned long mfn;
> +
> +     if (!xen_hvm_domain())
> +             return -ENODEV;
> +
> +     if (xencons_if != NULL)
> +             return -EBUSY;
> +
> +     r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
> +     if (r < 0)
> +             return -ENODEV;
> +     console_evtchn = v;
> +     hvm_get_parameter(HVM_PARAM_CONSOLE_PFN, &v);
> +     if (r < 0)
> +             return -ENODEV;
> +     mfn = v;
> +     xencons_if = ioremap(mfn << PAGE_SHIFT, PAGE_SIZE);

mfn here is used equivalently to console_pfn, isn't it? Perhaps this
ioremap could be done in xencons_interface for parity with the pv case?

Or perhaps better the __va / mfn_to_virt stuff from xencons_interface
should move into the init functions to initialise xencons_if, at which
point the xencons_interface() function becomes a dumb wrapper (or even
better goes away in favour of accessing the variable direct).

> +     if (xencons_if == NULL)
> +             return -ENODEV;
> +
> +     return 0;
> +}
> +
>  static int __init xen_hvc_init(void)
>  {
>       struct hvc_struct *hp;
>       struct hv_ops *ops;
> +     int r;
>  
> -     if (!xen_pv_domain())
> +     if (!xen_domain())
> +             return -ENODEV;
> +
> +     if (xen_pv_domain() && !xen_initial_domain() &&
> +                     !xen_start_info->console.domU.evtchn)
>               return -ENODEV;
>  
>       if (xen_initial_domain()) {
>               ops = &dom0_hvc_ops;
>               xencons_irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
>       } else {
> -             if (!xen_start_info->console.domU.evtchn)
> -                     return -ENODEV;
> -
>               ops = &domU_hvc_ops;
> -             xencons_irq = 
> bind_evtchn_to_irq(xen_start_info->console.domU.evtchn);
> +             if (xen_pv_domain()) {
> +                     console_pfn = 
> mfn_to_pfn(xen_start_info->console.domU.mfn);
> +                     console_evtchn = xen_start_info->console.domU.evtchn;
> +             } else {
> +                     r = xen_hvm_console_init();
> +                     if (r < 0)
> +                             return r;
> +             }
> +             xencons_irq = bind_evtchn_to_irq(console_evtchn);
>       }
>       if (xencons_irq < 0)
>               xencons_irq = 0; /* NO_IRQ */
> @@ -186,15 +233,13 @@ static int __init xen_hvc_init(void)
>  
>       hvc = hp;
>  
> -     console_pfn = mfn_to_pfn(xen_start_info->console.domU.mfn);
> -
>       return 0;
>  }
>  
>  void xen_console_resume(void)
>  {
>       if (xencons_irq)
> -             rebind_evtchn_irq(xen_start_info->console.domU.evtchn, 
> xencons_irq);
> +             rebind_evtchn_irq(console_evtchn, xencons_irq);
>  }
>  
>  static void __exit xen_hvc_fini(void)
> @@ -205,16 +250,22 @@ static void __exit xen_hvc_fini(void)
>  
>  static int xen_cons_init(void)
>  {
> -     struct hv_ops *ops;
> +     const struct hv_ops *ops;
>  
> -     if (!xen_pv_domain())
> +     if (!xen_domain())
>               return 0;
>  
>       if (xen_initial_domain())
>               ops = &dom0_hvc_ops;
> -     else
> +     else {
>               ops = &domU_hvc_ops;
>  
> +             if (xen_pv_domain())
> +                     console_evtchn = xen_start_info->console.domU.evtchn;
> +             else
> +                     xen_hvm_console_init();
> +     }

Might be cleaner to have xen_{pv,hvm}_console_init even if the pv one is
just a one liner?

xen_cons_init and xen_hvc_init seem to have a fair bit in common now,
perhaps there is room for pulling some stuff up into a common function?

> +
>       hvc_instantiate(HVC_COOKIE, 0, ops);
>       return 0;
>  }
> diff --git a/include/xen/interface/hvm/params.h 
> b/include/xen/interface/hvm/params.h
> index 1888d8c..1b4f923 100644
> --- a/include/xen/interface/hvm/params.h
> +++ b/include/xen/interface/hvm/params.h
> @@ -90,6 +90,10 @@
>  /* Boolean: Enable aligning all periodic vpts to reduce interrupts */
>  #define HVM_PARAM_VPT_ALIGN    16
>  
> -#define HVM_NR_PARAMS          17
> +/* Console debug shared memory ring and event channel */
> +#define HVM_PARAM_CONSOLE_PFN    17
> +#define HVM_PARAM_CONSOLE_EVTCHN 18
> +
> +#define HVM_NR_PARAMS          19
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.