[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH V12 05/17] xen: Add xenfv machine
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH V12 05/17] xen: Add xenfv machine |
Date: |
Fri, 08 Apr 2011 15:48:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
[ Late comments, I know, sorry. Just happen to came across this. ]
On 2011-03-29 20:27, address@hidden wrote:
> From: Anthony PERARD <address@hidden>
>
> Introduce the Xen FV (Fully Virtualized) machine to Qemu, some more Xen
> specific call will be added in further patches.
>
> Signed-off-by: Anthony PERARD <address@hidden>
> ---
> hw/pc.c | 19 +++++++++++++++++--
> hw/pc_piix.c | 17 +++++++++++++++++
> hw/xen.h | 4 ++++
> 3 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 6939c04..d7732d4 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -41,6 +41,7 @@
> #include "sysemu.h"
> #include "blockdev.h"
> #include "ui/qemu-spice.h"
> +#include "xen.h"
>
> /* output Bochs bios info messages */
> //#define DEBUG_BIOS
> @@ -918,7 +919,11 @@ static void pc_cpu_reset(void *opaque)
> CPUState *env = opaque;
>
> cpu_reset(env);
> - env->halted = !cpu_is_bsp(env);
> + if (!xen_enabled()) {
> + env->halted = !cpu_is_bsp(env);
> + } else {
> + env->halted = 1;
> + }
Not a fault of your patch, but pc_cpu_reset should not exist in the
first place. Setting env->halted should be done in i386's cpu_reset.
I think Xen would be better off with installing a custom VCPU reset
handler and overwrite halted according to its own needs. KVM is doing
the same. Then we could clean up pc_cpu_reset without bothering Xen.
> }
>
> static CPUState *pc_new_cpu(const char *cpu_model)
> @@ -952,7 +957,12 @@ void pc_cpus_init(const char *cpu_model)
> #endif
> }
>
> - for(i = 0; i < smp_cpus; i++) {
> + if (!xen_enabled()) {
> + for(i = 0; i < smp_cpus; i++) {
> + pc_new_cpu(cpu_model);
> + }
> + } else {
> + /* Xen require only one Qemu VCPU */
> pc_new_cpu(cpu_model);
This looks a bit fishy. What is the semantic of -smp 2 or more in Xen
mode? If that is an invalid/unused configuration option, catch that and
reject it instead of installing this workaround. If it has a valid
semantic, please elaborate why you need to restrict the number of
instantiated cpus. Just to optimize memory usage?
> }
> }
> @@ -980,6 +990,11 @@ void pc_memory_init(ram_addr_t ram_size,
> *above_4g_mem_size_p = above_4g_mem_size;
> *below_4g_mem_size_p = below_4g_mem_size;
>
> + if (xen_enabled()) {
> + /* Nothing to do for Xen */
> + return;
> + }
> +
This looks fragile /wrt potential future changes of pc_memory_init.
Can't those bits Xen is interested in, ie. the above/below_4g_mem_size
calculation, be moved into a separate function or even to the caller
(should be trivial enough, the interface of pc_memory_init is clumsy in
this regard anyway) so that you can simply skip pc_memory_init when in
Xen mode?
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
- [Qemu-devel] Re: [PATCH V12 05/17] xen: Add xenfv machine,
Jan Kiszka <=
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Anthony PERARD, 2011/04/11
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Jan Kiszka, 2011/04/11
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Anthony PERARD, 2011/04/12
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Jan Kiszka, 2011/04/12
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Stefano Stabellini, 2011/04/13
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Jan Kiszka, 2011/04/13
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Stefano Stabellini, 2011/04/13
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Jan Kiszka, 2011/04/13
- Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine, Stefano Stabellini, 2011/04/13