Re: [XenPPC] Re: patch flow

2007-01-04 Thread Jimi Xenidis


On Jan 3, 2007, at 12:01 PM, Hollis Blanchard wrote:


On Sun, 2006-12-17 at 18:00 +, Xen patchbot-xenppc-unstable wrote:

# HG changeset patch
# User Jimi Xenidis [EMAIL PROTECTED]
# Node ID b04e24db308f2215c6bafaf358d1c10da79f244f
# Parent  965d3e42dddaf5971001f7d172d192f925537644
[XEN][POWERPC] get cpu_*_maps correct so physinfo and affinity is  
accurate


Signed-off-by: Jimi Xenidis [EMAIL PROTECTED]


Please commit all uncontroversial patches first to xenppc-merge, then
pull xenppc-merge into xenppc-unstable.


Unfortunately, the above patch was controversial and the rework in on  
my queue, but thats not the point here.



It's OK if we change our minds
later and revert changesets in -merge.

To recap, xenppc-merge contains patches going upstream; it is pulled
directly into xen-unstable. xenppc-unstable is xen-unstable plus any
temporary hacks we need to make progress. (When you define
xenppc-unstable this way, it obviously should be a child tree, not a
parent tree.)


I think this model is premature since the only purpose of -merge is  
to be a consistent pull tree.
Unfortunately, -merge is incomplete, may not run or even build, we  
cannot base work on that.




This will ensure that we never have the same difficulty staying in  
sync

with xen-unstable that we've had in the past.


I'm not sure how..
Are you asking all developers to send diffs based on the -merge tree?
Are you asking the maintainers to go thru the following process?
 1) apply diffs to childof-unstable, test
 2) apply to childof-merge and then push -merg blind
 3) pull -merge into -unstable
 4) reset childof-unstable

I think we need to get -unstable to a point where patches are  
acceptable make the tip of -merge be that snapshot and continue  
working in -unstable.


So lets bear down and get whatever change set that is in -unstable  
into -merge.


We can consider what you propose when -merge works completely

-JX

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


Re: [XenPPC] [patch] multiboot2 support

2007-01-04 Thread Jimi Xenidis

On Jan 3, 2007, at 12:32 PM, Hollis Blanchard wrote:


Xen on x86 uses GRUB's multiboot capabilities to load an arbitrary
number of images, including Xen, dom0 kernel, dom0 initrd, and ACM
policy. On PowerPC, we've had to build Xen, the dom0 kernel and the  
dom0

initrd all into the same file to get them loaded.


We also boot from yaboot which allows us to separate dom0 from xen,  
as does newer PIBS firmware via r3  r4 and trivial to teach SLOF and  
other OF how as well. Please do not assume that we are fully linked  
only and do not remove that logic.  Using your new boot loader case  
should be additive.




Since we currently
boot directly from Open Firmware, early PPC Xen code then fakes up a
multiboot info structure which later PPC Xen code extracts data from.

GRUB2, which supports PowerPC, is defining a new multiboot format
because the original was far too x86-specific. That loader is not yet
committed but is fairly complete, so to test it out I'm adapting  
PPC Xen

to it.

This is the first step: I've replaced our early multiboot code to fake
up a multiboot2 structure instead. I haven't yet tried to boot this  
from

GRUB2, but we will likely want to continue supporting directly booting
from firmware so this code needs to work.

Of interest is that I've changed the memory map to simplify some of  
the
early memory allocation code (which unlike x86 tried to handle  
unordered

discontiguous allocations). With this patch, our memory map now looks
like this:

exception handlers (0x0 to 0x4000)
RTAS
Open Firmware device tree


RTAS and OFD are subject to avail properties in from OF and whether  
or not they can be claimed.  I don't think you should write code that  
assumes they are here.



boot allocator bitmap (common Xen code)
Xen heap
...
_start (0x40; 64MB)

