Re: [Qemu-arm] [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine

["mar.krzeminski" <mar.krzeminski@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6108 bytes --]



W dniu 26.09.2016 o 10:56, Cédric Le Goater pisze:
> On 09/26/2016 10:25 AM, KONRAD Frederic wrote:
>>
>> Le 24/09/2016 à 10:55, Edgar E. Iglesias a écrit :
>>> On Sat, Sep 24, 2016 at 10:25:39AM +0200, Cédric Le Goater wrote:
>>>> On 09/23/2016 08:26 PM, mar.krzeminski wrote:
>>>>> Hi Cedric,
>>>>>
>>>>> W dniu 23.09.2016 o 10:28, Cédric Le Goater pisze:
>>>>>> On 09/23/2016 10:17 AM, Peter Maydell wrote:
>>>>>>> On 23 September 2016 at 08:19, Cédric Le Goater <clg@kaod.org> wrote:
>>>>>>>> But the goal is to boot from the device, so I added a memory region alias
>>>>>>>> at 0 to trigger the flash module mmios at boot time, as this is where
>>>>>>>> u-boot expects to be.
>>>>>>>>
>>>>>>>> and I fell in this trap :/
>>>>>>>>
>>>>>>>>           aspeed_smc_flash_read: To 0x0 of size 1: 0xbe mode:0
>>>>>>>>           Bad ram pointer (nil)
>>>>>>>>           Aborted (core dumped)
>>>>>>>>
>>>>>>>> There is a failure in get_page_addr_code(), possibly because qemu uses
>>>>>>>> byte per byte reads of the code (cpu_ldub_code). But this is beyond my
>>>>>>>> understanding of qemu's internal.
>>>>>>> This is a bug in how we report the problem, but the underlying
>>>>>>> issue here is attempting to execute from something that's not RAM
>>>>>>> or ROM. You can't execute code out of something backed by MMIO.
>>>>>> OK. So I see two solutions. T
>>>>>>
>>>>>> The "brutal" one which is to copy the flash contents in a rom blob
>>>>>> at 0, but there is still an issue in getting access to the storage
>>>>>> anyhow, as it is internal to m25p80. Or we should get the name of the
>>>>>> backing file of the drive but I am not sure we are expected to do
>>>>>> that as I don't see any API for it.
>>>>>>
>>>>>> The other solution is something like this patch which lets the storage
>>>>>> of the flash device be assigned externally.
>>>>> Since I do not like dirty hacks in the code, I want just to suggest a
>>>>> workaround, that probably you will not like ;]
>>>> It's a feature ! :)
>>> CC: Mark and Fred
>>>
>>> I agree, I think these are fair attempts to try to solve a real problem.
>>>
>>>
>>>> I am just trying to find a solution to this issue. So, we could also
>>>> introduce a routine :
>>>>
>>>> +uint8_t *m25p80_get_storage(DeviceState *dev, uint32_t *size)
>>>> +{
>>>> +    Flash *s = M25P80(dev);
>>>> +
>>>> +    *size = s->size;
>>>> +    return s->storage;
>>>> +}
>>>>
>>>> and use rom_add_blob_fixed() with the return values. Maybe this is less
>>>> intrusive in the m25p80 device model flow. Thoughts ?
>>>>
>>>>> As Qemu expects that first running code will be in ROM or RAM memory,
>>>>> you can implement in your board -bios option that you will use to
>>>>> pass u-boot binary to rom memory, or even use generic loader functionality
>>>>> when it reach master.
>>>> but if we use -bios <file> to have a ROM, we will need to pass a second
>>>> time <file> as a drive to have a CS0 flash slave:
>>>>
>>>>      -bios "flash-palmetto-test"  \
>>>>      -drive file="flash-palmetto-test",format=raw,if=mtd \
>>>>      -drive file="palmetto.pnor",format=raw,if=mtd
>>>>
>>>> This feels awkward. The virt platform and vexpress forbid that for
>>>> instance.
Just to clarify, I mean a use of -bios option or generic loader just to 
allow this board to boot from u-boot
by directly potting u-boot in the memory where flash should be mapped.
I will not solve memory mapped flash problem at all..
>>>>
>>>> Are there any other platform with similar need ?
>>> Yes, ZynqMP has a linear memory mapping of SPI memory contents. It gets
>>> slightly more complicated there as the mapping supports various modes
>>> including parallel SPIs with striping done by the controller (i.e
>>> bytes as seen via the linearly mapped area are interleaved from multiple
>>> SPI memories).
>>>
>>> So a plain RAM mapping of m25p80_get_storage won't work.
>>>
>>> A ROM version of the linear data might work at controller level. The controller
>>> would have to populate the ROM area at startup (slow) and keep it in sync for all
>>> writes. We would also need to signal write events so that TCG can flush it's
>>> caches. TCG only keeps track of modifications being made to cached code if changes
>>> are done with writes to RAM area (not if they happen indirectly via other registers).
I was trying that in a very simple way, I stopped because I used 128MiB 
Flash (memory consumption
and speed).
>>>
>>> Another solution is to implement support in TCG to execute from MMIO mapped areas.
>>> We'd also need to solve the problem of signalling content changes to TCG.
>>>
>>> I think these approaches have some overlap. Personally, I think TCG executing from
>>> MMIO areas would be quite useful. It's also useful for QEMU & SystemC integration.
I can agree here, this could solve this problem, and allow to do more.
>> Hi,
>>
>> The solution we are preparing for this is to allow a MMIO region to execute code
>> as Edgar just suggested by adding two callbacks:
> Yes it seems the best option to boot.
>
> The SPI controller I am working on maintains a set of mapping windows for
> each flash slave. But it's up to software to make sure the window size and
> the flash size are in sync. So, in a qemu world, we will need some flexibility
> if we want to access directly the flash storage. I don't think this patch is
> the right approach though. Let's boot first.
I believe we agree here. Exposing underlying file is to big shortcut,
from the other hand it will allow to boot from flash.
>
>>    - one which allows to get a pointer from an MMIO region nothing if
>>      execution is not possible from it and some hint to know if it's
>>      readable, writable.
>>    - one function to "invalidate" the pointer which will flush the tb,
>>      etc.
> I don't know enough about TCG to comment, I am very willing to give it
> a try when ever you have something ready.
I am also interested in testing that.

Thanks,
Marcin
> Thanks,
>
> C.
>
>> We have fixed a second issue fairly linked to this: if we do a DMA access to the
>> MMIO areas it is split in 1,2,4 bytes transaction which is really slow.
>


[-- Attachment #2: Type: text/html, Size: 8885 bytes --]