[PATCH 0/5] some small cleanups for xen_machine_pv.c

[PATCH 0/5] some small cleanups for xen_machine_pv.c

[Gerd Hoffmann]

  Hi,

Here are a few cleanup patches for qemu-xen's xen_machine_pv.c, in
preparation for merging into upstream qemu.

please apply,
  Gerd

[PATCH 1/5] xen_machine_pv.c: switch to C99 initializers.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/xen_machine_pv.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index 674ffca..df34590 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -72,10 +72,10 @@ static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
 }
 
 QEMUMachine xenpv_machine = {
-    "xenpv",
-    "Xen Para-virtualized PC",
-    xen_init_pv,
-    BIOS_SIZE | RAMSIZE_FIXED,
+    .name = "xenpv",
+    .desc = "Xen Para-virtualized PC",
+    .init = xen_init_pv,
+    .ram_require = BIOS_SIZE | RAMSIZE_FIXED,
     .max_cpus = 1,
     .nodisk_ok = 1,
 };
-- 
1.6.1.3

[PATCH 2/5] xen_machine_pv.c: delete pointless coomment.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/xen_machine_pv.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index df34590..624d8dd 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -33,11 +33,6 @@ enum xen_mode xen_mode = XEN_EMULATE;
 
 extern void init_blktap(void);
 
-
-/* The Xen PV machine currently provides
- *   - a virtual framebuffer
- *   - ....
- */
 static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
 			const char *boot_device, DisplayState *ds,
 			const char *kernel_filename,
-- 
1.6.1.3

[PATCH 3/5] Fixup dummy cpu setup.

[Gerd Hoffmann]

Passing NULL to cpu_init() doesn't work in upstream qemu.
Also make sure the dummy cpu is in halted mode.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/xen_machine_pv.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index 624d8dd..df28be9 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -51,7 +51,15 @@ static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
 #endif
 
     /* Initialize a dummy CPU */