0x40 is 4MiB
I had a paragraph on how 64MiB was bad and then realized it is still  
4MiB :)

[Xen text/data]
_end
modules
[dom0, dom0 initrd, etc]
Xen heap
...
[fixed size]
domain heap
...
[consumes all remaining memory]

Comments and testing are welcome, or I'll probably check this in in a
day or so.

Signed-off-by: Hollis Blanchard [EMAIL PROTECTED]

diff -r dbc74db14a4b xen/arch/powerpc/boot_of.c
--- a/xen/arch/powerpc/boot_of.cTue Dec 12 14:35:07 2006 -0600
+++ b/xen/arch/powerpc/boot_of.cWed Jan 03 10:50:07 2007 -0600
@@ -22,7 +22,7 @@
 #include xen/config.h
 #include xen/init.h
 #include xen/lib.h
-#include xen/multiboot.h
+#include xen/multiboot2.h
 #include xen/version.h
 #include xen/spinlock.h
 #include xen/serial.h
@@ -30,6 +30,7 @@
 #include xen/sched.h
 #include asm/page.h
 #include asm/io.h
+#include asm/boot.h
 #include exceptions.h
 #include of-devtree.h
 #include oftree.h
@@ -77,6 +78,28 @@ static int bof_chosen;
 static int bof_chosen;

 static struct of_service s;
