diff mbox

[v7,2/3] generic-loader: Add a generic loader

Message ID 1d2acebce401b23134d2211d7488df71a9f056b9.1464201427.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis May 25, 2016, 6:49 p.m. UTC
Add a generic loader to QEMU which can be used to load images or set
memory values.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
V7:
 - Rebase
V6:
 - Add error checking
V5:
 - Rebase
V4:
 - Allow the loader to work with every architecture
 - Move the file to hw/core
 - Increase the maximum number of CPUs
 - Make the CPU operations conditional
 - Convert the cpu option to cpu-num
 - Require the user to specify endianess
V3:
 - Pass the ram_size to load_image_targphys()
V2:
 - Add maintainers entry
 - Perform bounds checking
 - Register and unregister the reset in the realise/unrealise
Changes since RFC:
 - Add BE support

 MAINTAINERS                      |   6 ++
 hw/core/Makefile.objs            |   2 +
 hw/core/generic-loader.c         | 170 +++++++++++++++++++++++++++++++++++++++
 include/hw/core/generic-loader.h |  45 +++++++++++
 4 files changed, 223 insertions(+)
 create mode 100644 hw/core/generic-loader.c
 create mode 100644 include/hw/core/generic-loader.h

Comments

Peter Maydell June 9, 2016, 5:50 p.m. UTC | #1
On 25 May 2016 at 19:49, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Add a generic loader to QEMU which can be used to load images or set
> memory values.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> V7:
>  - Rebase
> V6:
>  - Add error checking
> V5:
>  - Rebase
> V4:
>  - Allow the loader to work with every architecture
>  - Move the file to hw/core
>  - Increase the maximum number of CPUs
>  - Make the CPU operations conditional
>  - Convert the cpu option to cpu-num
>  - Require the user to specify endianess
> V3:
>  - Pass the ram_size to load_image_targphys()
> V2:
>  - Add maintainers entry
>  - Perform bounds checking
>  - Register and unregister the reset in the realise/unrealise
> Changes since RFC:
>  - Add BE support
>
>  MAINTAINERS                      |   6 ++
>  hw/core/Makefile.objs            |   2 +
>  hw/core/generic-loader.c         | 170 +++++++++++++++++++++++++++++++++++++++
>  include/hw/core/generic-loader.h |  45 +++++++++++
>  4 files changed, 223 insertions(+)
>  create mode 100644 hw/core/generic-loader.c
>  create mode 100644 include/hw/core/generic-loader.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 81e7fac..b4a77d8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -972,6 +972,12 @@ F: hw/acpi/nvdimm.c
>  F: hw/mem/nvdimm.c
>  F: include/hw/mem/nvdimm.h
>
> +Generic Loader
> +M: Alistair Francis <alistair.francis@xilinx.com>
> +S: Maintained
> +F: hw/core/generic-loader.c
> +F: include/hw/core/generic-loader.h
> +
>  Subsystems
>  ----------
>  Audio
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index 70951d4..eb00e7d 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -15,3 +15,5 @@ common-obj-$(CONFIG_SOFTMMU) += null-machine.o
>  common-obj-$(CONFIG_SOFTMMU) += loader.o
>  common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
>  common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
> +
> +obj-$(CONFIG_SOFTMMU) += generic-loader.o
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> new file mode 100644
> index 0000000..7160d58
> --- /dev/null
> +++ b/hw/core/generic-loader.c
> @@ -0,0 +1,170 @@
> +/*
> + * Generic Loader
> + *
> + * Copyright (C) 2014 Li Guang
> + * Copyright (C) 2016 Xilinx Inc.
> + * Written by Li Guang <lig.fnst@cn.fujitsu.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qom/cpu.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/dma.h"
> +#include "hw/loader.h"
> +#include "qapi/error.h"
> +#include "hw/core/generic-loader.h"
> +
> +#define CPU_NONE 0xFFFF
> +
> +static void generic_loader_reset(void *opaque)
> +{
> +    GenericLoaderState *s = GENERIC_LOADER(opaque);
> +
> +    if (s->cpu) {
> +        CPUClass *cc = CPU_GET_CLASS(s->cpu);
> +        cpu_reset(s->cpu);
> +        if (cc) {
> +            cc->set_pc(s->cpu, s->addr);
> +        }
> +    }
> +
> +    if (s->data_len) {
> +        assert(s->data_len < sizeof(s->data));
> +        dma_memory_write((s->cpu ? s->cpu : first_cpu)->as, s->addr, &s->data,
> +                         s->data_len);
> +    }
> +}
> +
> +static void generic_loader_realize(DeviceState *dev, Error **errp)
> +{
> +    GenericLoaderState *s = GENERIC_LOADER(dev);
> +    hwaddr entry;
> +    int big_endian;
> +    int size = 0;
> +
> +    /* Perform some eror checking on the users options */

