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

[mar.krzeminski]

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 <address@hidden> 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.