diff mbox

[v2,2/3] hvc_xen: fix xenboot for DomUs

Message ID 1456402239-4179-2-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefano Stabellini Feb. 25, 2016, 12:10 p.m. UTC
The xenboot early console has been partially broken for DomU for a long
time: the output would only go to the hypervisor via hypercall
(HYPERVISOR_console_io), while it wouldn't actually go to the DomU
console. The reason is that domU_write_console would return early as no
xencons structs are configured for it.

Add an appropriate xencons struct for xenboot from the xenboot setup
callback.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---
Changes in v2:
- add return to xenboot_setup_console
---
 drivers/tty/hvc/hvc_xen.c |   32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

Comments

Boris Ostrovsky Feb. 25, 2016, 3:04 p.m. UTC | #1
On 02/25/2016 07:10 AM, Stefano Stabellini wrote:
> The xenboot early console has been partially broken for DomU for a long
> time: the output would only go to the hypervisor via hypercall
> (HYPERVISOR_console_io), while it wouldn't actually go to the DomU
> console. The reason is that domU_write_console would return early as no
> xencons structs are configured for it.
>
> Add an appropriate xencons struct for xenboot from the xenboot setup
> callback.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> ---
> Changes in v2:
> - add return to xenboot_setup_console
> ---
>   drivers/tty/hvc/hvc_xen.c |   32 ++++++++++++++++++++++++++------
>   1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 68b8ec8..bf787aa 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -246,6 +246,18 @@ err:
>   	return -ENODEV;
>   }
>   
> +static int xen_early_pv_console_init(struct xencons_info *info, int vtermno)

Nit: not sure whether "early" is the right word to use here: when called 
from xen_pv_console_init() it's not really on the early path.

Other than that:
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