"error". "user's".

> +    if (s->data || s->data_len  || s->data_be) {
> +        /* User is loading memory values */
> +        if (s->file) {
> +            error_setg(errp, "Specifying a file is not supported when loading "
> +                       "memory values.");

I think we don't generally have trailing '.' for error_setg() messages.

> +            return;
> +        } else if (s->force_raw) {
> +            error_setg(errp, "Specifying force raw is not supported when "
> +                       "loading memory values.");
> +            return;
> +        } else if (!s->data || !s->data_len) {
> +            error_setg(errp, "Both data and data length must be specified.");
> +            return;
> +        }
> +    } else if (s->file || s->force_raw)  {
> +        /* User is loading an image */
> +        if (s->data || s->data_len || s->data_be) {
> +            error_setg(errp, "Data can not be specified when loading an "
> +                       "image.");
> +            return;
> +        }
> +    }
> +
> +    qemu_register_reset(generic_loader_reset, dev);
> +
> +    if (s->cpu_num != CPU_NONE) {
> +        s->cpu = qemu_get_cpu(s->cpu_num);
> +        if (!s->cpu) {
> +            error_setg(errp, "Specified boot CPU#%d is nonexistent",
> +                       s->cpu_num);
> +            return;
> +        }
> +    }
> +
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    big_endian = 1;
> +#else
> +    big_endian = 0;
> +#endif
> +
> +    if (s->file) {
> +        if (!s->force_raw) {
> +            size = load_elf(s->file, NULL, NULL, &entry, NULL, NULL,
> +                            big_endian, 0, 0, 0);
> +
> +            if (size < 0) {
> +                size = load_uimage(s->file, &entry, NULL, NULL, NULL, NULL);
> +            }
> +        }
> +
> +        if (size < 0) {
> +            /* Default to the maximum size being the machine's ram size */
> +            size = load_image_targphys(s->file, s->addr, ram_size);
> +        } else {
> +            s->addr = entry;
> +        }
> +
> +        if (size < 0) {
> +            error_setg(errp, "Cannot load specified image %s", s->file);
> +            return;
> +        }
> +    }
> +
> +    if (s->data_len && (s->data_len > sizeof(s->data))) {
> +        error_setg(errp, "data-len cannot be more then the data size");
> +        return;
> +    }

