[ Late comments, I know, sorry. Just happen to came across this. ]
On 2011-03-29 20:27, anthony.perard@xxxxxxxxxx wrote:
> From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>
> 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 <anthony.perard@xxxxxxxxxx>
> ---
> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home