Re: dom0 bootstrap for xenstore

Re: dom0 bootstrap for xenstore

[Rusty Russell]

On Thu, 2005-06-16 at 06:17 +0100, Keir Fraser wrote:
> > I suggested that we simply mmap /dev/kmem for the xenstored to access
> > the domain0 page for the moment.  That doesn't work: we'll do something
> > else.
> 
> Just use xc_map_foreign_range(), as you would for mapping any other 
> domain's xenstore page.

So here's my patch against latest bk, including test program but we have
an issue.  On unmap, I hit the BUG_ON() on mm/rmap.c:482.  Is this some
issue with using xc_map_foreign_range() on non-foreign pages?

Rusty.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal xen/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c xen-dom0-store/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c
--- xen/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c	2005-06-16 18:03:10.000000000 +1000
+++ xen-dom0-store/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c	2005-06-16 15:12:49.000000000 +1000
@@ -196,6 +196,34 @@ static int privcmd_ioctl(struct inode *i
     }
     break;
 
+    case IOCTL_PRIVCMD_INITDOMAIN_STORE:
+    {
+        extern int do_xenbus_probe(void*);
+
+        if (xen_start_info.store_evtchn != 0) {
+	    ret = -EINVAL;
+	    break;
+	}
+
+	/* Allocate page. */
+	xen_start_info.store_page = get_zeroed_page(GFP_KERNEL);
+	if (!xen_start_info.store_page) {
+	    ret = -ENOMEM;
+	    break;
+	}
+
+	/* Initial connect. Setup channel and page. */
+	xen_start_info.store_evtchn = data;
+	ret = pfn_to_mfn(virt_to_phys((void *)xen_start_info.store_page)
+			 >> PAGE_SHIFT);
+
+	strcpy((char *)xen_start_info.store_page, "HELLO THERE!");
+
+	/* We'll return then this will wait for daemon to answer */
+	//kthread_run(do_xenbus_probe, NULL, "xenbus_probe");
+    }
+    break;
+
     default:
         ret = -EINVAL;
         break;
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal xen/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h xen-dom0-store/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h
--- xen/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h	2005-06-16 18:03:13.000000000 +1000
+++ xen-dom0-store/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h	2005-06-16 13:47:48.000000000 +1000
@@ -84,5 +84,7 @@ typedef struct privcmd_blkmsg
     _IOC(_IOC_NONE, 'P', 3, sizeof(privcmd_mmapbatch_t))
 #define IOCTL_PRIVCMD_GET_MACH2PHYS_START_MFN \
     _IOC(_IOC_READ, 'P', 4, sizeof(unsigned long))
+#define IOCTL_PRIVCMD_INITDOMAIN_STORE \
+    _IOC(_IOC_READ, 'P', 5, 0)
 
 #endif /* __PRIVCMD_H__ */
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal xen/tools/xenstore/Makefile xen-dom0-store/tools/xenstore/Makefile
--- xen/tools/xenstore/Makefile	2005-06-16 18:03:27.000000000 +1000
+++ xen-dom0-store/tools/xenstore/Makefile	2005-06-16 14:46:51.000000000 +1000
@@ -82,18 +82,22 @@ stresstest: xs_stress xenstored_test
 	rm -rf $(TESTDIR)/store
 	export $(TESTENV); PID=`./xenstored_test --output-pid`; ./xs_stress 10000; ret=$$?; kill $$PID; exit $$ret
 