This seems like it should go earlier, with the other parameter
validation checks. (Also, isn't this actually checking "is data-len
larger than our supported maximum of 8" ?)

> +
> +    /* Convert the data endiannes */
> +    if (s->data_be) {
> +        s->data = cpu_to_be64(s->data);
> +    } else {
> +        s->data = cpu_to_le64(s->data);
> +    }
> +}
> +
> +static void generic_loader_unrealize(DeviceState *dev, Error **errp)
> +{
> +    qemu_unregister_reset(generic_loader_reset, dev);
> +}
> +
> +static Property generic_loader_props[] = {
> +    DEFINE_PROP_UINT64("addr", GenericLoaderState, addr, 0),
> +    DEFINE_PROP_UINT64("data", GenericLoaderState, data, 0),
> +    DEFINE_PROP_UINT8("data-len", GenericLoaderState, data_len, 0),
> +    DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false),
> +    DEFINE_PROP_UINT16("cpu-num", GenericLoaderState, cpu_num, CPU_NONE),
> +    DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false),
> +    DEFINE_PROP_STRING("file", GenericLoaderState, file),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void generic_loader_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = generic_loader_realize;
> +    dc->unrealize = generic_loader_unrealize;
> +    dc->props = generic_loader_props;
> +    dc->desc = "Generic Loader";
> +}
> +
> +static TypeInfo generic_loader_info = {
> +    .name = TYPE_GENERIC_LOADER,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(GenericLoaderState),
> +    .class_init = generic_loader_class_init,
> +};
> +
> +static void generic_loader_register_type(void)
> +{
> +    type_register_static(&generic_loader_info);
> +}
> +
> +type_init(generic_loader_register_type)
> diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h
> new file mode 100644
> index 0000000..ceec51f
> --- /dev/null
> +++ b/include/hw/core/generic-loader.h
> @@ -0,0 +1,45 @@
> +/*
> + * Generic Loader
> + *
> + * Copyright (C) 2014 Li Guang
> + * Written by Li Guang <lig.fnst@cn.fujitsu.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#ifndef GENERIC_LOADER_H
> +#define GENERIC_LOADER_H
> +
> +#include "elf.h"
> +
> +typedef struct GenericLoaderState {
> +    /* <private> */
> +    DeviceState parent_obj;
> +
> +    /* <public> */
> +    CPUState *cpu;
> +
> +    uint64_t addr;
> +    uint64_t data;
> +    uint8_t data_len;
> +    uint16_t cpu_num;

(Why 16 bits in particular?)

> +
> +    char *file;
> +
> +    bool force_raw;
> +    bool data_be;
> +} GenericLoaderState;
> +
> +#define TYPE_GENERIC_LOADER "loader"
> +#define GENERIC_LOADER(obj) OBJECT_CHECK(GenericLoaderState, (obj), \
> +                                         TYPE_GENERIC_LOADER)
> +
> +#endif

