linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[Mukesh Rathor]

Hi,

Not sure if this already got reported, but it appears 

commit b7dd0e3
Author: David Vrabel <david.vrabel@citrix.com>
Date:   Fri Jul 11 16:42:34 2014 +0100

    x86/xen: safely map and unmap grant frames when in atomic context

has broken boot of domU PVH. The reason being arch_gnttab_map_shared
is getting called before gnttab_shared_vm_area gets initialized by
arch_gnttab_init(). As the comment says,

--->
/* Call it _before_ __gnttab_init as we need to initialize the
 *  * xen_auto_xlat_grant_frames first. */
core_initcall(xen_pvh_gnttab_setup);
<---

we can't wait till arch_gnttab_init is called by __gnttab_init().

I'm also wondering why didn't this get caught by a simple boot test
of PVH domU before this code was merged? 

thanks,
Mukesh

Re: linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[David Vrabel]

On 02/08/14 00:46, Mukesh Rathor wrote:
> Hi,
> 
> Not sure if this already got reported, but it appears 
> 
> commit b7dd0e3
> Author: David Vrabel <david.vrabel@citrix.com>
> Date:   Fri Jul 11 16:42:34 2014 +0100
> 
>     x86/xen: safely map and unmap grant frames when in atomic context
> 
> has broken boot of domU PVH. The reason being arch_gnttab_map_shared
> is getting called before gnttab_shared_vm_area gets initialized by
> arch_gnttab_init(). As the comment says,

Well that makes no sense since all those functions are PV only.  This
patch did not change how PVH guests were initialized.

David

Re: linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[Konrad Rzeszutek Wilk]

On Mon, Aug 04, 2014 at 11:33:54AM +0100, David Vrabel wrote:
> On 02/08/14 00:46, Mukesh Rathor wrote:
> > Hi,
> > 
> > Not sure if this already got reported, but it appears 

No it has not. Thank you for reporting it!
> > 
> > commit b7dd0e3
> > Author: David Vrabel <david.vrabel@citrix.com>
> > Date:   Fri Jul 11 16:42:34 2014 +0100
> > 
> >     x86/xen: safely map and unmap grant frames when in atomic context
> > 
> > has broken boot of domU PVH. The reason being arch_gnttab_map_shared
> > is getting called before gnttab_shared_vm_area gets initialized by
> > arch_gnttab_init(). As the comment says,
> 
> Well that makes no sense since all those functions are PV only.  This
> patch did not change how PVH guests were initialized.

PVH is PV and HVM. It uses the same paths and then some! :-) See
6926f6d6109714aab7b26df7099b12555e36676f as the grant mechanism
is HVM. It could be reworked so that '__gnttab_init' ends up
calling xlated_setup_gnttab_pages?

Mukesh, the reason it did not get caught is that upstream Xen is
broken in as the libxl patch that Roger had posted has not yet
been committed. This is 'libxl: create PVH guests with max memory assigned'.

> 
> David
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[David Vrabel]

On 02/08/14 00:46, Mukesh Rathor wrote:
> Hi,
> 
> Not sure if this already got reported, but it appears 
> 
> commit b7dd0e3
> Author: David Vrabel <david.vrabel@citrix.com>
> Date:   Fri Jul 11 16:42:34 2014 +0100
> 
>     x86/xen: safely map and unmap grant frames when in atomic context
> 
> has broken boot of domU PVH. The reason being arch_gnttab_map_shared
> is getting called before gnttab_shared_vm_area gets initialized by
> arch_gnttab_init(). As the comment says,
> 
> --->
> /* Call it _before_ __gnttab_init as we need to initialize the
>  *  * xen_auto_xlat_grant_frames first. */
> core_initcall(xen_pvh_gnttab_setup);
> <---
> 
> we can't wait till arch_gnttab_init is called by __gnttab_init().

Does this fix it?  I can't test it because the only box I have easily
available for testing is AMD.

David

8<--------------------------------
x86/xen: use vmap() to map grant table pages in PVH guests

Commit b7dd0e350e0b (x86/xen: safely map and unmap grant frames when
in atomic context) causes PVH guests to crash in
arch_gnttab_map_shared() when they attempted to map the pages for the
grant table.