+xs_dom0_test: xs_dom0_test.o utils.o
+	$(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -lxc -o $@
+
 TAGS:
 	etags `find . -name '*.[ch]'`
 
 tarball: clean
 	cd .. && tar -c -j -v -h -f xenstore.tar.bz2 xenstore/
 
-install: xenstored libxenstore.a
+install: xenstored libxenstore.a xs_dom0_test
 	$(INSTALL_DIR) -p $(DESTDIR)/var/run/xenstored
 	$(INSTALL_DIR) -p $(DESTDIR)/var/lib/xenstored
 	$(INSTALL_DIR) -p $(DESTDIR)/usr/sbin
 	$(INSTALL_DIR) -p $(DESTDIR)/usr/include
 	$(INSTALL_PROG) xenstored $(DESTDIR)/usr/sbin
+	$(INSTALL_PROG) xs_dom0_test $(DESTDIR)/usr/sbin
 	$(INSTALL_DIR) -p $(DESTDIR)/usr/$(LIBDIR)
 	$(INSTALL_DATA) libxenstore.a $(DESTDIR)/usr/$(LIBDIR)
 	$(INSTALL_DATA) xs.h $(DESTDIR)/usr/include
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal xen/tools/xenstore/xs_dom0_test.c xen-dom0-store/tools/xenstore/xs_dom0_test.c
--- xen/tools/xenstore/xs_dom0_test.c	1970-01-01 10:00:00.000000000 +1000
+++ xen-dom0-store/tools/xenstore/xs_dom0_test.c	2005-06-16 16:51:00.000000000 +1000
@@ -0,0 +1,43 @@
+/* Test introduction of domain 0 */
+#include <linux/ioctl.h>
+#include <sys/ioctl.h>
+#include "xs.h"
+#include "utils.h"
+#include <xc.h>
+#include <xen/linux/privcmd.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/mman.h>
+
+int main()
+{
+	int h, local = 0, kernel = 0;
+	long err;
+	void *page;
+
+	h = xc_interface_open();
+	if (h < 0)
+		barf_perror("Failed to open xc");
+
+	if (xc_evtchn_bind_interdomain(h, DOMID_SELF, 0, &local, &kernel) != 0)
+		barf_perror("Failed to bind interdomain");
+
+	printf("Got ports %i & %i\n", local, kernel);
+
+	err = ioctl(h, IOCTL_PRIVCMD_INITDOMAIN_STORE, kernel);
+	if (err < 0)
+		barf_perror("Failed to initialize store");
+	printf("Got mfn %li\n", err);
+
+	page = xc_map_foreign_range(h, 0, getpagesize(), PROT_READ|PROT_WRITE,
+				    err);
+	if (!page)
+		barf_perror("Failed to map page %li", err);
+	printf("Mapped page at %p\n", page);
+	printf("Page says %s\n", (char *)page);
+	munmap(page, getpagesize());
+	printf("unmapped\n");
+	
+	return 0;
+}
+	

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: dom0 bootstrap for xenstore

[Keir Fraser]

On 16 Jun 2005, at 09:11, Rusty Russell wrote:

> So here's my patch against latest bk, including test program but we 
> have
> an issue.  On unmap, I hit the BUG_ON() on mm/rmap.c:482.  Is this some
> issue with using xc_map_foreign_range() on non-foreign pages?

Ah, it's not working because xc_map_foreign_range does not raise 
refcnts on the page being mapped. unmap_range then does the right thing 
for foreign pages, and things like the local domain's shared_info page 
which is really a Xen heap page, but will do the wrong thing for 
ordinary pages out of the local domain's page allocator (like the 
xenstored page). It will try to tear down rmap, drop refcnts, atc and 
get itself into a mess.

On XenLinux, /dev/mem and /dev/kmem only work for mapping I/O memory.