+
+static unsigned char xentags[512];
+static int xentags_pos;
+
+static int of_printf(const char *fmt, ...)
+__attribute__ ((format (printf, 1, 2)));
+
+static void *alloc_tag(int key, int len)
+{
+struct mb2_tag_header *tag;
+


Does len include sizeof(*tag)? it looks like it does, why not make it  
implicit?
Since the the largest member of *tag is a uint32_t then it must be 4  
byte aligned, please make sure your allocations are rounded up to 4  
bytes, unless you _know_ that len is, but I'd do it anyway.



+if (xentags_pos + len  sizeof(xentags))
+of_panic(Couldn't allocate multiboot tag.);
+
+tag = (struct mb2_tag_header *)(xentags + xentags_pos);
+tag-key = key;
+tag-len = len;
+
+xentags_pos += len;
+
+return tag;
+}

 static int __init of_call(
 const char *service, u32 nargs, u32 nrets, s32 rets[], ...)
@@ -160,8 +183,6 @@ static int __init of_write(int ih, const
 return sum;
 }

-static int of_printf(const char *fmt, ...)
-__attribute__ ((format (printf, 1, 2)));
 static int __init of_printf(const char *fmt, ...)
 {
 static char buf[1024];
@@ -609,8 +630,11 @@ static ulong boot_of_mem_init(void)
 return 0;
 }

-static void boot_of_bootargs(multiboot_info_t *mbi)
-{
+static char *boot_of_bootargs(char **dom0_cmdline)
+{
+static const char *sepr[] = { -- ,  || };
+char *p;
+int sepr_index;
 int rc;

 if (builtin_cmdline[0] == '\0') {
@@ -620,10 +644,22 @@ static void boot_of_bootargs(multiboot_i
 of_panic(bootargs[] not big enough for /chosen/ 
bootargs\n);

 }

-mbi-flags |= MBI_CMDLINE;
-mbi-cmdline = (ulong)builtin_cmdline;
-
 of_printf(bootargs = %s\n, builtin_cmdline);
+
+/* look for delimiter: -- or || */
+for (sepr_index = 0; sepr_index  ARRAY_SIZE(sepr); sepr_index+ 
+){

+p = strstr((char *)builtin_cmdline, sepr[sepr_index]);


Is the cast necessary?


+if (p != NULL) {
+/* Xen proper 

Re: [XenPPC] [patch] multiboot2 support

2007-01-04 Thread Hollis Blanchard
Hi Jimi, thanks for the comments.

I'm really not interested in reworking all this stuff, which is why I
took shortcuts like relocating the modules rather than spending effort
on your preferred solution. Unfortunately, all this code was intimately
linked with the multiboot structure, so I had to change it.

On Thu, 2007-01-04 at 12:27 -0500, Jimi Xenidis wrote:
 On Jan 3, 2007, at 12:32 PM, Hollis Blanchard wrote:
 
  Xen on x86 uses GRUB's multiboot capabilities to load an arbitrary
  number of images, including Xen, dom0 kernel, dom0 initrd, and ACM
  policy. On PowerPC, we've had to build Xen, the dom0 kernel and the
  dom0
  initrd all into the same file to get them loaded.
 
 We also boot from yaboot which allows us to separate dom0 from xen,
 as does newer PIBS firmware via r3  r4 and trivial to teach SLOF and
 other OF how as well. Please do not assume that we are fully linked
 only and do not remove that logic.  Using your new boot loader case
 should be additive.

I believe you yourself told me those other methods were unimportant and
could be removed. :)

  With this patch, our memory map now looks
  like this:
 
  exception handlers (0x0 to 0x4000)
  RTAS
  Open Firmware device tree
 
 RTAS and OFD are subject to avail properties in from OF and whether
 or not they can be claimed.  I don't think you should write code that
 assumes they are here.

The code does not. Anyways, regardless of the exact placement (which is
subject to available), the order remains the same.

  +static void *alloc_tag(int key, int len)
  +{
  +struct mb2_tag_header *tag;
  +
 
 Does len include sizeof(*tag)? it looks like it does, why not make it
 implicit?
 Since the the largest member of *tag is a uint32_t then it must be 4
 byte aligned, please make sure your allocations are rounded up to 4
 bytes, unless you _know_ that len is, but I'd do it anyway.

It's easy to go back and forth on this issue (I have already). In the
end this is best because you can alloc_tag(FOO, sizeof(foo));

  +static char *boot_of_bootargs(char **dom0_cmdline)
  +{
  +static const char *sepr[] = { -- ,  || };
  +char *p;
  +int sepr_index;
   int rc;
 
   if (builtin_cmdline[0] == '\0') {
  @@ -620,10 +644,22 @@ static void boot_of_bootargs(multiboot_i
   of_panic(bootargs[] not big enough for /chosen/
  bootargs\n);
   }
 
  -mbi-flags |= MBI_CMDLINE;
  -mbi-cmdline = (ulong)builtin_cmdline;
  -
   of_printf(bootargs = %s\n, builtin_cmdline);
  +
  +/* look for delimiter: -- or || */
  +for (sepr_index = 0; sepr_index  ARRAY_SIZE(sepr); sepr_index+
  +){
  +p = strstr((char *)builtin_cmdline, sepr[sepr_index]);
 
 Is the cast necessary?

Doesn't look like it.

  +if (p != NULL) {
  +/* Xen proper should never know about the dom0 args.  */
  +*(char *)p = '\0';
 
 Why casting here?

This code was moved from another location where p was const. I'll fix.

  +static int __init boot_of_rtas(void)
   {
   int rtas_node;
   int rtas_instance;
  @@ -1026,12 +1060,10 @@ static int __init boot_of_rtas(module_t
   rtas_end = mem + size;
   rtas_msr = of_msr;
 
  -mod-mod_start = rtas_base;
  -mod-mod_end = rtas_end;
   return 1;
 
 hmm, aren't you going to create a tag so you know where RTAS has been
 placed and how big it is?

No. RTAS is not a module GRUB passes to kernels.

  +/* Create a structure as if we were loaded by a multiboot
  bootloader. */
  +struct mb2_tag_header __init *boot_of_multiboot(void)
  +{
  +struct mb2_tag_start *start;
  +struct mb2_tag_name *name;
  +struct mb2_tag_module *xenmod;
  +struct mb2_tag_end *end;
  +char *xen_cmdline;
  +char *dom0_cmdline = NULL;
  +static char *namestr = xen;
  +
  +/* create start tag. start-size is filled in later. */
  +start = alloc_tag(MB2_TAG_START, sizeof(*start));
  +
  +/* create name tag. */
  +name = alloc_tag(MB2_TAG_NAME, sizeof(*name) + strlen(namestr));
 
 Are you intentionally skipping the '\0' that in the alloc

Nope, good catch!

  @@ -141,43 +143,9 @@ static void ofd_walk_mem(void *m, walk_m
   }
   }
 
  -static void setup_xenheap(module_t *mod, int mcount)
  -{
  -int i;
  -ulong freemem;
  -
  -freemem = ALIGN_UP((ulong)_end, PAGE_SIZE);
  -
  -for (i = 0; i  mcount; i++) {
  -u32 s;
  -
  -if (mod[i].mod_end == mod[i].mod_start)
  -continue;
  -
  -s = ALIGN_DOWN(mod[i].mod_start, PAGE_SIZE);
  -
  -if (mod[i].mod_start  (ulong)_start 
  -mod[i].mod_start  (ulong)_end) {
  -/* mod was linked in */
  -continue;
  -}
  -
  -if (s  freemem)
  -panic(module addresses must assend\n);
  -
  -free_xenheap(freemem, s);
  -freemem = ALIGN_UP(mod[i].mod_end, PAGE_SIZE);
  -
  -}
  -
  -/* the rest of the xenheap, starting at the end of 

Re: [XenPPC] [patch] multiboot2 support

2007-01-04 Thread Jimi Xenidis


On Jan 4, 2007, at 3:02 PM, Hollis Blanchard wrote:


Hi Jimi, thanks for the comments.

I'm really not interested in reworking all this stuff, which is why I
took shortcuts like relocating the modules rather than spending effort
on your preferred solution.


Ok, then it can wait till grub is ready and stable?
Otherwise the code does not add anything other than a new data  
structure.

I thought you intended on pushing this soon?


Unfortunately, all this code was intimately
linked with the multiboot structure, so I had to change it.

On Thu, 2007-01-04 at 12:27 -0500, Jimi Xenidis wrote:

On Jan 3, 2007, at 12:32 PM, Hollis Blanchard wrote:


Xen on x86 uses GRUB's multiboot capabilities to load an arbitrary
number of images, including Xen, dom0 kernel, dom0 initrd, and ACM
policy. On PowerPC, we've had to build Xen, the dom0 kernel and the
dom0
initrd all into the same file to get them loaded.


We also boot from yaboot which allows us to separate dom0 from xen,
as does newer PIBS firmware via r3  r4 and trivial to teach SLOF and
other OF how as well. Please do not assume that we are fully linked
only and do not remove that logic.  Using your new boot loader case
should be additive.


I believe you yourself told me those other methods were unimportant  
and

could be removed. :)


IIRC I agreed that defining location in the cmdline could certainly  
go, but the r3,r4 pairing should stay.





With this patch, our memory map now looks
like this:

exception handlers (0x0 to 0x4000)
RTAS
Open Firmware device tree


RTAS and OFD are subject to avail properties in from OF and whether
or not they can be claimed.  I don't think you should write code that
assumes they are here.


The code does not.
actually your patch in setup.c assumes the memory after _end is free  
and clear, this is not the case if the OFD was placed after the image  
which is certainly possible, see below.



Anyways, regardless of the exact placement (which is
subject to available), the order remains the same.


well, not really, RTAS could fit but OFD could easily be after the  
image.  It does now because we I made it smaller becaus SLOF/PIBS  
devtrees are small. Unfortunately apple devtrees are HUGE and the  
original size that we allocated (to accommodate them) cause it to  
exist after the xen image.





+static void *alloc_tag(int key, int len)
+{
+struct mb2_tag_header *tag;
+


Does len include sizeof(*tag)? it looks like it does, why not make it
implicit?
Since the the largest member of *tag is a uint32_t then it must be 4
byte aligned, please make sure your allocations are rounded up to 4
bytes, unless you _know_ that len is, but I'd do it anyway.


It's easy to go back and forth on this issue (I have already). In the
end this is best because you can alloc_tag(FOO, sizeof(foo));


I'm sure you have, but please don't forget to round up your allocation.




+static char *boot_of_bootargs(char **dom0_cmdline)
+{
+static const char *sepr[] = { -- ,  || };
+char *p;
+int sepr_index;
 int rc;

 if (builtin_cmdline[0] == '\0') {
@@ -620,10 +644,22 @@ static void boot_of_bootargs(multiboot_i
 of_panic(bootargs[] not big enough for /chosen/
bootargs\n);
 }

-mbi-flags |= MBI_CMDLINE;
-mbi-cmdline = (ulong)builtin_cmdline;
-
 of_printf(bootargs = %s\n, builtin_cmdline);
+
+/* look for delimiter: -- or || */
+for (sepr_index = 0; sepr_index  ARRAY_SIZE(sepr); sepr_index+
+){
+p = strstr((char *)builtin_cmdline, sepr[sepr_index]);


Is the cast necessary?


Doesn't look like it.


+if (p != NULL) {
+/* Xen proper should never know about the dom0  
args.  */

+*(char *)p = '\0';


Why casting here?


This code was moved from another location where p was const. I'll fix.


+static int __init boot_of_rtas(void)
 {
 int rtas_node;
 int rtas_instance;
@@ -1026,12 +1060,10 @@ static int __init boot_of_rtas(module_t
 rtas_end = mem + size;
 rtas_msr = of_msr;

-mod-mod_start = rtas_base;
-mod-mod_end = rtas_end;
 return 1;


hmm, aren't you going to create a tag so you know where RTAS has been
placed and how big it is?


No. RTAS is not a module GRUB passes to kernels.


right, no tag is necessary since you have rtas_{base,end}.


+/* Create a structure as if we were loaded by a multiboot
bootloader. */
+struct mb2_tag_header __init *boot_of_multiboot(void)
+{
+struct mb2_tag_start *start;
+struct mb2_tag_name *name;
+struct mb2_tag_module *xenmod;
+struct mb2_tag_end *end;
+char *xen_cmdline;
+char *dom0_cmdline = NULL;
+static char *namestr = xen;
+
+/* create start tag. start-size is filled in later. */
+start = alloc_tag(MB2_TAG_START, sizeof(*start));
+
+/* create name tag. */
+name = alloc_tag(MB2_TAG_NAME, sizeof(*name) + strlen 
(namestr));


Are you intentionally skipping the '\0' that in the alloc


Nope, good catch!


@@ -141,43 +143,9 @@ 

RE: [XenPPC] systemsim-gpul problems

2007-01-04 Thread Yoder Stuart-B08248
Whad'ya know...if you wait long enough it works!
 
Thanks Mark.




From: Mark F Mergen [mailto:[EMAIL PROTECTED] 
Sent: Thursday, January 04, 2007 4:30 PM
To: Yoder Stuart-B08248; xen-ppc-devel@lists.xensource.com
Subject: Fw: [XenPPC] systemsim-gpul problems



Sorry, I should have read your note more carefully.  Yes, my
latest builds do boot all the way into Linux, but the timing is
drastically altered from the past.  There are places in the boot
sequence (and I think what you mention: after protocol family 17 is one
of them) where you must wait a VERY long time for something to happen.
I can't tell you exactly how long, but it took me a while and accidental
inattention to what was happening before I discovered this.  I have not
had time to work on why this is so.  Also, if you do get all the way to
a Linux command prompt, you may conclude it is not responding to
commands.  Just type a command that you think should work (such as pwd),
hit Enter, and then WAIT A LONG TIME.  If your experience turns out to
be like mine, eventually the characters you typed will be echoed and the
command will be executed with proper output, however slowly.  Obviously,
more debugging is needed.  Maybe it's something simple, like some
simulator option for how real time is reflected to the simulated
machine.  Maybe you will be rewarded by a little (or a LOT of) patience
in your next try. 

Mark Mergen 

- Forwarded by Mark F Mergen/Watson/IBM on 01/04/2007 05:15
PM - 

Mark F Mergen/Watson/IBM 

01/04/2007 05:12 PM 

To
[EMAIL PROTECTED] 
cc
Yoder Stuart-B08248 [EMAIL PROTECTED],
xen-ppc-devel@lists.xensource.com 
Subject
Re: [XenPPC] systemsim-gpul problemsLink
Notes://D01MLC04/852564F1000C2D56/C57E6BC8EC0391E7852571CA007FA25B/9BD9
00E03249E9608525725900705623 





I pull only from xenppc-unstable.hg and linux-ppc-2.6.hg.  I'm
up to date with all latest that was pushed to them, and this combo runs
on systemsim-gpul.  There was a bug(s?) that caused symptoms like you
mention, but they were fixed by Amos Waterland about 2nd week in
December, and pushed to aformentioned repositories by the maintainers.
I can't quote or vouch for specific changesets. 

Mark Mergen 




[EMAIL PROTECTED] 
Sent by: [EMAIL PROTECTED] 

01/04/2007 03:11 PM 
Please respond to
[EMAIL PROTECTED]


To
Yoder Stuart-B08248 [EMAIL PROTECTED] 
cc
xen-ppc-devel@lists.xensource.com 
Subject
Re: [XenPPC] systemsim-gpul problems






On Thu, 2007-01-04 at 10:32 -0700, Yoder Stuart-B08248 wrote:
 I've been trying to get systemsim-gpul working with the 
 latest Xen-PPC.
 
 The last changeset that worked was 7ad4645e7a54 (11/22/06).
 
 Changeset ce8c1e26b2ae (Early boot memory avoidance
improvemnts)
 broke the simulator with the 'Could not allocate RTAS tree' /
HANG
 error.
 
 With changeset 878ce1f78ad3 (Fix systemsim-gpul failure to
boot)
 and later changesets the RTAS allocation / HANG problem is
fixed
 but the simulator still won't boot into Linux.
 
 If I compare logs of the working and non-working Xen/Linux
runs
 in the simulator, with the current Xen Linux hangs near the
end
 of its boot.   Last messages from Linux are:
 
 i2c /dev entries driver IPv4 over IPv4 tunneling driver
 TCP bic registered
 NET: Registered protocol family 1
 NET: Registered protocol family 17
 
 ...then nothing.
 
 Shortly after this point in the boot is where the RAMDISK is
 decompressed and accessed.  I'm wondering if the boot related
memory
 improvements have affected a RAMDISK built into Linux and Xen.

Have you tried attaching GDB to systemsim to figure out what's
going on?

 Has anyone else had recent changesets working on the
simulator?

I haven't tried simulator in quite some time...

-- 
Hollis Blanchard
IBM Linux Technology Center



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



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

RE: [XenPPC] systemsim-gpul problems

2007-01-04 Thread Hollis Blanchard
On Thu, 2007-01-04 at 16:48 -0700, Yoder Stuart-B08248 wrote:
 Whad'ya know...if you wait long enough it works!

So simulator performance is acceptable until mid-way through dom0 boot?
It would be good to figure out the source of the slowdown.

-- 
Hollis Blanchard
IBM Linux Technology Center


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


RE: [XenPPC] systemsim-gpul problems

2007-01-04 Thread Michal Ostrowski
Could be that midway through a Linux boot all code is sequential, but at
a later point Linux tries to initialize its timing facilities.  This
involves linux spinning on the timebase register which forces systemsim
to simulate many, many useless cycles.   Beyond that Linux
initialization sees a lot of cache initialization which requires memory
zeroing, which also is not simulator friendly.

I see this sort of behavior simply by running linux on the simulator wit
or without a hypervisor.

On Thu, 2007-01-04 at 18:19 -0600, Hollis Blanchard wrote:
 On Thu, 2007-01-04 at 16:48 -0700, Yoder Stuart-B08248 wrote:
  Whad'ya know...if you wait long enough it works!
 
 So simulator performance is acceptable until mid-way through dom0 boot?
 It would be good to figure out the source of the slowdown.
 
-- 
Michal Ostrowski [EMAIL PROTECTED]


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


RE: [XenPPC] systemsim-gpul problems: reserved memory areas

2007-01-04 Thread Hollis Blanchard
On Thu, 2007-01-04 at 14:15 -0700, Yoder Stuart-B08248 wrote:

 I have turned on debug prints in arch/powerpc/boot_of.c.
 One thing I'm puzzling over is why boot_of_alloc_init()
 is marking overlapping regions of memory.  Does that 
 make sense??

It doesn't exactly make sense, but in this case it's not a real
problem...

 [snip]
 boot_of_alloc_init: marking 0x0 - 0x0^M
 boot_of_alloc_init: marking 0x0 - 0x400^M

These two come from your Open Firmware device tree. They don't make
sense IMHO. systemsim may have a bug in its device tree.

We could also have a bug in the way we parse the available property,
but I believe our code has been tested on PowerMac and SLOF, and it
looks correct to me.

 boot_of_alloc_init: marking 0x40 - 0x88b000^M

This is our code reserving Xen's text, _start to _end.

 boot_of_alloc_init: marking 0x0 - 0x3000^M

This is our code reserving the exception handlers.

-- 
Hollis Blanchard
IBM Linux Technology Center


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


[XenPPC] [PATCH]fix xencomm_copy_{from, to}_guest.

2007-01-04 Thread Isaku Yamahata

fix xencomm_copy_{from, to}_guest.
It should not call paddr_to_maddr() with invalid address.

Originally this issue was found in xen/ia64 xencomm code and
in fact I didn't test this patch because currently ia64 xencomm forked
from common code. They should be consolidated somehow.

-- 
yamahata
# HG changeset patch
# User [EMAIL PROTECTED]
# Date 1167966417 -32400
# Node ID 25cdcc5f21f8147371aed5fb8f56d93479df0ca8
# Parent  338ceb7b1f0993bf9735c0c1c5d21e39c381cf2f
fix xencomm_copy_{from, to}_guest.
It should not call paddr_to_maddr() with invalid address.
PATCHNAME: fix_xencomm_copy_tofrom_guest

Signed-off-by: Isaku Yamahata [EMAIL PROTECTED]

diff -r 338ceb7b1f09 -r 25cdcc5f21f8 xen/common/xencomm.c
--- a/xen/common/xencomm.c  Thu Jan 04 10:58:01 2007 +
+++ b/xen/common/xencomm.c  Fri Jan 05 12:06:57 2007 +0900
@@ -119,7 +119,7 @@ xencomm_copy_from_guest(void *to, const 
 chunksz -= chunk_skip;
 skip -= chunk_skip;
 
-if (skip == 0) {
+if (skip == 0  chunksz  0) {
 unsigned long src_maddr;
 unsigned long dest = (unsigned long)to + to_pos;
 unsigned int bytes = min(chunksz, n - to_pos);
@@ -225,7 +225,7 @@ xencomm_copy_to_guest(void *to, const vo
 chunksz -= chunk_skip;
 skip -= chunk_skip;
 
-if (skip == 0) {
+if (skip == 0  chunksz  0) {
 unsigned long dest_maddr;
 unsigned long source = (unsigned long)from + from_pos;
 unsigned int bytes = min(chunksz, n - from_pos);
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel