Re: [Qemu-devel] [PATCH] hw/misc/blob-loader: add a generic blob loader

[Peter Crosthwaite]

On Mon, Jan 6, 2014 at 3:24 PM, Li Guang <address@hidden> wrote:
> Li Guang wrote:
>>
>> Peter Crosthwaite wrote:
>>>
>>> On Mon, Jan 6, 2014 at 2:22 PM, Li Guang<address@hidden>  wrote:
>>>>
>>>> Peter Crosthwaite wrote:
>>>>>
>>>>> On Mon, Jan 6, 2014 at 1:55 PM, Li Guang<address@hidden>
>>>>> wrote:
>>>>>
>>>>>> Li Guang wrote:
>>>>>>
>>>>>>> Paolo Bonzini wrote:
>>>>>>>
>>>>>>>> Il 02/01/2014 11:51, Peter Crosthwaite ha scritto:
>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>> Yeah, also because init is in fact a legacy interface to realize.
>>>>>>>>
>>>>>>>> Paolo
>>>>>>>>
>>>>>>>>
>>>>>>> Ok, thanks!
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Sorry, seems load blob at reset handler can't do the right job,
>>>>>> while the same action can play very well at init or realize.
>>>>>>
>>>>> What's the exact problem with the reset idea?
>>>>>
>>>>>
>>>>>
>>>> code snippet:
>>>> static void blob_loader_reset(DeviceState *dev)
>>>> {
>>>>      BlobLoaderState *s = BLOB_LOADER(dev);
>>>>
>>>>      if (load_image_targphys(s->file, s->hwaddr, MAX_BLOB_SIZE)<  0) {
>>>>          error_report("can't load %s\n", s->file);
>>>>          exit(1);
>>>>      }
>>>> }
>>>>
>>>> if it is device reset handler, no result,
>>>> kernel doesn't find and parse blob,
>>>> if it is called at device realize phase,
>>>> it works.
>>>>
>>> Need to figure out why I think. There's no fundamental problem here
>>> AFAIK. It must be a bug somewhere.
>>>
>>>> Thanks!
>>>>
>>>>
>>>>>> any suggestion to figure out this problem?
>>>
>>> Run QEMU in GDB and break on your new reset function to see if it is
>>> ever called. If not have a look into QOM/qdev to see how resets work
>>> and how that plays with -device args. If yes, have a look into
>>> load_image_targphys and see why that's not working.
>>>
>>>
>>
>> reset handler definitely be called, and
>> load_image_targphys has no problem,
>> I guess it may be impacted by other RAM related codes,
>> just can't figure out it quickly.
>>
>
> Ok, I found the problem finally,
> load_image_targphys depends on rom_reset to take effect,
> if it's located at reset handler, rom_reset is called before it,
> it surely failed to do the right job.
>
> so, I have to write my own code to load blob into RAM.
>

dma_memory_write() work?

Regards,
Peter

> Thanks!
>
>