thanks
-- PMM
Alistair Francis June 13, 2016, 5:25 p.m. UTC | #2
On Thu, Jun 9, 2016 at 10:50 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 May 2016 at 19:49, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> Add a generic loader to QEMU which can be used to load images or set
>> memory values.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>> V7:
>>  - Rebase
>> V6:
>>  - Add error checking
>> V5:
>>  - Rebase
>> V4:
>>  - Allow the loader to work with every architecture
>>  - Move the file to hw/core
>>  - Increase the maximum number of CPUs
>>  - Make the CPU operations conditional
>>  - Convert the cpu option to cpu-num
>>  - Require the user to specify endianess
>> V3:
>>  - Pass the ram_size to load_image_targphys()
>> V2:
>>  - Add maintainers entry
>>  - Perform bounds checking
>>  - Register and unregister the reset in the realise/unrealise
>> Changes since RFC:
>>  - Add BE support
>>
>>  MAINTAINERS                      |   6 ++
>>  hw/core/Makefile.objs            |   2 +
>>  hw/core/generic-loader.c         | 170 +++++++++++++++++++++++++++++++++++++++
>>  include/hw/core/generic-loader.h |  45 +++++++++++
>>  4 files changed, 223 insertions(+)
>>  create mode 100644 hw/core/generic-loader.c
>>  create mode 100644 include/hw/core/generic-loader.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 81e7fac..b4a77d8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -972,6 +972,12 @@ F: hw/acpi/nvdimm.c
>>  F: hw/mem/nvdimm.c
>>  F: include/hw/mem/nvdimm.h
>>
>> +Generic Loader
>> +M: Alistair Francis <alistair.francis@xilinx.com>
>> +S: Maintained
>> +F: hw/core/generic-loader.c
>> +F: include/hw/core/generic-loader.h
>> +
>>  Subsystems
>>  ----------
>>  Audio
>> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
>> index 70951d4..eb00e7d 100644
>> --- a/hw/core/Makefile.objs
>> +++ b/hw/core/Makefile.objs
>> @@ -15,3 +15,5 @@ common-obj-$(CONFIG_SOFTMMU) += null-machine.o
>>  common-obj-$(CONFIG_SOFTMMU) += loader.o
>>  common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
>>  common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
>> +
>> +obj-$(CONFIG_SOFTMMU) += generic-loader.o
>> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
>> new file mode 100644
>> index 0000000..7160d58
>> --- /dev/null
>> +++ b/hw/core/generic-loader.c
>> @@ -0,0 +1,170 @@
>> +/*
>> + * Generic Loader
>> + *
>> + * Copyright (C) 2014 Li Guang
>> + * Copyright (C) 2016 Xilinx Inc.
>> + * Written by Li Guang <lig.fnst@cn.fujitsu.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>> + * for more details.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qom/cpu.h"
>> +#include "hw/sysbus.h"
>> +#include "sysemu/dma.h"
>> +#include "hw/loader.h"
>> +#include "qapi/error.h"
>> +#include "hw/core/generic-loader.h"
>> +
>> +#define CPU_NONE 0xFFFF
>> +
>> +static void generic_loader_reset(void *opaque)
>> +{
>> +    GenericLoaderState *s = GENERIC_LOADER(opaque);
>> +
>> +    if (s->cpu) {
>> +        CPUClass *cc = CPU_GET_CLASS(s->cpu);
>> +        cpu_reset(s->cpu);
>> +        if (cc) {
>> +            cc->set_pc(s->cpu, s->addr);
>> +        }
>> +    }
>> +
>> +    if (s->data_len) {
>> +        assert(s->data_len < sizeof(s->data));
>> +        dma_memory_write((s->cpu ? s->cpu : first_cpu)->as, s->addr, &s->data,
>> +                         s->data_len);
>> +    }
>> +}
>> +
>> +static void generic_loader_realize(DeviceState *dev, Error **errp)
>> +{
>> +    GenericLoaderState *s = GENERIC_LOADER(dev);
>> +    hwaddr entry;
>> +    int big_endian;
>> +    int size = 0;
>> +
>> +    /* Perform some eror checking on the users options */
>
> "error". "user's".

Fixed.

