[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] bsd-user: move arch/OS dependent code out o
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] bsd-user: move arch/OS dependent code out of main.c |
Date: |
Thu, 3 Jul 2014 14:45:20 +0100 |
On 20 June 2014 01:19, Sean Bruno <address@hidden> wrote:
> From: Stacey Son <address@hidden>
>
> This change moves the cpu initialization and main loop code from
> main.c to the OS and arch dependent directories. This eliminates
> many of the #ifdef's in main.c. The cpu initialization and loop
> code is now located in the arch directory along with target arch
> support code.
>
> Signed-off-by: Sean Bruno <address@hidden>
> ---
> bsd-user/Makefile.objs | 2 +-
> bsd-user/elfload.c | 2 +-
> bsd-user/freebsd/host_os.h | 46 ++
> bsd-user/freebsd/target_os_vmparam.h | 23 +
> bsd-user/i386/target_arch.h | 13 +
> bsd-user/i386/target_arch_cpu.c | 79 +++
> bsd-user/i386/target_arch_cpu.h | 300 +++++++++++
> bsd-user/i386/target_arch_vmparam.h | 28 +
> bsd-user/i386/target_signal.h | 6 -
> bsd-user/main.c | 927
> +++++++--------------------------
> bsd-user/mmap.c | 2 +-
> bsd-user/netbsd/host_os.h | 31 ++
> bsd-user/netbsd/os-strace.h | 2 +-
> bsd-user/netbsd/target_os_vmparam.h | 23 +
> bsd-user/openbsd/host_os.h | 31 ++
> bsd-user/openbsd/os-strace.h | 2 +-
> bsd-user/openbsd/target_os_vmparam.h | 23 +
> bsd-user/qemu.h | 21 +-
> bsd-user/sparc/target_arch.h | 11 +
> bsd-user/sparc/target_arch_cpu.c | 113 ++++
> bsd-user/sparc/target_arch_cpu.h | 158 ++++++
> bsd-user/sparc/target_arch_vmparam.h | 37 ++
> bsd-user/sparc/target_signal.h | 5 -
> bsd-user/sparc64/target_arch.h | 11 +
> bsd-user/sparc64/target_arch_cpu.c | 118 +++++
> bsd-user/sparc64/target_arch_cpu.h | 191 +++++++
> bsd-user/sparc64/target_arch_vmparam.h | 37 ++
> bsd-user/sparc64/target_signal.h | 5 -
> bsd-user/x86_64/target_arch.h | 13 +
> bsd-user/x86_64/target_arch_cpu.c | 79 +++
> bsd-user/x86_64/target_arch_cpu.h | 322 ++++++++++++
> bsd-user/x86_64/target_arch_vmparam.h | 28 +
> bsd-user/x86_64/target_signal.h | 5 -
> 33 files changed, 1930 insertions(+), 764 deletions(-)
Oof. I'm afraid this still needs to be separated out into
separate patches a bit more. This patch seems to have:
* some things which are unrelated to the code-refactoring
it claims to be doing (I've noted a few below)
* movement of multiple different functions (most notably,
this patch deals with both the main cpu loop and also
the target-cpu-init) which could be done in separate patches
* some minor style/whitespace changes
> +#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
> +/*
> + * When running 32-on-64 we should make sure we can fit all of the possible
> + * guest address space into a contiguous chunk of virtual host memory.
> + *
> + * This way we will never overlap with our own libraries or binaries or stack
> + * or anything else that QEMU maps.
> + */
> +unsigned long reserved_va = TARGET_RESERVED_VA;
> +#else
> unsigned long reserved_va;
> #endif
> +#endif /* CONFIG_USE_GUEST_BASE */
For instance, this is a separate change -- it's moving into
line with linux-user for the default reserved address, which
is good but not related to moving the arch/OS dependent code.
> +/* Helper routines for implementing atomic operations. */
>
> +/*
> + * To implement exclusive operations we force all cpus to synchronize.
> + * We don't require a full sync, only that no cpus are executing guest code.
> + * The alternative is to map target atomic ops onto host eqivalents,
> + * which requires quite a lot of per host/target work.
> + */
> +static pthread_mutex_t cpu_list_mutex = PTHREAD_MUTEX_INITIALIZER;
> +static pthread_mutex_t exclusive_lock = PTHREAD_MUTEX_INITIALIZER;
> +static pthread_cond_t exclusive_cond = PTHREAD_COND_INITIALIZER;
> +static pthread_cond_t exclusive_resume = PTHREAD_COND_INITIALIZER;
> +static int pending_cpus;
This atomic-operations stuff also looks like a new feature
that should go in its own patch.
> +
> +#if defined(CONFIG_USE_NPTL)
Anything using CONFIG_USE_NPTL needs fixing, because
configure no longer sets that at all: using NPTL became the
default once we converted all the Linux archs to it.
> +void gemu_log(const char *fmt, ...)
> +{
> + va_list ap;
> +
> + va_start(ap, fmt);
> + vfprintf(stderr, fmt, ap);
> + va_end(ap);
> +}
This function just randomly moved location; seems a bit
pointless, but if you want to do it don't stuff it into this patch.
> @@ -767,7 +362,7 @@ int main(int argc, char **argv)
> #endif
>
> optind = 1;
> - for(;;) {
> + for (;;) {
Stray style/whitespace only change, don't bother or put it in
its own patch.
> - if (loader_exec(filename, argv+optind, target_environ, regs, info) != 0)
> {
> + if (loader_exec(filename, argv+optind, target_environ, regs, info)) {
Another style-only change.
> @@ -25,7 +25,7 @@ static inline void do_os_print_sysarch(const struct
> syscallname *name,
> abi_long arg5, abi_long arg6)
> {
> qemu_log("qemu: Unsupported syscall %s\n", __func__);
> - return -TARGET_ENOSYS;
> + return;
> }
Oh look, here's the fix for a bug I pointed out in review of
patch 1/4 :-)
thanks
-- PMM
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 2/4] bsd-user: move arch/OS dependent code out of main.c,
Peter Maydell <=