WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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