>
>> +    if (s->data || s->data_len  || s->data_be) {
>> +        /* User is loading memory values */
>> +        if (s->file) {
>> +            error_setg(errp, "Specifying a file is not supported when loading "
>> +                       "memory values.");
>
> I think we don't generally have trailing '.' for error_setg() messages.

Ok, I have removed all the trailing '.'s.

>
>> +            return;
>> +        } else if (s->force_raw) {
>> +            error_setg(errp, "Specifying force raw is not supported when "
>> +                       "loading memory values.");
>> +            return;
>> +        } else if (!s->data || !s->data_len) {
>> +            error_setg(errp, "Both data and data length must be specified.");
>> +            return;
>> +        }
>> +    } else if (s->file || s->force_raw)  {
>> +        /* User is loading an image */
>> +        if (s->data || s->data_len || s->data_be) {
>> +            error_setg(errp, "Data can not be specified when loading an "
>> +                       "image.");
>> +            return;
>> +        }
>> +    }
>> +
>> +    qemu_register_reset(generic_loader_reset, dev);
>> +
>> +    if (s->cpu_num != CPU_NONE) {
>> +        s->cpu = qemu_get_cpu(s->cpu_num);
>> +        if (!s->cpu) {
>> +            error_setg(errp, "Specified boot CPU#%d is nonexistent",
>> +                       s->cpu_num);
>> +            return;
>> +        }
>> +    }
>> +
>> +#ifdef TARGET_WORDS_BIGENDIAN
>> +    big_endian = 1;
>> +#else
>> +    big_endian = 0;
>> +#endif
>> +
>> +    if (s->file) {
>> +        if (!s->force_raw) {
>> +            size = load_elf(s->file, NULL, NULL, &entry, NULL, NULL,
>> +                            big_endian, 0, 0, 0);
>> +
>> +            if (size < 0) {
>> +                size = load_uimage(s->file, &entry, NULL, NULL, NULL, NULL);
>> +            }
>> +        }
>> +
>> +        if (size < 0) {
>> +            /* Default to the maximum size being the machine's ram size */
>> +            size = load_image_targphys(s->file, s->addr, ram_size);
>> +        } else {
>> +            s->addr = entry;
>> +        }
>> +
>> +        if (size < 0) {
>> +            error_setg(errp, "Cannot load specified image %s", s->file);
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (s->data_len && (s->data_len > sizeof(s->data))) {
>> +        error_setg(errp, "data-len cannot be more then the data size");
>> +        return;
>> +    }
>
> This seems like it should go earlier, with the other parameter
> validation checks. (Also, isn't this actually checking "is data-len
> larger than our supported maximum of 8" ?)

I have moved this earlier and added a check for being greater then 8.

>
>> +
>> +    /* Convert the data endiannes */
>> +    if (s->data_be) {
>> +        s->data = cpu_to_be64(s->data);
>> +    } else {
>> +        s->data = cpu_to_le64(s->data);
>> +    }
>> +}
>> +
>> +static void generic_loader_unrealize(DeviceState *dev, Error **errp)
>> +{
>> +    qemu_unregister_reset(generic_loader_reset, dev);
>> +}
>> +
>> +static Property generic_loader_props[] = {
>> +    DEFINE_PROP_UINT64("addr", GenericLoaderState, addr, 0),
>> +    DEFINE_PROP_UINT64("data", GenericLoaderState, data, 0),
>> +    DEFINE_PROP_UINT8("data-len", GenericLoaderState, data_len, 0),
>> +    DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false),
>> +    DEFINE_PROP_UINT16("cpu-num", GenericLoaderState, cpu_num, CPU_NONE),
>> +    DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false),
>> +    DEFINE_PROP_STRING("file", GenericLoaderState, file),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void generic_loader_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = generic_loader_realize;
>> +    dc->unrealize = generic_loader_unrealize;
>> +    dc->props = generic_loader_props;
>> +    dc->desc = "Generic Loader";
>> +}
>> +
>> +static TypeInfo generic_loader_info = {
>> +    .name = TYPE_GENERIC_LOADER,
>> +    .parent = TYPE_DEVICE,
>> +    .instance_size = sizeof(GenericLoaderState),
>> +    .class_init = generic_loader_class_init,
>> +};
>> +
>> +static void generic_loader_register_type(void)
>> +{
>> +    type_register_static(&generic_loader_info);
>> +}
>> +
>> +type_init(generic_loader_register_type)
>> diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h
>> new file mode 100644
>> index 0000000..ceec51f
>> --- /dev/null
>> +++ b/include/hw/core/generic-loader.h
>> @@ -0,0 +1,45 @@
>> +/*
>> + * Generic Loader
>> + *
>> + * Copyright (C) 2014 Li Guang
>> + * Written by Li Guang <lig.fnst@cn.fujitsu.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>> + * for more details.
>> + */
>> +
>> +#ifndef GENERIC_LOADER_H
>> +#define GENERIC_LOADER_H
>> +
>> +#include "elf.h"
>> +
>> +typedef struct GenericLoaderState {
>> +    /* <private> */
>> +    DeviceState parent_obj;
>> +
>> +    /* <public> */
>> +    CPUState *cpu;
>> +
>> +    uint64_t addr;
>> +    uint64_t data;
>> +    uint8_t data_len;
>> +    uint16_t cpu_num;
>
> (Why 16 bits in particular?)

No reason in particular. Do you think it should be something else?

Thanks,

Alistair

>
>> +
>> +    char *file;
>> +
>> +    bool force_raw;
>> +    bool data_be;
>> +} GenericLoaderState;
>> +
>> +#define TYPE_GENERIC_LOADER "loader"
>> +#define GENERIC_LOADER(obj) OBJECT_CHECK(GenericLoaderState, (obj), \
>> +                                         TYPE_GENERIC_LOADER)
>> +
>> +#endif
>
> thanks
> -- PMM
>
Peter Maydell June 13, 2016, 6:05 p.m. UTC | #3
On 13 June 2016 at 18:25, Alistair Francis <alistair.francis@xilinx.com> wrote:
> On Thu, Jun 9, 2016 at 10:50 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 25 May 2016 at 19:49, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>> +    uint16_t cpu_num;
>>
>> (Why 16 bits in particular?)
>
> No reason in particular. Do you think it should be something else?