> +{
> +	info->evtchn = xen_start_info->console.domU.evtchn;
> +	/* GFN == MFN for PV guest */
> +	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
> +	info->vtermno = vtermno;
> +
> +	list_add_tail(&info->list, &xenconsoles);
> +
> +	return 0;
> +}
> +
>   static int xen_pv_console_init(void)
>   {
>   	struct xencons_info *info;
> @@ -265,13 +277,8 @@ static int xen_pv_console_init(void)
>   		/* already configured */
>   		return 0;
>   	}
> -	info->evtchn = xen_start_info->console.domU.evtchn;
> -	/* GFN == MFN for PV guest */
> -	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
> -	info->vtermno = HVC_COOKIE;
> -
>   	spin_lock(&xencons_lock);
> -	list_add_tail(&info->list, &xenconsoles);
> +	xen_early_pv_console_init(info, HVC_COOKIE);
>   	spin_unlock(&xencons_lock);
>   
>   	return 0;
> @@ -599,6 +606,18 @@ static int xen_cons_init(void)
>   console_initcall(xen_cons_init);
>   
>   #ifdef CONFIG_EARLY_PRINTK
> +static int __init xenboot_setup_console(struct console *console, char *string)
> +{
> +	static struct xencons_info xenboot;
> +
> +	if (xen_initial_domain())
> +		return 0;
> +	if (!xen_pv_domain())
> +		return -ENODEV;
> +
> +	return xen_early_pv_console_init(&xenboot, 0);
> +}
> +
>   static void xenboot_write_console(struct console *console, const char *string,
>   				  unsigned len)
>   {
> @@ -629,6 +648,7 @@ static void xenboot_write_console(struct console *console, const char *string,
>   struct console xenboot_console = {
>   	.name		= "xenboot",
>   	.write		= xenboot_write_console,
> +	.setup		= xenboot_setup_console,
>   	.flags		= CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
>   	.index		= -1,
>   };
Stefano Stabellini Feb. 26, 2016, 3:39 p.m. UTC | #2
On Thu, 25 Feb 2016, Boris Ostrovsky wrote:
> On 02/25/2016 07:10 AM, Stefano Stabellini wrote:
> > The xenboot early console has been partially broken for DomU for a long
> > time: the output would only go to the hypervisor via hypercall
> > (HYPERVISOR_console_io), while it wouldn't actually go to the DomU
> > console. The reason is that domU_write_console would return early as no
> > xencons structs are configured for it.
> > 
> > Add an appropriate xencons struct for xenboot from the xenboot setup
> > callback.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > ---
> > Changes in v2:
> > - add return to xenboot_setup_console
> > ---
> >   drivers/tty/hvc/hvc_xen.c |   32 ++++++++++++++++++++++++++------
> >   1 file changed, 26 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> > index 68b8ec8..bf787aa 100644
> > --- a/drivers/tty/hvc/hvc_xen.c
> > +++ b/drivers/tty/hvc/hvc_xen.c
> > @@ -246,6 +246,18 @@ err:
> >   	return -ENODEV;
> >   }
> >   +static int xen_early_pv_console_init(struct xencons_info *info, int
> > vtermno)
> 
> Nit: not sure whether "early" is the right word to use here: when called from
> xen_pv_console_init() it's not really on the early path.

What about xencons_info_pv_init?


> Other than that:
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Thanks!


> > +{
> > +	info->evtchn = xen_start_info->console.domU.evtchn;
> > +	/* GFN == MFN for PV guest */
> > +	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
> > +	info->vtermno = vtermno;
> > +
> > +	list_add_tail(&info->list, &xenconsoles);
> > +
> > +	return 0;
> > +}
> > +
> >   static int xen_pv_console_init(void)
> >   {
> >   	struct xencons_info *info;
> > @@ -265,13 +277,8 @@ static int xen_pv_console_init(void)
> >   		/* already configured */
> >   		return 0;
> >   	}
> > -	info->evtchn = xen_start_info->console.domU.evtchn;
> > -	/* GFN == MFN for PV guest */
> > -	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
> > -	info->vtermno = HVC_COOKIE;
> > -
> >   	spin_lock(&xencons_lock);
> > -	list_add_tail(&info->list, &xenconsoles);
> > +	xen_early_pv_console_init(info, HVC_COOKIE);
> >   	spin_unlock(&xencons_lock);
> >     	return 0;
> > @@ -599,6 +606,18 @@ static int xen_cons_init(void)
> >   console_initcall(xen_cons_init);
> >     #ifdef CONFIG_EARLY_PRINTK
> > +static int __init xenboot_setup_console(struct console *console, char
> > *string)
> > +{
> > +	static struct xencons_info xenboot;
> > +
> > +	if (xen_initial_domain())
> > +		return 0;
> > +	if (!xen_pv_domain())
> > +		return -ENODEV;
> > +
> > +	return xen_early_pv_console_init(&xenboot, 0);
> > +}
> > +
> >   static void xenboot_write_console(struct console *console, const char
> > *string,
> >   				  unsigned len)
> >   {
> > @@ -629,6 +648,7 @@ static void xenboot_write_console(struct console
> > *console, const char *string,
> >   struct console xenboot_console = {
> >   	.name		= "xenboot",
> >   	.write		= xenboot_write_console,
> > +	.setup		= xenboot_setup_console,
> >   	.flags		= CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
> >   	.index		= -1,
> >   };
>
Boris Ostrovsky Feb. 26, 2016, 4:09 p.m. UTC | #3
On 02/26/2016 10:39 AM, Stefano Stabellini wrote:
> On Thu, 25 Feb 2016, Boris Ostrovsky wrote:
>> On 02/25/2016 07:10 AM, Stefano Stabellini wrote:
>>> The xenboot early console has been partially broken for DomU for a long
>>> time: the output would only go to the hypervisor via hypercall
>>> (HYPERVISOR_console_io), while it wouldn't actually go to the DomU
>>> console. The reason is that domU_write_console would return early as no
>>> xencons structs are configured for it.
>>>
>>> Add an appropriate xencons struct for xenboot from the xenboot setup
>>> callback.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>
>>> ---
>>> Changes in v2:
>>> - add return to xenboot_setup_console
>>> ---
>>>    drivers/tty/hvc/hvc_xen.c |   32 ++++++++++++++++++++++++++------
>>>    1 file changed, 26 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
>>> index 68b8ec8..bf787aa 100644
>>> --- a/drivers/tty/hvc/hvc_xen.c
>>> +++ b/drivers/tty/hvc/hvc_xen.c
>>> @@ -246,6 +246,18 @@ err:
>>>    	return -ENODEV;
>>>    }
>>>    +static int xen_early_pv_console_init(struct xencons_info *info, int
>>> vtermno)
>> Nit: not sure whether "early" is the right word to use here: when called from
>> xen_pv_console_init() it's not really on the early path.
> What about xencons_info_pv_init?

Sure, that sounds good.

(BTW, I didn't mean that my R-b was conditional on this)


-boris

>
>
>> Other than that:
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Thanks!
>
>
>>> +{
>>> +	info->evtchn = xen_start_info->console.domU.evtchn;
>>> +	/* GFN == MFN for PV guest */
>>> +	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
>>> +	info->vtermno = vtermno;
>>> +
>>> +	list_add_tail(&info->list, &xenconsoles);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static int xen_pv_console_init(void)
>>>    {
>>>    	struct xencons_info *info;
>>> @@ -265,13 +277,8 @@ static int xen_pv_console_init(void)
>>>    		/* already configured */
>>>    		return 0;
>>>    	}
>>> -	info->evtchn = xen_start_info->console.domU.evtchn;
>>> -	/* GFN == MFN for PV guest */
>>> -	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
>>> -	info->vtermno = HVC_COOKIE;
>>> -
>>>    	spin_lock(&xencons_lock);
>>> -	list_add_tail(&info->list, &xenconsoles);
>>> +	xen_early_pv_console_init(info, HVC_COOKIE);
>>>    	spin_unlock(&xencons_lock);
>>>      	return 0;
>>> @@ -599,6 +606,18 @@ static int xen_cons_init(void)
>>>    console_initcall(xen_cons_init);
>>>      #ifdef CONFIG_EARLY_PRINTK
>>> +static int __init xenboot_setup_console(struct console *console, char
>>> *string)
>>> +{
>>> +	static struct xencons_info xenboot;
>>> +
>>> +	if (xen_initial_domain())
>>> +		return 0;
>>> +	if (!xen_pv_domain())
>>> +		return -ENODEV;
>>> +
>>> +	return xen_early_pv_console_init(&xenboot, 0);
>>> +}
>>> +
>>>    static void xenboot_write_console(struct console *console, const char
>>> *string,
>>>    				  unsigned len)
>>>    {
>>> @@ -629,6 +648,7 @@ static void xenboot_write_console(struct console
>>> *console, const char *string,
>>>    struct console xenboot_console = {
>>>    	.name		= "xenboot",
>>>    	.write		= xenboot_write_console,
>>> +	.setup		= xenboot_setup_console,
>>>    	.flags		= CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
>>>    	.index		= -1,
>>>    };
Stefano Stabellini Feb. 26, 2016, 4:57 p.m. UTC | #4
On Fri, 26 Feb 2016, Boris Ostrovsky wrote:
> On 02/26/2016 10:39 AM, Stefano Stabellini wrote:
> > On Thu, 25 Feb 2016, Boris Ostrovsky wrote:
> > > On 02/25/2016 07:10 AM, Stefano Stabellini wrote:
> > > > The xenboot early console has been partially broken for DomU for a long
> > > > time: the output would only go to the hypervisor via hypercall
> > > > (HYPERVISOR_console_io), while it wouldn't actually go to the DomU
> > > > console. The reason is that domU_write_console would return early as no
> > > > xencons structs are configured for it.
> > > > 
> > > > Add an appropriate xencons struct for xenboot from the xenboot setup
> > > > callback.
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > 
> > > > ---
> > > > Changes in v2:
> > > > - add return to xenboot_setup_console
> > > > ---
> > > >    drivers/tty/hvc/hvc_xen.c |   32 ++++++++++++++++++++++++++------
> > > >    1 file changed, 26 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> > > > index 68b8ec8..bf787aa 100644
> > > > --- a/drivers/tty/hvc/hvc_xen.c
> > > > +++ b/drivers/tty/hvc/hvc_xen.c
> > > > @@ -246,6 +246,18 @@ err:
> > > >    	return -ENODEV;
> > > >    }
> > > >    +static int xen_early_pv_console_init(struct xencons_info *info, int
> > > > vtermno)
> > > Nit: not sure whether "early" is the right word to use here: when called
> > > from
> > > xen_pv_console_init() it's not really on the early path.
> > What about xencons_info_pv_init?
> 
> Sure, that sounds good.
> 
> (BTW, I didn't mean that my R-b was conditional on this)

No problem. I made this change and committed the three patches to
for-linus-4.6, based on v4.5-rc5.
diff mbox

Patch

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 68b8ec8..bf787aa 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -246,6 +246,18 @@  err:
 	return -ENODEV;
 }
 
+static int xen_early_pv_console_init(struct xencons_info *info, int vtermno)
+{
+	info->evtchn = xen_start_info->console.domU.evtchn;
+	/* GFN == MFN for PV guest */
+	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
+	info->vtermno = vtermno;
+
+	list_add_tail(&info->list, &xenconsoles);
+
+	return 0;
+}
+
 static int xen_pv_console_init(void)
 {
 	struct xencons_info *info;
@@ -265,13 +277,8 @@  static int xen_pv_console_init(void)
 		/* already configured */
 		return 0;
 	}
-	info->evtchn = xen_start_info->console.domU.evtchn;
-	/* GFN == MFN for PV guest */
-	info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
-	info->vtermno = HVC_COOKIE;
-
 	spin_lock(&xencons_lock);
-	list_add_tail(&info->list, &xenconsoles);
+	xen_early_pv_console_init(info, HVC_COOKIE);
 	spin_unlock(&xencons_lock);
 
 	return 0;
@@ -599,6 +606,18 @@  static int xen_cons_init(void)
 console_initcall(xen_cons_init);
 
 #ifdef CONFIG_EARLY_PRINTK
+static int __init xenboot_setup_console(struct console *console, char *string)
+{
+	static struct xencons_info xenboot;
+
+	if (xen_initial_domain())
+		return 0;
+	if (!xen_pv_domain())
+		return -ENODEV;
+
+	return xen_early_pv_console_init(&xenboot, 0);
+}
+
 static void xenboot_write_console(struct console *console, const char *string,
 				  unsigned len)
 {
@@ -629,6 +648,7 @@  static void xenboot_write_console(struct console *console, const char *string,
 struct console xenboot_console = {
 	.name		= "xenboot",
 	.write		= xenboot_write_console,
+	.setup		= xenboot_setup_console,
 	.flags		= CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
 	.index		= -1,
 };