This use of a PV-specific function during the PVH grant table setup is
non-obvious and not needed.  The standard vmap() function does the
right thing.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Reported-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 arch/x86/xen/grant-table.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c
index ebfa9b2..767c9cb 100644
--- a/arch/x86/xen/grant-table.c
+++ b/arch/x86/xen/grant-table.c
@@ -168,6 +168,7 @@ static int __init xlated_setup_gnttab_pages(void)
 {
 	struct page **pages;
 	xen_pfn_t *pfns;
+	void *vaddr;
 	int rc;
 	unsigned int i;
 	unsigned long nr_grant_frames = gnttab_max_grant_frames();
@@ -193,21 +194,20 @@ static int __init xlated_setup_gnttab_pages(void)
 	for (i = 0; i < nr_grant_frames; i++)
 		pfns[i] = page_to_pfn(pages[i]);
 
-	rc = arch_gnttab_map_shared(pfns, nr_grant_frames, nr_grant_frames,
-				    &xen_auto_xlat_grant_frames.vaddr);
-
-	if (rc) {
+	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
+	if (!vaddr) {
 		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
 			nr_grant_frames, rc);
 		free_xenballooned_pages(nr_grant_frames, pages);
 		kfree(pages);
 		kfree(pfns);
-		return rc;
+		return -ENOMEM;
 	}
 	kfree(pages);
 
 	xen_auto_xlat_grant_frames.pfn = pfns;
 	xen_auto_xlat_grant_frames.count = nr_grant_frames;
+	xen_auto_xlat_grant_frames.vaddr = vaddr;
 
 	return 0;
 }
-- 
1.7.10.4

Re: linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[Mukesh Rathor]

On Tue, 5 Aug 2014 12:00:12 +0100
David Vrabel <david.vrabel@citrix.com> wrote:

> On 02/08/14 00:46, Mukesh Rathor wrote:
> > Hi,
> > 
> > Not sure if this already got reported, but it appears 
> > 
> > commit b7dd0e3
> > Author: David Vrabel <david.vrabel@citrix.com>
> > Date:   Fri Jul 11 16:42:34 2014 +0100
> > 
> >     x86/xen: safely map and unmap grant frames when in atomic
> > context
> > 
> > has broken boot of domU PVH. The reason being arch_gnttab_map_shared
> > is getting called before gnttab_shared_vm_area gets initialized by
> > arch_gnttab_init(). As the comment says,
> > 
> > --->
> > /* Call it _before_ __gnttab_init as we need to initialize the
> >  *  * xen_auto_xlat_grant_frames first. */
> > core_initcall(xen_pvh_gnttab_setup);
> > <---
> > 
> > we can't wait till arch_gnttab_init is called by __gnttab_init().
> 
> Does this fix it?  I can't test it because the only box I have easily
> available for testing is AMD.
> 
> David
> 

Well, I don't see the crash, but guest hangs. My hunch is the pte
entries that are populated are not correct, but I don't have time to
look into it right now, so I'll let you debug.

[    1.905278] systemd[1]: Started dracut pre-udev hook.
[    1.905870] systemd[1]: Starting udev Kernel Device Manager...
[    1.945204] systemd-udevd[275]: starting version 204
[    2.625252] systemd-sysctl (526) used greatest stack depth: 13192 bytes left
[   49.832064] random: nonblocking pool is initialized
.. hang.. 

thanks
mukesh

Re: linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[Konrad Rzeszutek Wilk]

On Wed, Aug 06, 2014 at 02:23:25PM -0700, Mukesh Rathor wrote:
> On Tue, 5 Aug 2014 12:00:12 +0100
> David Vrabel <david.vrabel@citrix.com> wrote:
> 
> > On 02/08/14 00:46, Mukesh Rathor wrote:
> > > Hi,
> > > 
> > > Not sure if this already got reported, but it appears 
> > > 
> > > commit b7dd0e3
> > > Author: David Vrabel <david.vrabel@citrix.com>
> > > Date:   Fri Jul 11 16:42:34 2014 +0100
> > > 
> > >     x86/xen: safely map and unmap grant frames when in atomic
> > > context
> > > 
> > > has broken boot of domU PVH. The reason being arch_gnttab_map_shared
> > > is getting called before gnttab_shared_vm_area gets initialized by
> > > arch_gnttab_init(). As the comment says,
> > > 
> > > --->
> > > /* Call it _before_ __gnttab_init as we need to initialize the
> > >  *  * xen_auto_xlat_grant_frames first. */
> > > core_initcall(xen_pvh_gnttab_setup);
> > > <---
> > > 
> > > we can't wait till arch_gnttab_init is called by __gnttab_init().
> > 
> > Does this fix it?  I can't test it because the only box I have easily
> > available for testing is AMD.
> > 
> > David
> > 
> 
> Well, I don't see the crash, but guest hangs. My hunch is the pte
> entries that are populated are not correct, but I don't have time to
> look into it right now, so I'll let you debug.