Most other places that deal with cpu indexes use 'int'.
There doesn't seem any pressing reason to limit it to a
16 bit value here. (You want uint32_t for migration
purposes rather than just 'int' though.)

thanks
-- PMM
diff mbox

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 81e7fac..b4a77d8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -972,6 +972,12 @@  F: hw/acpi/nvdimm.c
 F: hw/mem/nvdimm.c
 F: include/hw/mem/nvdimm.h
 
+Generic Loader
+M: Alistair Francis <alistair.francis@xilinx.com>
+S: Maintained
+F: hw/core/generic-loader.c
+F: include/hw/core/generic-loader.h
+
 Subsystems
 ----------
 Audio
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 70951d4..eb00e7d 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -15,3 +15,5 @@  common-obj-$(CONFIG_SOFTMMU) += null-machine.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
 common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
+
+obj-$(CONFIG_SOFTMMU) += generic-loader.o
diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
new file mode 100644
index 0000000..7160d58
--- /dev/null
+++ b/hw/core/generic-loader.c
@@ -0,0 +1,170 @@ 
+/*
+ * Generic Loader
+ *
+ * Copyright (C) 2014 Li Guang
+ * Copyright (C) 2016 Xilinx Inc.
+ * Written by Li Guang <lig.fnst@cn.fujitsu.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "qom/cpu.h"
+#include "hw/sysbus.h"
+#include "sysemu/dma.h"
+#include "hw/loader.h"
+#include "qapi/error.h"
+#include "hw/core/generic-loader.h"
+
+#define CPU_NONE 0xFFFF
+
+static void generic_loader_reset(void *opaque)
+{
+    GenericLoaderState *s = GENERIC_LOADER(opaque);
+
+    if (s->cpu) {
+        CPUClass *cc = CPU_GET_CLASS(s->cpu);
+        cpu_reset(s->cpu);
+        if (cc) {
+            cc->set_pc(s->cpu, s->addr);
+        }
+    }
+
+    if (s->data_len) {
+        assert(s->data_len < sizeof(s->data));
+        dma_memory_write((s->cpu ? s->cpu : first_cpu)->as, s->addr, &s->data,
+                         s->data_len);
+    }
+}
+
+static void generic_loader_realize(DeviceState *dev, Error **errp)
+{
+    GenericLoaderState *s = GENERIC_LOADER(dev);
+    hwaddr entry;
+    int big_endian;
+    int size = 0;
+
+    /* Perform some eror checking on the users options */
+    if (s->data || s->data_len  || s->data_be) {
+        /* User is loading memory values */
+        if (s->file) {
+            error_setg(errp, "Specifying a file is not supported when loading "
+                       "memory values.");
+            return;
+        } else if (s->force_raw) {
+            error_setg(errp, "Specifying force raw is not supported when "
+                       "loading memory values.");
+            return;
+        } else if (!s->data || !s->data_len) {
+            error_setg(errp, "Both data and data length must be specified.");
+            return;
+        }
+    } else if (s->file || s->force_raw)  {
+        /* User is loading an image */
+        if (s->data || s->data_len || s->data_be) {
+            error_setg(errp, "Data can not be specified when loading an "
+                       "image.");
+            return;
+        }
+    }
+
+    qemu_register_reset(generic_loader_reset, dev);
+
+    if (s->cpu_num != CPU_NONE) {
+        s->cpu = qemu_get_cpu(s->cpu_num);
+        if (!s->cpu) {
+            error_setg(errp, "Specified boot CPU#%d is nonexistent",
+                       s->cpu_num);
+            return;
+        }
+    }
+
+#ifdef TARGET_WORDS_BIGENDIAN
+    big_endian = 1;
+#else
+    big_endian = 0;
+#endif
+
+    if (s->file) {
+        if (!s->force_raw) {
+            size = load_elf(s->file, NULL, NULL, &entry, NULL, NULL,
+                            big_endian, 0, 0, 0);
+
+            if (size < 0) {
+                size = load_uimage(s->file, &entry, NULL, NULL, NULL, NULL);
+            }
+        }
+
+        if (size < 0) {
+            /* Default to the maximum size being the machine's ram size */
+            size = load_image_targphys(s->file, s->addr, ram_size);
+        } else {
+            s->addr = entry;
+        }
+
+        if (size < 0) {
+            error_setg(errp, "Cannot load specified image %s", s->file);
+            return;
+        }
+    }
+
+    if (s->data_len && (s->data_len > sizeof(s->data))) {
+        error_setg(errp, "data-len cannot be more then the data size");
+        return;
+    }
+
+    /* Convert the data endiannes */
+    if (s->data_be) {
+        s->data = cpu_to_be64(s->data);
+    } else {
+        s->data = cpu_to_le64(s->data);
+    }
+}
+
+static void generic_loader_unrealize(DeviceState *dev, Error **errp)
+{
+    qemu_unregister_reset(generic_loader_reset, dev);
+}
+
+static Property generic_loader_props[] = {
+    DEFINE_PROP_UINT64("addr", GenericLoaderState, addr, 0),
+    DEFINE_PROP_UINT64("data", GenericLoaderState, data, 0),
+    DEFINE_PROP_UINT8("data-len", GenericLoaderState, data_len, 0),
+    DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false),
+    DEFINE_PROP_UINT16("cpu-num", GenericLoaderState, cpu_num, CPU_NONE),
+    DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false),
+    DEFINE_PROP_STRING("file", GenericLoaderState, file),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void generic_loader_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = generic_loader_realize;
+    dc->unrealize = generic_loader_unrealize;
+    dc->props = generic_loader_props;
+    dc->desc = "Generic Loader";
+}
+
+static TypeInfo generic_loader_info = {
+    .name = TYPE_GENERIC_LOADER,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(GenericLoaderState),
+    .class_init = generic_loader_class_init,
+};
+
+static void generic_loader_register_type(void)
+{
+    type_register_static(&generic_loader_info);
+}
+
+type_init(generic_loader_register_type)
diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h
new file mode 100644
index 0000000..ceec51f
--- /dev/null
+++ b/include/hw/core/generic-loader.h
@@ -0,0 +1,45 @@ 
+/*
+ * Generic Loader
+ *
+ * Copyright (C) 2014 Li Guang
+ * Written by Li Guang <lig.fnst@cn.fujitsu.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#ifndef GENERIC_LOADER_H
+#define GENERIC_LOADER_H
+
+#include "elf.h"
+
+typedef struct GenericLoaderState {
+    /* <private> */
+    DeviceState parent_obj;
+
+    /* <public> */
+    CPUState *cpu;
+
+    uint64_t addr;
+    uint64_t data;
+    uint8_t data_len;
+    uint16_t cpu_num;
+
+    char *file;
+
+    bool force_raw;
+    bool data_be;
+} GenericLoaderState;
+
+#define TYPE_GENERIC_LOADER "loader"
+#define GENERIC_LOADER(obj) OBJECT_CHECK(GenericLoaderState, (obj), \
+                                         TYPE_GENERIC_LOADER)
+
+#endif