-    env = cpu_init(NULL);
+    if (cpu_model == NULL) {
+#ifdef TARGET_X86_64
+        cpu_model = "qemu64";
+#else
+        cpu_model = "qemu32";
+#endif
+    }
+    env = cpu_init(cpu_model);
+    env->halted = 1;
 
     /* Initialize backend core & drivers */
     if (-1 == xen_be_init()) {
-- 
1.6.1.3

Re: [PATCH 3/5] Fixup dummy cpu setup.

[Simon Horman]

On Fri, Mar 20, 2009 at 05:02:49PM +0100, Gerd Hoffmann wrote:
> Passing NULL to cpu_init() doesn't work in upstream qemu.
> Also make sure the dummy cpu is in halted mode.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/xen_machine_pv.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
> index 624d8dd..df28be9 100644
> --- a/hw/xen_machine_pv.c
> +++ b/hw/xen_machine_pv.c
> @@ -51,7 +51,15 @@ static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
>  #endif
>  
>      /* Initialize a dummy CPU */
> -    env = cpu_init(NULL);
> +    if (cpu_model == NULL) {
> +#ifdef TARGET_X86_64
> +        cpu_model = "qemu64";
> +#else
> +        cpu_model = "qemu32";
> +#endif
> +    }
> +    env = cpu_init(cpu_model);
> +    env->halted = 1;
>  
>      /* Initialize backend core & drivers */
>      if (-1 == xen_be_init()) {

Is there a way to achieve this without ifdef nastiness?

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en

Re: [PATCH 3/5] Fixup dummy cpu setup.

[Gerd Hoffmann]

Simon Horman wrote:
>> -    env = cpu_init(NULL);
>> +    if (cpu_model == NULL) {
>> +#ifdef TARGET_X86_64
>> +        cpu_model = "qemu64";
>> +#else
>> +        cpu_model = "qemu32";
>> +#endif
>> +    }
>> +    env = cpu_init(cpu_model);

> Is there a way to achieve this without ifdef nastiness?

Well, one could try to make cpu_init(NULL) pick a sane default.  But in
the end that most likely would just be moving the ifdef to another place.

cheers,
  Gerd

Re: [PATCH 3/5] Fixup dummy cpu setup.

[Simon Horman]

On Mon, Mar 23, 2009 at 12:25:15PM +0100, Gerd Hoffmann wrote:
> Simon Horman wrote:
> >> -    env = cpu_init(NULL);
> >> +    if (cpu_model == NULL) {
> >> +#ifdef TARGET_X86_64
> >> +        cpu_model = "qemu64";
> >> +#else
> >> +        cpu_model = "qemu32";
> >> +#endif
> >> +    }
> >> +    env = cpu_init(cpu_model);
> 
> > Is there a way to achieve this without ifdef nastiness?
> 
> Well, one could try to make cpu_init(NULL) pick a sane default.  But in
> the end that most likely would just be moving the ifdef to another place.

Understood.

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en

[PATCH 4/5] use uint32_t for xen_domid

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/xen.h            |    2 +-
 hw/xen_machine_pv.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/xen.h b/hw/xen.h
index 4bcc0f1..3c8da41 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -14,7 +14,7 @@ enum xen_mode {
     XEN_ATTACH        // attach to xen domain created by xend
 };
 
-extern int xen_domid;
+extern uint32_t xen_domid;
 extern enum xen_mode xen_mode;
 
 #endif /* QEMU_HW_XEN_H */
diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index df28be9..6525856 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -28,7 +28,7 @@
 #include "boards.h"
 #include "xen_backend.h"
 
-int xen_domid;
+uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 
 extern void init_blktap(void);
-- 
1.6.1.3

RE: [PATCH 4/5] use uint32_t for xen_domid

[Dan Magenheimer]

May not matter but isn't a domid limited to 16-bits
in the hypervisor?  Probably should be consistent
lest sign-extension or something cause a problem.

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, March 20, 2009 10:03 AM
> To: xen-devel@lists.xensource.com
> Cc: Ian.Jackson@eu.citrix.com; Gerd Hoffmann;
> stefano.stabellini@eu.citrix.com
> Subject: [Xen-devel] [PATCH 4/5] use uint32_t for xen_domid
> 
> 
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/xen.h            |    2 +-
>  hw/xen_machine_pv.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xen.h b/hw/xen.h
> index 4bcc0f1..3c8da41 100644
> --- a/hw/xen.h
> +++ b/hw/xen.h
> @@ -14,7 +14,7 @@ enum xen_mode {
>      XEN_ATTACH        // attach to xen domain created by xend
>  };
>  
> -extern int xen_domid;
> +extern uint32_t xen_domid;
>  extern enum xen_mode xen_mode;
>  
>  #endif /* QEMU_HW_XEN_H */
> diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
> index df28be9..6525856 100644
> --- a/hw/xen_machine_pv.c
> +++ b/hw/xen_machine_pv.c
> @@ -28,7 +28,7 @@
>  #include "boards.h"
>  #include "xen_backend.h"
>  
> -int xen_domid;
> +uint32_t xen_domid;
>  enum xen_mode xen_mode = XEN_EMULATE;
>  
>  extern void init_blktap(void);
> -- 
> 1.6.1.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

Re: [PATCH 4/5] use uint32_t for xen_domid

[Gerd Hoffmann]

Dan Magenheimer wrote:
> May not matter but isn't a domid limited to 16-bits
> in the hypervisor?  Probably should be consistent
> lest sign-extension or something cause a problem.

In /usr/include/xenctrl.h it is uint32_t everythere.

cheers,
  Gerd

[PATCH 5/5] kill unused variables

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/xen_machine_pv.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index 6525856..b7f8e28 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -41,8 +41,6 @@ static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
 			const char *cpu_model,
 			const char *direct_pci)
 {
-    struct xenfb *xenfb;
-    extern int domid;
     CPUState *env;
 
 #ifndef CONFIG_STUBDOM
-- 
1.6.1.3