Re: [PATCH 1/6] lguest: host code tidyups

2007-05-16 Thread hch
On Wed, May 16, 2007 at 09:32:16AM +1000, Rusty Russell wrote:
> On Tue, 2007-05-15 at 21:47 +1000, Stephen Rothwell wrote:
> > On Tue, 15 May 2007 21:42:35 +1000 Stephen Rothwell <[EMAIL PROTECTED]> 
> > wrote:
> > >
> > > On Tue, 15 May 2007 11:17:07 +1000 Rusty Russell <[EMAIL PROTECTED]> 
> > > wrote:
> > > > -   on_each_cpu(adjust_pge, 0, 0, 1);
> > > > +   on_each_cpu(adjust_pge, (void *)0, 0, 1);
> > >
> > > Sorry?  What ever happened to a simple NULL?
> > 
> > Oh, I guess that is an explicit (numeric) 0 (of some type) caste to
> > "void *" because of the prototype - rather than not passing anything?
> 
> Indeed.  We really want to pass a bool, but on_each_cpu uses a void *.
> Hence the clearest solution seemed "(void *)0" and "(void *)1" in the
> callers.

Cleanest way to do that is to pass the value by reference.

const int some_useful_name = 0;

 on_each_cpu(adjust_pge, &some_useful_name, 0, 1);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/6] lguest: host code tidyups

2007-05-15 Thread Rusty Russell
On Tue, 2007-05-15 at 21:47 +1000, Stephen Rothwell wrote:
> On Tue, 15 May 2007 21:42:35 +1000 Stephen Rothwell <[EMAIL PROTECTED]> wrote:
> >
> > On Tue, 15 May 2007 11:17:07 +1000 Rusty Russell <[EMAIL PROTECTED]> wrote:
> > > - on_each_cpu(adjust_pge, 0, 0, 1);
> > > + on_each_cpu(adjust_pge, (void *)0, 0, 1);
> >
> > Sorry?  What ever happened to a simple NULL?
> 
> Oh, I guess that is an explicit (numeric) 0 (of some type) caste to
> "void *" because of the prototype - rather than not passing anything?

Indeed.  We really want to pass a bool, but on_each_cpu uses a void *.
Hence the clearest solution seemed "(void *)0" and "(void *)1" in the
callers.

Thanks,
Rusty.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/6] lguest: host code tidyups

