comments below
On 08/16/13 13:13, arm...@redhat.com wrote:
> From: Markus Armbruster <arm...@redhat.com>
>
> Don't explode QEMUMachineInitArgs before passing it to
> sun4m_hw_init(), sun4uv_init().
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> hw/sparc/sun4m.c | 113
> ++++++++++++-----------------------------------------
> hw/sparc64/sun4u.c | 52 +++++++-----------------
> 2 files changed, 40 insertions(+), 125 deletions(-)
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 942ca37..36ef36f 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -836,12 +836,10 @@ static void dummy_fdc_tc(void *opaque, int irq, int
> level)
> {
> }
>
> -static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t
> RAM_size,
> - const char *boot_device,
> - const char *kernel_filename,
> - const char *kernel_cmdline,
> - const char *initrd_filename, const char *cpu_model)
> +static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> + QEMUMachineInitArgs *args)
> {
> + const char *cpu_model = args->cpu_model;
This may not be strictly necessary; in the first patch you modify
args->cpu_model too.
In any case the above is not wrong.
> @@ -878,13 +875,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>
> initrd_size = 0;
> initrd_addr = 0;
> - kernel_size = sun4u_load_kernel(kernel_filename, initrd_filename,
> + kernel_size = sun4u_load_kernel(args->kernel_filename,
> + args->initrd_filename,
> ram_size, &initrd_size, &initrd_addr,
> &kernel_addr, &kernel_entry);
The patch is correct / faithful here, but I smell some fubar in the code
that the patch keeps intact.
"ram_size" is apparently the global variable from "vl.c". So this
function gets the RAM size twice, once via RAM_size / args->ram_size,
then via the "ram_size" global. (They should have identical values,
"args.ram_size" in main() is initialized with "ram_size" the global.)
The rest of the code below this hunk (in the full source file, not just
in the patch) alternates between RAM_size / args->ram_size and
"ram_size" quite schizophrenically too; see eg. FW_CFG_RAM_SIZE.
Anyway the patch only improves things.
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