Either we need a new interface for mmaping local xenstore page, or we 
need to hack one of the existing /dev/mem or xc_map_foreign_range 
ioctl. None of these options is very pleasant. :-(  I think simplest 
would be to extend the gross /proc/xen/privcmd to have a mmap that we 
can use to mmap xenstore page (maybe make a little more general purpose 
by turning offset field of mmap into an enumeration, so we can map 
other useful Xen-specific bits and bobs via it in the future).

  -- Keir

Re: Re: dom0 bootstrap for xenstore

[Keir Fraser]

On 16 Jun 2005, at 14:55, Keir Fraser wrote:

> Either we need a new interface for mmaping local xenstore page, or we 
> need to hack one of the existing /dev/mem or xc_map_foreign_range 
> ioctl. None of these options is very pleasant. :-(  I think simplest 
> would be to extend the gross /proc/xen/privcmd to have a mmap that we 
> can use to mmap xenstore page (maybe make a little more general 
> purpose by turning offset field of mmap into an enumeration, so we can 
> map other useful Xen-specific bits and bobs via it in the future).

Brainwave: what if you mark the xenstore page as PageReserved? This is 
easy to do if you allocate the page from within the kernel (rather than 
user space) as you can mark it reserved at the same time you allocate 
it. Then you ought to be able to use xc_map_foreign_page without 
problems. And it makes sense -- the xenstore page is not a normal 
page-cache/VM page.

I think this is a one-line addition to your previous patch. :-)

  -- Keir

Re: dom0 bootstrap for xenstore

[Mike Wray]

Keir Fraser wrote:
> 
> On 16 Jun 2005, at 09:11, Rusty Russell wrote:
> 
>> So here's my patch against latest bk, including test program but we have
>> an issue.  On unmap, I hit the BUG_ON() on mm/rmap.c:482.  Is this some
>> issue with using xc_map_foreign_range() on non-foreign pages?
> 
> 
> Ah, it's not working because xc_map_foreign_range does not raise refcnts 
> on the page being mapped. unmap_range then does the right thing for 
> foreign pages, and things like the local domain's shared_info page which 
> is really a Xen heap page, but will do the wrong thing for ordinary 
> pages out of the local domain's page allocator (like the xenstored 
> page). It will try to tear down rmap, drop refcnts, atc and get itself 
> into a mess.

Isn't it possible to fix xc_map_foreign_range() to do the right thing for
'ordinary' pages?

