Re: [XenPPC] [PATCH 1/3] libxc: add start_info_t node to devtree

2007-01-25 Thread Ryan Harper
* Jimi Xenidis [EMAIL PROTECTED] [2007-01-24 12:42]:
 The data that was in start_info_t should be just simple bindings in / 
 xen since they describe xen, there is no need to create a new node.
 many of the props are not needed since they are expressed elsewhere  
 in the devtree or perhaps differently.
 more below.

New rev:

-dropped /xen/start_info_t, hanging new node /xen/store
-fixed up /xen/console/reg to be proper u64 baseu64 size value
-fixed up /xen/console/interrupts to use value passed from xend
-renamed property 'shared_info' to 'shared-info'
-fixed 'shared-info' to be a proper u64 baseu64 size value
-reduced the number of pages reserved at the end of RMA from 4 to 3
 as we no longer need to reserve a page for start_info_t

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253
[EMAIL PROTECTED]


diffstat output:
 mk_flatdevtree.c |   45 +++--
 mk_flatdevtree.h |6 ++-
 xc_linux_build.c |   83 ++-
 3 files changed, 80 insertions(+), 54 deletions(-)

Signed-off-by: Ryan Harper [EMAIL PROTECTED]
---
diff -r ed5ee9dde0bd tools/libxc/powerpc64/mk_flatdevtree.c
--- a/tools/libxc/powerpc64/mk_flatdevtree.cSun Jan 21 08:17:46 2007 -0500
+++ b/tools/libxc/powerpc64/mk_flatdevtree.cThu Jan 25 11:57:41 2007 -0600
@@ -316,13 +316,16 @@ int make_devtree(struct ft_cxt *root,
  unsigned long shadow_mb,
  unsigned long initrd_base,
  unsigned long initrd_len,
- const char *bootargs)
+ const char *bootargs,
+ unsigned long console_evtchn,
+ unsigned long store_evtchn,
+ unsigned long nr_pages)
 {
 struct boot_param_header *bph = NULL;
 uint64_t val[2];
 uint32_t val32[2];
 unsigned long remaining;
-unsigned long rma_reserve = 4 * PAGE_SIZE;
+unsigned long rma_reserve = 3 * PAGE_SIZE;
 unsigned long initrd_end = initrd_base + initrd_len;
 int64_t shadow_mb_log;
 uint64_t pft_size;
@@ -332,6 +335,9 @@ int make_devtree(struct ft_cxt *root,
 char *cpuname = NULL;
 int saved_errno;
 int dtb_fd = -1;
+uint64_t shared_info_addr = (rma_bytes - PAGE_SIZE);
+uint64_t store_mfn = (rma_bytes - (2*PAGE_SIZE))  PAGE_SHIFT;
+uint64_t console_mfn = (rma_bytes - (3*PAGE_SIZE))  PAGE_SHIFT;
 uint32_t cpu0_phandle = get_phandle();
 uint32_t xen_phandle = get_phandle();
 uint32_t rma_phandle = get_phandle();
@@ -419,11 +425,6 @@ int make_devtree(struct ft_cxt *root,
 /* xen = root.addnode('xen') */
 ft_begin_node(root, xen);
 
-/* start-info is the first page in the RMA reserved area */
-val[0] = cpu_to_be64((u64) (rma_bytes - rma_reserve));
-val[1] = cpu_to_be64((u64) PAGE_SIZE);
-ft_prop(root, start-info, val, sizeof(val));
-
 /*  xen.addprop('version', 'Xen-3.0-unstable\0') */
 ft_prop_str(root, version, Xen-3.0-unstable);
 
@@ -432,6 +433,11 @@ int make_devtree(struct ft_cxt *root,
 val[1] = cpu_to_be64((u64) 0);
 ft_prop(root, reg, val, sizeof(val));
 
+/* point to shared_info_t page base addr */
+val[0] = cpu_to_be64((u64) shared_info_addr);
+val[1] = cpu_to_be64((u64) PAGE_SIZE);
+ft_prop(root, shared-info, val, sizeof(val));
+
 /* xen.addprop('domain-name', imghandler.vm.getName() + '\0') */
 /* libxc doesn't know the domain name, that is purely a xend thing */
 /* ft_prop_str(root, domain-name, domain_name); */
@@ -442,12 +448,33 @@ int make_devtree(struct ft_cxt *root,
 /* xencons = xen.addnode('console') */
 ft_begin_node(root, console);
 
-/* xencons.addprop('interrupts', 1, 0) */
-val32[0] = cpu_to_be32((u32) 1);
+/* console_mfn */
+val[0] = cpu_to_be64((u64) console_mfn);
+val[1] = cpu_to_be64((u64) PAGE_SIZE);
+ft_prop(root, reg, val, sizeof(val));
+
+/* xencons.addprop('interrupts', console_evtchn, 0) */
+val32[0] = cpu_to_be32((u32) console_evtchn);
 val32[1] = cpu_to_be32((u32) 0);
 ft_prop(root, interrupts, val32, sizeof(val32));
 
 /* end of console */
+ft_end_node(root);
+
+/* start store node */
+ft_begin_node(root, store);
+
+/* store_mfn */
+val[0] = cpu_to_be64((u64) store_mfn);
+val[1] = cpu_to_be64((u64) PAGE_SIZE);
+ft_prop(root, reg, val, sizeof(val));
+
+/* store event channel */
+val32[0] = cpu_to_be32((u32) store_evtchn);
+val32[1] = cpu_to_be32((u32) 0);
+ft_prop(root, interrupts, val32, sizeof(val32));
+
+/* end of store */
 ft_end_node(root);
 
 /* end of xen node */
diff -r ed5ee9dde0bd tools/libxc/powerpc64/mk_flatdevtree.h
--- a/tools/libxc/powerpc64/mk_flatdevtree.hSun Jan 21 08:17:46 2007 -0500
+++ b/tools/libxc/powerpc64/mk_flatdevtree.hThu Jan 25 11:27:58 2007 -0600
@@ -32,8 +32,10 @@ extern int make_devtree(struct ft_cxt *r
 unsigned long 

Re: [XenPPC] [PATCH 1/3] libxc: add start_info_t node to devtree

2007-01-25 Thread Jimi Xenidis

Thanks _a_lot_ Ryan.. this stuff is really tedious.
Just a few more things.

BTW: what about start_info-flags = SIF_PRIVILEGED or  
SIF_INITDOMAIN.  You will not need properties now but you will need  
to make sure there absence is recognized in linux/.../setup.c.


On Jan 25, 2007, at 2:16 PM, Ryan Harper wrote:


* Jimi Xenidis [EMAIL PROTECTED] [2007-01-24 12:42]:
The data that was in start_info_t should be just simple bindings  
in /

xen since they describe xen, there is no need to create a new node.
many of the props are not needed since they are expressed elsewhere
in the devtree or perhaps differently.
more below.


New rev:

-dropped /xen/start_info_t, hanging new node /xen/store
-fixed up /xen/console/reg to be proper u64 baseu64 size value
-fixed up /xen/console/interrupts to use value passed from xend
-renamed property 'shared_info' to 'shared-info'
-fixed 'shared-info' to be a proper u64 baseu64 size value
-reduced the number of pages reserved at the end of RMA from 4 to 3
 as we no longer need to reserve a page for start_info_t

--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253
[EMAIL PROTECTED]


diffstat output:
 mk_flatdevtree.c |   45 +++--
 mk_flatdevtree.h |6 ++-
 xc_linux_build.c |   83 + 
+-

 3 files changed, 80 insertions(+), 54 deletions(-)

Signed-off-by: Ryan Harper [EMAIL PROTECTED]
---
diff -r ed5ee9dde0bd tools/libxc/powerpc64/mk_flatdevtree.c
--- a/tools/libxc/powerpc64/mk_flatdevtree.c	Sun Jan 21 08:17:46  
2007 -0500
+++ b/tools/libxc/powerpc64/mk_flatdevtree.c	Thu Jan 25 11:57:41  
2007 -0600

@@ -316,13 +316,16 @@ int make_devtree(struct ft_cxt *root,
  unsigned long shadow_mb,
  unsigned long initrd_base,
  unsigned long initrd_len,
- const char *bootargs)
+ const char *bootargs,
+ unsigned long console_evtchn,
+ unsigned long store_evtchn,
+ unsigned long nr_pages)

nr_pages no long needed

Since you later look for the console_mfn and store_mfn later, it  
would be better in the caller and pass then in.
FYI: It is xend that decides where store and console go. shared_info  
is a hypervisor contract so you can continue to calculate it here if  
you want.


Also the devtree should not contain MFNs for frame numbers of any  
kind, simply addresses.



 {
 struct boot_param_header *bph = NULL;
 uint64_t val[2];
 uint32_t val32[2];
 unsigned long remaining;
-unsigned long rma_reserve = 4 * PAGE_SIZE;
+unsigned long rma_reserve = 3 * PAGE_SIZE;


base this on MIN(store, console)


 unsigned long initrd_end = initrd_base + initrd_len;
 int64_t shadow_mb_log;
 uint64_t pft_size;
@@ -332,6 +335,9 @@ int make_devtree(struct ft_cxt *root,
 char *cpuname = NULL;
 int saved_errno;
 int dtb_fd = -1;
+uint64_t shared_info_addr = (rma_bytes - PAGE_SIZE);
+uint64_t store_mfn = (rma_bytes - (2*PAGE_SIZE))  PAGE_SHIFT;
+uint64_t console_mfn = (rma_bytes - (3*PAGE_SIZE))  PAGE_SHIFT;


as above.. not MFNs


 uint32_t cpu0_phandle = get_phandle();
 uint32_t xen_phandle = get_phandle();
 uint32_t rma_phandle = get_phandle();
@@ -419,11 +425,6 @@ int make_devtree(struct ft_cxt *root,
 /* xen = root.addnode('xen') */
 ft_begin_node(root, xen);

-/* start-info is the first page in the RMA reserved area */
-val[0] = cpu_to_be64((u64) (rma_bytes - rma_reserve));
-val[1] = cpu_to_be64((u64) PAGE_SIZE);
-ft_prop(root, start-info, val, sizeof(val));
-
 /*  xen.addprop('version', 'Xen-3.0-unstable\0') */
 ft_prop_str(root, version, Xen-3.0-unstable);


This property should be called compatible



@@ -432,6 +433,11 @@ int make_devtree(struct ft_cxt *root,
 val[1] = cpu_to_be64((u64) 0);
 ft_prop(root, reg, val, sizeof(val));

+/* point to shared_info_t page base addr */
+val[0] = cpu_to_be64((u64) shared_info_addr);
+val[1] = cpu_to_be64((u64) PAGE_SIZE);
+ft_prop(root, shared-info, val, sizeof(val));
+
 /* xen.addprop('domain-name', imghandler.vm.getName() + '\0') */
 /* libxc doesn't know the domain name, that is purely a xend  
thing */

 /* ft_prop_str(root, domain-name, domain_name); */
@@ -442,12 +448,33 @@ int make_devtree(struct ft_cxt *root,
 /* xencons = xen.addnode('console') */
 ft_begin_node(root, console);

-/* xencons.addprop('interrupts', 1, 0) */
-val32[0] = cpu_to_be32((u32) 1);
+/* console_mfn */
+val[0] = cpu_to_be64((u64) console_mfn);

address


+val[1] = cpu_to_be64((u64) PAGE_SIZE);
+ft_prop(root, reg, val, sizeof(val));
+
+/* xencons.addprop('interrupts', console_evtchn, 0) */
+val32[0] = cpu_to_be32((u32) console_evtchn);
 val32[1] = cpu_to_be32((u32) 0);
 ft_prop(root, interrupts, val32, sizeof(val32));

 /* end of 

Re: [XenPPC] [PATCH 1/3] libxc: add start_info_t node to devtree

2007-01-25 Thread Ryan Harper
* Jimi Xenidis [EMAIL PROTECTED] [2007-01-25 13:57]:
 Thanks _a_lot_ Ryan.. this stuff is really tedious.
 Just a few more things.
 
 BTW: what about start_info-flags = SIF_PRIVILEGED or  
 SIF_INITDOMAIN.  You will not need properties now but you will need  
 to make sure there absence is recognized in linux/.../setup.c.

OK, I'll take a look.

 Signed-off-by: Ryan Harper [EMAIL PROTECTED]
 ---
 diff -r ed5ee9dde0bd tools/libxc/powerpc64/mk_flatdevtree.c
 --- a/tools/libxc/powerpc64/mk_flatdevtree.c Sun Jan 21 08:17:46  
 2007 -0500
 +++ b/tools/libxc/powerpc64/mk_flatdevtree.c Thu Jan 25 11:57:41  
 2007 -0600
 @@ -316,13 +316,16 @@ int make_devtree(struct ft_cxt *root,
   unsigned long shadow_mb,
   unsigned long initrd_base,
   unsigned long initrd_len,
 - const char *bootargs)
 + const char *bootargs,
 + unsigned long console_evtchn,
 + unsigned long store_evtchn,
 + unsigned long nr_pages)
 nr_pages no long needed

Right.


 
 Since you later look for the console_mfn and store_mfn later, it  
 would be better in the caller and pass then in.
 FYI: It is xend that decides where store and console go. shared_info  
 is a hypervisor contract so you can continue to calculate it here if  
 you want.
 
 Also the devtree should not contain MFNs for frame numbers of any  
 kind, simply addresses.

I don't follow.  the start_info_t structure explicitly wants an mfn, how
else am I suppose to fill that value out in linux?

 
  {
  struct boot_param_header *bph = NULL;
  uint64_t val[2];
  uint32_t val32[2];
  unsigned long remaining;
 -unsigned long rma_reserve = 4 * PAGE_SIZE;
 +unsigned long rma_reserve = 3 * PAGE_SIZE;
 
 base this on MIN(store, console)

Why? Don't we always have a store and a console page?

 
  unsigned long initrd_end = initrd_base + initrd_len;
  int64_t shadow_mb_log;
  uint64_t pft_size;
 @@ -332,6 +335,9 @@ int make_devtree(struct ft_cxt *root,
  char *cpuname = NULL;
  int saved_errno;
  int dtb_fd = -1;
 +uint64_t shared_info_addr = (rma_bytes - PAGE_SIZE);
 +uint64_t store_mfn = (rma_bytes - (2*PAGE_SIZE))  PAGE_SHIFT;
 +uint64_t console_mfn = (rma_bytes - (3*PAGE_SIZE))  PAGE_SHIFT;
 
 as above.. not MFNs

See my comment above.

  /*  xen.addprop('version', 'Xen-3.0-unstable\0') */
  ft_prop_str(root, version, Xen-3.0-unstable);
 
 This property should be called compatible

OK.

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253
[EMAIL PROTECTED]

___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


Re: [XenPPC] [PATCH 1/3] libxc: add start_info_t node to devtree

2007-01-25 Thread Jimi Xenidis


On Jan 25, 2007, at 3:51 PM, Ryan Harper wrote:


* Jimi Xenidis [EMAIL PROTECTED] [2007-01-25 14:40]:

Since you later look for the console_mfn and store_mfn later, it
would be better in the caller and pass then in.
FYI: It is xend that decides where store and console go.  
shared_info
is a hypervisor contract so you can continue to calculate it  
here if

you want.

Also the devtree should not contain MFNs for frame numbers of any
kind, simply addresses.


I don't follow.  the start_info_t structure explicitly wants an
mfn, how
else am I suppose to fill that value out in linux?


they are MFNs on x86, in PPC they are domain physical addresses.


Maybe I've just got it named funny, I don't know.  And I'm still
confused as to what you want me to put in?


I'm suggesting that we do not use a frame number in the devtree but  
use the address that the frame number represents.
On PPC, MFN is a misnomer, this value is not an MFN but a domain PFN  
(that is the page belongs to the domain), its just a bad name for us  
because the structure is for x86.  Later (on PPC) we convert the  
domain PFN to and phys addr.




The value that I put in start_info-console.domU.mfn is:

((rma_pages  12) - (2*PAGE_SIZE))  PAGE_SHIFT

where rma_pages = (1  26)  PAGE_SHIFT

the resulting value is 0x3ffe.  Is this value correct and I just  
have an

incorrect name for it (mfn)?

yes


Am I getting lucky? I've tested the
patches and dom0 and domU boot.


Not luck, we just preserved a crappy name for a crappy struct that in  
PPC land will dissapear.











{
   struct boot_param_header *bph = NULL;
   uint64_t val[2];
   uint32_t val32[2];
   unsigned long remaining;
-unsigned long rma_reserve = 4 * PAGE_SIZE;
+unsigned long rma_reserve = 3 * PAGE_SIZE;


base this on MIN(store, console)


Why? Don't we always have a store and a console page?


yes we do, but they could be in any order, all you are trying to do
is make sure the pages are in the reserve map.  Another possibility
is you could not assume contiguity and create a reservation entry for
each of the three pages.


I don't understand what is wrong with the above.  We are reserving the
last X pages of the RMA.  1 for shared_info, 1 for console, one for
store.  I futher don't understand what the value MIN(store,console)
gives me.  Sorry for being dense here.


I know it is confusing, thats why it is so important we get it right.
First, I suggested that you pass in these addresses.
Once they are passed in, this function has no idea which one has the  
lowest address.
There is nothing from stoping from changing the order of these 2  
pages, as long as both sides agree.

-JX



___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


Re: [XenPPC] [PATCH 1/3] libxc: add start_info_t node to devtree

2007-01-24 Thread Segher Boessenkool

This patch creates a new node, /xen/start_info_t in the flat devtree.


Could you name it start-info or something similar to
be more in line with normal device tree naming conventions?


Segher


___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


Re: [XenPPC] [PATCH 1/3] libxc: add start_info_t node to devtree

2007-01-24 Thread Jimi Xenidis
The data that was in start_info_t should be just simple bindings in / 
xen since they describe xen, there is no need to create a new node.
many of the props are not needed since they are expressed elsewhere  
in the devtree or perhaps differently.

more below.

On Jan 24, 2007, at 12:41 PM, Ryan Harper wrote:


This patch creates a new node, /xen/start_info_t in the flat devtree.
It adds a property for each field of the start_info_t structure that
xc_linux_build used to fill-out.  I've also removed the helper  
functions

which created/filled-out the start_info_t structure.

This patch depends on Patch2 which modifies linux:xen_init_early().

--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253
[EMAIL PROTECTED]


diffstat output:
 mk_flatdevtree.c |   36 +-
 mk_flatdevtree.h |6 ++--
 xc_linux_build.c |   76 ++ 
+

 3 files changed, 67 insertions(+), 51 deletions(-)

Signed-off-by: Ryan Harper [EMAIL PROTECTED]
---
diff -r ed5ee9dde0bd tools/libxc/powerpc64/mk_flatdevtree.c
--- a/tools/libxc/powerpc64/mk_flatdevtree.c	Sun Jan 21 08:17:46  
2007 -0500
+++ b/tools/libxc/powerpc64/mk_flatdevtree.c	Tue Jan 23 09:50:43  
2007 -0600

@@ -316,7 +316,10 @@ int make_devtree(struct ft_cxt *root,
  unsigned long shadow_mb,
  unsigned long initrd_base,
  unsigned long initrd_len,
- const char *bootargs)
+ const char *bootargs,
+ unsigned long console_evtchn,
+ unsigned long store_evtchn,
+ unsigned long nr_pages)
 {
 struct boot_param_header *bph = NULL;
 uint64_t val[2];
@@ -419,11 +422,6 @@ int make_devtree(struct ft_cxt *root,
 /* xen = root.addnode('xen') */
 ft_begin_node(root, xen);

-/* start-info is the first page in the RMA reserved area */
-val[0] = cpu_to_be64((u64) (rma_bytes - rma_reserve));
-val[1] = cpu_to_be64((u64) PAGE_SIZE);
-ft_prop(root, start-info, val, sizeof(val));
-
 /*  xen.addprop('version', 'Xen-3.0-unstable\0') */
 ft_prop_str(root, version, Xen-3.0-unstable);

@@ -448,6 +446,32 @@ int make_devtree(struct ft_cxt *root,
 ft_prop(root, interrupts, val32, sizeof(val32));

 /* end of console */
+ft_end_node(root);
+
+/* mark up start_info fields here */
+ft_begin_node(root, start_info_t);
+
+ft_prop_str(root, magic, xen-3.0-powerpc64HV);
No need for magic since we are not trying to verify that the memory  
location is good.
If we _do_ need this as some kind of compatibility statement then I'd  
say the property name is compatible



+
+val[0] = cpu_to_be64((u64) nr_pages);
+ft_prop(root, nr_pages, (val[0]), sizeof(val[0]));
/memory node has this covered and linux knows about it, AFAICT we  
don't need it at all
If we do need it then you'll have to walk the LMB and count the pages  
like it is done in:
  arch/powerpc/mm/hash_utils_64.c htab_initialize 429 void __init  
htab_initialize(void )



+
+val[0] = cpu_to_be64((u64) (rma_bytes - PAGE_SIZE));
+ft_prop(root, shared_info, (val[0]), sizeof(val[0]));


memory areas generally contain a size so that should be included.
I believe that _ is reserved for the standard root bindings, use  
use -.

So the name should be shared-info


+
+val[0] = cpu_to_be64((u64) ((rma_bytes  PAGE_SHIFT) - 2));
+ft_prop(root, store_mfn, (val[0]), sizeof(val[0]));
To there are to items that describe Xen Store, its location and its  
interrupt.

I would think the correct binding here is:
 /xen/store/reg: store_mfn  PAGE_SHIFT
 /xen/store/interrupts: store_evtchn 0
the latter is 2 ints.

Same goes for console.


+
+ft_prop_int(root, store_evtchn, store_evtchn);
+
+/* start_info-console.domU.mfn */
+val[0] = cpu_to_be64((u64) ((rma_bytes  PAGE_SHIFT) - 3));
+ft_prop(root, console_domU_mfn, (val[0]), sizeof(val[0]));
+
+/* start_info-console.domU.evtchn */
+ft_prop_int(root, console_domU_evtchn, console_evtchn);
+
+/* end of start_info_t */
 ft_end_node(root);

 /* end of xen node */
diff -r ed5ee9dde0bd tools/libxc/powerpc64/mk_flatdevtree.h
--- a/tools/libxc/powerpc64/mk_flatdevtree.h	Sun Jan 21 08:17:46  
2007 -0500
+++ b/tools/libxc/powerpc64/mk_flatdevtree.h	Mon Jan 22 15:38:46  
2007 -0600

@@ -32,8 +32,10 @@ extern int make_devtree(struct ft_cxt *r
 unsigned long shadow_mb,
 unsigned long initrd_base,
 unsigned long initrd_len,
-const char *bootargs);
-
+const char *bootargs,
+unsigned long console_evtchn,
+unsigned long store_evtchn,
+unsigned long nr_pages);
 #define MAX_PATH 200
 #define BUFSIZE 1024
 #define BPH_SIZE 16*1024
diff -r ed5ee9dde0bd tools/libxc/powerpc64/xc_linux_build.c
---