2007-05-15 Thread Stephen Rothwell
On Tue, 15 May 2007 21:42:35 +1000 Stephen Rothwell <[EMAIL PROTECTED]> wrote:
>
> On Tue, 15 May 2007 11:17:07 +1000 Rusty Russell <[EMAIL PROTECTED]> wrote:
> >
> > @@ -420,7 +421,7 @@ static int __init init(void)
> > lock_cpu_hotplug();
> > if (cpu_has_pge) { /* We have a broader idea of "global". */
> > cpu_had_pge = 1;
> > -   on_each_cpu(adjust_pge, 0, 0, 1);
> > +   on_each_cpu(adjust_pge, (void *)0, 0, 1);
>
> Sorry?  What ever happened to a simple NULL?

Oh, I guess that is an explicit (numeric) 0 (of some type) caste to
"void *" because of the prototype - rather than not passing anything?

--
Cheers,
Stephen Rothwell[EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/


pgplwidV8tBM1.pgp
Description: PGP signature


Re: [PATCH 1/6] lguest: host code tidyups

2007-05-15 Thread Stephen Rothwell
On Tue, 15 May 2007 11:17:07 +1000 Rusty Russell <[EMAIL PROTECTED]> wrote:
>
> @@ -420,7 +421,7 @@ static int __init init(void)
>   lock_cpu_hotplug();
>   if (cpu_has_pge) { /* We have a broader idea of "global". */
>   cpu_had_pge = 1;
> - on_each_cpu(adjust_pge, 0, 0, 1);
> + on_each_cpu(adjust_pge, (void *)0, 0, 1);

Sorry?  What ever happened to a simple NULL?

--
Cheers,
Stephen Rothwell[EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/


pgpJoE9QL9LOw.pgp
Description: PGP signature


[PATCH 1/6] lguest: host code tidyups

2007-05-14 Thread Rusty Russell
Christoph Hellwig said runs sparse:
1) page_tables.c unnecessary initialization
2) Change prototype of run_lguest and do cast in caller instead (when we add
   __user to cast, it runs over another line).
Al Viro pointed out the ugly cast in push_lguest_stack():
3) Stick with unsigned long for arg, removes 4 casts in total.

Most importantly, I now realize that Christoph's incorrect ranting
about lgread_u32 et al was in fact a subtle ploy to make me diagnose
the real issue: sparse 0.3 complains about casting a __user pointer
to/from u32, but not an "unsigned long".  They are (currently)
equivalent for lguest, but this is a much better solution than __force.

Kudos, Christoph, for such masterful manipulation!

Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
 drivers/lguest/core.c |   37 -
 drivers/lguest/hypercalls.c   |9 +++-
 drivers/lguest/interrupts_and_traps.c |   15 +++--
 drivers/lguest/io.c   |2 -
 drivers/lguest/lg.h   |   37 -
 drivers/lguest/lguest_user.c  |2 -
 drivers/lguest/page_tables.c  |2 -
 drivers/lguest/segments.c |6 ++---
 include/linux/lguest_launcher.h   |2 -
 9 files changed, 56 insertions(+), 56 deletions(-)

===
--- a/drivers/lguest/core.c
+++ b/drivers/lguest/core.c
@@ -212,39 +212,40 @@ int lguest_address_ok(const struct lgues
 }
 
 /* Just like get_user, but don't let guest access lguest binary. */
-u32 lgread_u32(struct lguest *lg, u32 addr)
+u32 lgread_u32(struct lguest *lg, unsigned long addr)
 {
u32 val = 0;
 
/* Don't let them access lguest binary */
if (!lguest_address_ok(lg, addr, sizeof(val))
|| get_user(val, (u32 __user *)addr) != 0)
-   kill_guest(lg, "bad read address %u", addr);
+   kill_guest(lg, "bad read address %#lx", addr);
return val;
 }
 
-void lgwrite_u32(struct lguest *lg, u32 addr, u32 val)
+void lgwrite_u32(struct lguest *lg, unsigned long addr, u32 val)
 {
if (!lguest_address_ok(lg, addr, sizeof(val))
|| put_user(val, (u32 __user *)addr) != 0)
-   kill_guest(lg, "bad write address %u", addr);
-}
-
-void lgread(struct lguest *lg, void *b, u32 addr, unsigned bytes)
+   kill_guest(lg, "bad write address %#lx", addr);
+}
+
+void lgread(struct lguest *lg, void *b, unsigned long addr, unsigned bytes)
 {
if (!lguest_address_ok(lg, addr, bytes)
|| copy_from_user(b, (void __user *)addr, bytes) != 0) {
/* copy_from_user should do this, but as we rely on it... */
memset(b, 0, bytes);
-   kill_guest(lg, "bad read address %u len %u", addr, bytes);
-   }
-}
-
-void lgwrite(struct lguest *lg, u32 addr, const void *b, unsigned bytes)
+   kill_guest(lg, "bad read address %#lx len %u", addr, bytes);
+   }
+}
+
+void lgwrite(struct lguest *lg, unsigned long addr, const void *b,
+unsigned bytes)
 {
if (!lguest_address_ok(lg, addr, bytes)
|| copy_to_user((void __user *)addr, b, bytes) != 0)
-   kill_guest(lg, "bad write address %u len %u", addr, bytes);
+   kill_guest(lg, "bad write address %#lx len %u", addr, bytes);
 }
 
 static void set_ts(void)
@@ -294,7 +295,7 @@ static void run_guest_once(struct lguest
 : "memory", "%edx", "%ecx", "%edi", "%esi");
 }
 
-int run_guest(struct lguest *lg, char *__user user)
+int run_guest(struct lguest *lg, unsigned long __user *user)
 {
while (!lg->dead) {
unsigned int cr2 = 0; /* Damn gcc */
@@ -302,8 +303,8 @@ int run_guest(struct lguest *lg, char *_
/* Hypercalls first: we might have been out to userspace */
do_hypercalls(lg);
if (lg->dma_is_pending) {
-   if (put_user(lg->pending_dma, (unsigned long *)user) ||
-   put_user(lg->pending_key, (unsigned long *)user+1))
+   if (put_user(lg->pending_dma, user) ||
+   put_user(lg->pending_key, user+1))
return -EFAULT;
return sizeof(unsigned long)*2;
}
@@ -367,7 +368,7 @@ int run_guest(struct lguest *lg, char *_
if (deliver_trap(lg, lg->regs->trapnum))
continue;
 
-   kill_guest(lg, "unhandled trap %i at %#x (%#x)",
+   kill_guest(lg, "unhandled trap %li at %#lx (%#lx)",
   lg->regs->trapnum, lg->regs->eip,
   lg->regs->trapnum == 14 ? cr2 : lg->regs->errcode);
}
@@ -420,7 +421,7 @@ static int __init init(void)
lock_cpu_hotplug();
if (cpu_has_pge) { /* We have a broader idea of "global". */
cpu_had