If you revert David's patch (x86/xen: safely map and unmap ..) does it
continue booting without the issue below?

Thanks!
> 
> [    1.905278] systemd[1]: Started dracut pre-udev hook.
> [    1.905870] systemd[1]: Starting udev Kernel Device Manager...
> [    1.945204] systemd-udevd[275]: starting version 204
> [    2.625252] systemd-sysctl (526) used greatest stack depth: 13192 bytes left
> [   49.832064] random: nonblocking pool is initialized
> .. hang.. 
> 
> thanks
> mukesh
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[David Vrabel]

On 06/08/14 22:23, Mukesh Rathor wrote:
> On Tue, 5 Aug 2014 12:00:12 +0100
> David Vrabel <david.vrabel@citrix.com> wrote:
> 
>> On 02/08/14 00:46, Mukesh Rathor wrote:
>>> Hi,
>>>
>>> Not sure if this already got reported, but it appears 
>>>
>>> commit b7dd0e3
>>> Author: David Vrabel <david.vrabel@citrix.com>
>>> Date:   Fri Jul 11 16:42:34 2014 +0100
>>>
>>>     x86/xen: safely map and unmap grant frames when in atomic
>>> context
>>>
>>> has broken boot of domU PVH. The reason being arch_gnttab_map_shared
>>> is getting called before gnttab_shared_vm_area gets initialized by
>>> arch_gnttab_init(). As the comment says,
>>>
>>> --->
>>> /* Call it _before_ __gnttab_init as we need to initialize the
>>>  *  * xen_auto_xlat_grant_frames first. */
>>> core_initcall(xen_pvh_gnttab_setup);
>>> <---
>>>
>>> we can't wait till arch_gnttab_init is called by __gnttab_init().
>>
>> Does this fix it?  I can't test it because the only box I have easily
>> available for testing is AMD.
>>
>> David
>>
> 
> Well, I don't see the crash, but guest hangs. My hunch is the pte
> entries that are populated are not correct, but I don't have time to
> look into it right now, so I'll let you debug.

If you don't care about working PVH support in Linux, someone else had
better step up and take care of it or its just going to remain broken
and I'll eventually have to consider removing it as unmaintained.

David

Re: linux PVH broken in 3.16.0-rc7 (arch_gnttab_map_shared)

[Mukesh Rathor]

On Wed, 6 Aug 2014 14:23:25 -0700
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Tue, 5 Aug 2014 12:00:12 +0100
> David Vrabel <david.vrabel@citrix.com> wrote:
> 
> > On 02/08/14 00:46, Mukesh Rathor wrote:
> > > Hi,
> > > 
> > > Not sure if this already got reported, but it appears 
> > > 
> > > commit b7dd0e3
> > > Author: David Vrabel <david.vrabel@citrix.com>
> > > Date:   Fri Jul 11 16:42:34 2014 +0100
> > > 
> > >     x86/xen: safely map and unmap grant frames when in atomic
> > > context
> > > 
> > > has broken boot of domU PVH. The reason being
> > > arch_gnttab_map_shared is getting called before
> > > gnttab_shared_vm_area gets initialized by arch_gnttab_init(). As
> > > the comment says,
> > > 
> > > --->
> > > /* Call it _before_ __gnttab_init as we need to initialize the
> > >  *  * xen_auto_xlat_grant_frames first. */
> > > core_initcall(xen_pvh_gnttab_setup);
> > > <---
> > > 
> > > we can't wait till arch_gnttab_init is called by __gnttab_init().
> > 
> > Does this fix it?  I can't test it because the only box I have
> > easily available for testing is AMD.
> > 
> > David
> > 
> 
> Well, I don't see the crash, but guest hangs. My hunch is the pte
> entries that are populated are not correct, but I don't have time to
> look into it right now, so I'll let you debug.
> 
> [    1.905278] systemd[1]: Started dracut pre-udev hook.
> [    1.905870] systemd[1]: Starting udev Kernel Device Manager...
> [    1.945204] systemd-udevd[275]: starting version 204
> [    2.625252] systemd-sysctl (526) used greatest stack depth: 13192
> bytes left [   49.832064] random: nonblocking pool is initialized
> .. hang.. 

Actually, my bad, I accidentally had older 3.16-rc7 initrd, but it
works with proper initrd. So:

Tested-by: Mukesh Rathor <mukesh.rathor@oracle.com>

thanks
Mukesh