On Thu, Jan 2, 2014 at 6:21 PM, Paolo Bonzini <pbonz...@redhat.com> wrote:
> Il 02/01/2014 06:50, Peter Crosthwaite ha scritto:
>> On Thu, Jan 2, 2014 at 3:35 PM, Li Guang <lig.f...@cn.fujitsu.com> wrote:
>>> this blob loader will be used to load a specified
>>> blob into a specified RAM address.
>>>
>>
>> Suggested-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
>>
>>> Signed-off-by: Li Guang <lig.f...@cn.fujitsu.com>
>>> ---
>>> it can be used now for allwinner-a10, like:
>>> "-device blob-loader,addr=0x43000000,file=/path/script.bin"
>>>
>>> reference:
>>> http://linux-sunxi.org/Sunxi-tools
>>>
>>> script file address:
>>> http://dl.dbank.com/c00aonvlmw
>>>
>>> Thanks to Peter Crosthwaite for the idea!
>>> ---
>>> default-configs/arm-softmmu.mak | 2 +
>>> hw/misc/Makefile.objs | 2 +
>>> hw/misc/blob-loader.c | 75
>>> +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 79 insertions(+), 0 deletions(-)
>>> create mode 100644 hw/misc/blob-loader.c
>>>
>>> diff --git a/default-configs/arm-softmmu.mak
>>> b/default-configs/arm-softmmu.mak
>>> index ce1d620..50c71a6 100644
>>> --- a/default-configs/arm-softmmu.mak
>>> +++ b/default-configs/arm-softmmu.mak
>>> @@ -87,3 +87,5 @@ CONFIG_INTEGRATOR_DEBUG=y
>>> CONFIG_ALLWINNER_A10_PIT=y
>>> CONFIG_ALLWINNER_A10_PIC=y
>>> CONFIG_ALLWINNER_A10=y
>>> +
>>> +CONFIG_BLOB_LOADER=y
>>
>> This shouldn't be arm specific. I would make the argument that the
>> blob loader has global validity.
>>
>>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>> index f674365..df28288 100644
>>> --- a/hw/misc/Makefile.objs
>>> +++ b/hw/misc/Makefile.objs
>>> @@ -42,3 +42,5 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>>> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>>>
>>> obj-$(CONFIG_PVPANIC) += pvpanic.o
>>> +
>>> +obj-$(CONFIG_BLOB_LOADER) += blob-loader.o
>>> diff --git a/hw/misc/blob-loader.c b/hw/misc/blob-loader.c
>>> new file mode 100644
>>> index 0000000..d7f1408
>>> --- /dev/null
>>> +++ b/hw/misc/blob-loader.c
>>> @@ -0,0 +1,75 @@
>>> +#include "hw/sysbus.h"
>>> +#include "hw/devices.h"
>>> +#include "hw/loader.h"
>>> +#include "qemu/error-report.h"
>>> +
>>> +typedef struct BlobLoaderState {
>>
>> /*< private >/*
>>
>>> + DeviceState qdev;
>>
>> parent_obj;
>>
>> /*< public >/*
>>
>>> + uint32_t addr;
>>
>> uint64_t or hwaddr. Blob loading shouldn't be limited to 32 bit.
>>
>>> + char *file;
>>> +} BlobLoaderState;
>>> +
>>> +#define TYPE_BLOB_LOADER "blob-loader"
>>> +#define BLOB_LOADER(obj) OBJECT_CHECK(BlobLoaderState, (obj),
>>> TYPE_BLOB_LOADER)
>>> +
>>> +static Property blob_loader_props[] = {
>>> + DEFINE_PROP_UINT32("addr", BlobLoaderState, addr, 0),
>>> + DEFINE_PROP_STRING("file", BlobLoaderState, file),
>>> + DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void do_blob_load(BlobLoaderState *s)
>>> +{
>>> + char *file_name;
>>> + int file_size;
>>> +
>>> + if (s->file == NULL) {
>>> + error_report("please spicify a file for blob loader\n");
>>> + return;
>>
>> Should you exit(1)? Better yet, return true for error and ..
>>
>>> + }
>>> + file_name = qemu_find_file(QEMU_FILE_TYPE_BIOS, s->file);
>>> + if (file_name == NULL) {
>>> + error_report("can't find %s\n", s->file);
>>> + return;
>>> + }
>>> + file_size = get_image_size(file_name);
>>> + if (file_size < 0) {
>>> + error_report("can't get file size of %s\n", file_name);
>>> + return;
>>> + }
>>> + if (load_image_targphys(file_name, s->addr, file_size) < 0) {
>>> + error_report("can't load %s\n", file_name);
>>> + return;
>>> + }
>>> +}
>>> +
>>> +static int blob_loader_init(DeviceState *dev)
>>> +{
>>> + BlobLoaderState *s = BLOB_LOADER(dev);
>>> +
>>> + do_blob_load(s);
>>
>> exit(1) here.
>
> No, please use "realize" and avoid init. This way you can use an Error*
> to report the error.
>
> Also, the actual load_image_targphys call probably should be done in a
> reset handler, not at realize time.
>
Ok I think that settles it. The actual blobbing needs to happen at
reset time. Perhaps the correct approach is to do as much as possible
(file-path / address sanitsation etc) at realize time, then only the
actual blob load happens at reset. Going on what Paolo said, I think
for this device ::init is actually a nop.
> Paolo
>
>>> + return 0;
>>> +}
>>> +
>>> +static void blob_loader_class_init(ObjectClass *klass, void *data)
>>
>> s/klass/oc.
>
> Actually klass is quite commonly used.
>
Yeh, I remember being told off for that one though :)
Regards,
Peter
> Paolo
>
>>> +{
>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> + dc->props = blob_loader_props;
>>> + dc->desc = "blob loader";
>>> + dc->init = blob_loader_init;
>>
>> I'm wondering whether blob loading is actually a reset step not an
>> init. Will doing it at init will play foul with VMSD, as your blob
>> loader will trample non-reset state on machine restore?
>>
>> Regards,
>> Peter
>>
>>> +}
>>> +
>>> +static TypeInfo blob_loader_info = {
>>> + .name = TYPE_BLOB_LOADER,
>>> + .parent = TYPE_DEVICE,
>>> + .instance_size = sizeof(BlobLoaderState),
>>> + .class_init = blob_loader_class_init,
>>> +};
>>> +
>>> +static void blob_loader_register_type(void)
>>> +{
>>> + type_register_static(&blob_loader_info);
>>> +}
>>> +
>>> +type_init(blob_loader_register_type)
>>> --
>>> 1.7.2.5
>>>
>>>
>>
>>
>
>