> On XenLinux, /dev/mem and /dev/kmem only work for mapping I/O memory.
> 
> Either we need a new interface for mmaping local xenstore page, or we 
> need to hack one of the existing /dev/mem or xc_map_foreign_range ioctl. 
> None of these options is very pleasant. :-(  I think simplest would be 
> to extend the gross /proc/xen/privcmd to have a mmap that we can use to 
> mmap xenstore page (maybe make a little more general purpose by turning 
> offset field of mmap into an enumeration, so we can map other useful 
> Xen-specific bits and bobs via it in the future).

OK, if xc_map_foreign_range() isn't fixable I guess we have to do something
like that.

Mike

Re: dom0 bootstrap for xenstore

[Keir Fraser]

On 16 Jun 2005, at 16:54, Mike Wray wrote:

>> Ah, it's not working because xc_map_foreign_range does not raise 
>> refcnts on the page being mapped. unmap_range then does the right 
>> thing for foreign pages, and things like the local domain's 
>> shared_info page which is really a Xen heap page, but will do the 
>> wrong thing for ordinary pages out of the local domain's page 
>> allocator (like the xenstored page). It will try to tear down rmap, 
>> drop refcnts, atc and get itself into a mess.
>
> Isn't it possible to fix xc_map_foreign_range() to do the right thing 
> for
> 'ordinary' pages?

I think PageReserved will fix this so we can use xc_map_foreign_range. 
The underlying ioctl should definitely have a safety catch added, to 
avoid inadvertently mapping local non-reserved pages.

  -- Keir

Re: Re: dom0 bootstrap for xenstore

[Rusty Russell]

On Thu, 2005-06-16 at 14:55 +0100, Keir Fraser wrote:
> Either we need a new interface for mmaping local xenstore page, or we 
> need to hack one of the existing /dev/mem or xc_map_foreign_range 
> ioctl. None of these options is very pleasant. :-(  I think simplest 
> would be to extend the gross /proc/xen/privcmd to have a mmap that we 
> can use to mmap xenstore page (maybe make a little more general purpose 
> by turning offset field of mmap into an enumeration, so we can map 
> other useful Xen-specific bits and bobs via it in the future).

Yes, that was going to be my next step.  It's nasty also because it
means a second path in the xenstore code, but it'll work.

I'll write that instead.
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: dom0 bootstrap for xenstore

[Rusty Russell]

On Thu, 2005-06-16 at 14:55 +0100, Keir Fraser wrote:
> On 16 Jun 2005, at 09:11, Rusty Russell wrote:
> 
> > So here's my patch against latest bk, including test program but we 
> > have
> > an issue.  On unmap, I hit the BUG_ON() on mm/rmap.c:482.  Is this some
> > issue with using xc_map_foreign_range() on non-foreign pages?
> 
> Ah, it's not working because xc_map_foreign_range does not raise 
> refcnts on the page being mapped.

OK, fixed, please apply the below patch against current bk.

Mike, this means backing out your changes to use domain_info to
communicate this.  I didn't try to extricate that...

Thanks,
Rusty.

diff -urpN --exclude dist --exclude html --exclude ps --exclude '*-xen0' --exclude '*-xenU' --exclude 'pristine-*' --exclude TAGS --exclude '*.o' --exclude asm-offsets.h --exclude asm-offsets.s --exclude .chkbuild --exclude '*~' --exclude '.*.d' --exclude classlist.h --exclude devlist.h --exclude asm --exclude banner.h --exclude 'ref-*' --exclude .makedep --exclude config-host.h --exclude config-host.mak --exclude keysym_adapter_sdl.h --exclude config.h --exclude config.mak --exclude pygrub --exclude bzimage_header.c --exclude build --exclude compile.h --minimal xen/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c xen-dom0-store/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c
--- xen/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c	2005-06-16 18:03:10.000000000 +1000
+++ xen-dom0-store/linux-2.6.11-xen-sparse/drivers/xen/privcmd/privcmd.c	2005-06-17 17:11:06.000000000 +1000
@@ -196,6 +196,36 @@ static int privcmd_ioctl(struct inode *i
     }
     break;
 
+    case IOCTL_PRIVCMD_INITDOMAIN_STORE:
+    {
+        extern int do_xenbus_probe(void*);
+
+        if (xen_start_info.store_evtchn != 0) {
+	    ret = -EINVAL;
+	    break;
+	}
+
+	/* Allocate page. */
+	xen_start_info.store_page = get_zeroed_page(GFP_KERNEL);
+	if (!xen_start_info.store_page) {
+	    ret = -ENOMEM;
+	    break;
+	}
+
+	/* We don't refcnt properly, so set reserved on page (this
+	 * allocation is permanent). */
+	SetPageReserved(virt_to_page(xen_start_info.store_page));
+
+	/* Initial connect. Setup channel and page. */
+	xen_start_info.store_evtchn = data;
+	ret = pfn_to_mfn(virt_to_phys((void *)xen_start_info.store_page)
+			 >> PAGE_SHIFT);
+
+	/* We'll return then this will wait for daemon to answer */
+	//kthread_run(do_xenbus_probe, NULL, "xenbus_probe");
+    }
+    break;
+
     default:
         ret = -EINVAL;
         break;
diff -urpN --exclude dist --exclude html --exclude ps --exclude '*-xen0' --exclude '*-xenU' --exclude 'pristine-*' --exclude TAGS --exclude '*.o' --exclude asm-offsets.h --exclude asm-offsets.s --exclude .chkbuild --exclude '*~' --exclude '.*.d' --exclude classlist.h --exclude devlist.h --exclude asm --exclude banner.h --exclude 'ref-*' --exclude .makedep --exclude config-host.h --exclude config-host.mak --exclude keysym_adapter_sdl.h --exclude config.h --exclude config.mak --exclude pygrub --exclude bzimage_header.c --exclude build --exclude compile.h --minimal xen/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h xen-dom0-store/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h
--- xen/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h	2005-06-16 18:03:13.000000000 +1000
+++ xen-dom0-store/linux-2.6.11-xen-sparse/include/asm-xen/linux-public/privcmd.h	2005-06-16 13:47:48.000000000 +1000
@@ -84,5 +84,7 @@ typedef struct privcmd_blkmsg
     _IOC(_IOC_NONE, 'P', 3, sizeof(privcmd_mmapbatch_t))
 #define IOCTL_PRIVCMD_GET_MACH2PHYS_START_MFN \
     _IOC(_IOC_READ, 'P', 4, sizeof(unsigned long))
+#define IOCTL_PRIVCMD_INITDOMAIN_STORE \
+    _IOC(_IOC_READ, 'P', 5, 0)
 
 #endif /* __PRIVCMD_H__ */

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: dom0 bootstrap for xenstore

[Mike Wray]

Rusty Russell wrote:
> On Thu, 2005-06-16 at 14:55 +0100, Keir Fraser wrote:
> 
>>On 16 Jun 2005, at 09:11, Rusty Russell wrote:
>>
>>
>>>So here's my patch against latest bk, including test program but we 
>>>have
>>>an issue.  On unmap, I hit the BUG_ON() on mm/rmap.c:482.  Is this some
>>>issue with using xc_map_foreign_range() on non-foreign pages?
>>
>>Ah, it's not working because xc_map_foreign_range does not raise 
>>refcnts on the page being mapped.
> 
> 
> OK, fixed, please apply the below patch against current bk.
> 
> Mike, this means backing out your changes to use domain_info to
> communicate this.  I didn't try to extricate that...
> 

OK, I fixed up the domain introduce code to use the privcmd for
dom0 and applied the change to make the page reserved.
The good news is that introducing dom0 to xenstore now works, and
mapping the store page works too.
The bad news is that the dom0 xenbus probe oopses as follows:

bounds: 0000 [#1]
PREEMPT
Modules linked in:
CPU:    0
EIP:    0061:[<c055d940>]    Not tainted VLI
EFLAGS: 00010246   (2.6.11.11-xen0)
EIP is at xenbus_probe_init+0x0/0x80
eax: 00000000   ebx: c5544000   ecx: 00000002   edx: 00000000
esi: c5a0fd10   edi: 00000000   ebp: c02c0ac0   esp: c5545fc4
ds: 007b   es: 007b   ss: 0069
Process xenbus_probe (pid: 2310, threadinfo=c5544000 task=c5521a00)
Stack: c01332ca 00000000 c5545fd8 00000000 fffffffc ffffffff ffffffff c0133220
        00000000 00000000 00000000 c0107b75 c5a0fd10 00000000 00000000
Call Trace:
  [<c01332ca>] kthread+0xaa/0xb0
  [<c0133220>] kthread+0x0/0xb0
  [<c0107b75>] kernel_thread_helper+0x5/0x10
Code: 75 67 69 6e 2e 73 6f 00 6c 69 62 6b 6f 6e 71 5f 73 69 64 65 62 61 72 5f 74 72 65 65 2
e 73 6f 00 2f 75 73 72 2f 6c 69 62 2f 6c 69 <62> 6b 6f 6e 71 5f 73 69 64 65 62 61 72 5f 74
72 65 65 2e 73 6f

Regards,

Mike

Re: dom0 bootstrap for xenstore

[Rusty Russell]

On Fri, 2005-06-17 at 14:00 +0100, Mike Wray wrote:
> OK, I fixed up the domain introduce code to use the privcmd for
> dom0 and applied the change to make the page reserved.
> The good news is that introducing dom0 to xenstore now works, and
> mapping the store page works too.
> The bad news is that the dom0 xenbus probe oopses as follows:

Almost certainly because it's calling into an __init function, which has
been removed.  I reworked that code and checked it in...

Cheers!
Rusty.
PS.  I've done the Great Renaming, now looking for other differences to
minimize...
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: dom0 bootstrap for xenstore

[Mike Wray]

Rusty Russell wrote:
> On Fri, 2005-06-17 at 14:00 +0100, Mike Wray wrote:
> 
>>OK, I fixed up the domain introduce code to use the privcmd for
>>dom0 and applied the change to make the page reserved.
>>The good news is that introducing dom0 to xenstore now works, and
>>mapping the store page works too.
>>The bad news is that the dom0 xenbus probe oopses as follows:
> 
> 
> Almost certainly because it's calling into an __init function, which has
> been removed.  I reworked that code and checked it in...

OK, I did wonder whether that might the case.
The oops has gone away and the dom-0 xenbus connects up OK.

Mike