Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
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 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 ! :)
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 to have a ROM, we will need to pass a second
time 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.
Are there any other platform with similar need ?
Thanks,
C.
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
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 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 ;] 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. Thanks, Marcin Thanks, C.
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
On 09/23/2016 10:17 AM, Peter Maydell wrote: > On 23 September 2016 at 08:19, Cédric Le Goater 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. Thanks, C.
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
On 23 September 2016 at 08:19, Cédric Le Goater 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. thanks -- PMM
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
Hello,
On 07/04/2016 07:57 PM, mar.krzeminski wrote:
>
>
> W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze:
>> Some SPI controllers, such as the Aspeed AST2400, have a mode in which
>> accesses to the flash content are no different than doing MMIOs. The
>> controller generates all the necessary commands to load (or store)
>> data in memory.
>>
>> To emulate such a behavior, we need to have access to the underlying
>> storage of the flash object. The purpose of the routine is to set the
>> storage of a preinitialized SPI flash object, which can then be used
>> by the object itself but also by a SPI controller to handled the
>> MMIOs.
> Hi,
>
> I am not sure if this approach is correct. I can not find any datasheet
> to this SPI controller, but as you mentioned in first paragraph, controller
> generates all commands (probably ones are somehow configurable).
> In this series you hack this behaviour and you do direct access to file.
> IMHO you should emulate sending such commands in SPI controller
> model.
I took some time to implement what you proposed, that is to emulate all
the SPI commands needed to handle the read access. This is easily tested
through the monitor. Looks good.
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.
It seems that we do need a ROM. This is how firmwares are handled, with
a load_image_targphys(), and also the pflash devices, through a ROM device
region.
The proposal below allows to modify the backing region of the flash device
model for this purpose. A ROM device region can be used to enable booting
from the device. It also allows some shortcuts in the model, like reading
directly from the storage for the benefit of speed. This is what Peter
Crosthwaite mentioned in a previous email. But this is not the primary
use. Being able to use a ROM memory region is.
Shall I resend ? or are there strong objections ?
Thanks,
C.
> Thanks,
> Marcin
>
>> This is sufficient to support read only accesses. For writes, we would
>> certainly need to externalize flash_write8() routine.
>>
>> Signed-off-by: Cédric Le Goater
>> ---
>> hw/block/m25p80.c| 14 +-
>> include/hw/block/flash.h | 2 ++
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index aa964280beab..fec0fd4c2228 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -29,6 +29,7 @@
>> #include "qemu/bitops.h"
>> #include "qemu/log.h"
>> #include "qapi/error.h"
>> +#include "hw/block/flash.h"
>> #ifndef M25P80_ERR_DEBUG
>> #define M25P80_ERR_DEBUG 0
>> @@ -1152,7 +1153,11 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
>> if (s->blk) {
>> DB_PRINT_L(0, "Binding to IF_MTD drive\n");
>> -s->storage = blk_blockalign(s->blk, s->size);
>> +
>> +/* using an external storage. see m25p80_create_rom() */
>> +if (!s->storage) {
>> +s->storage = blk_blockalign(s->blk, s->size);
>> +}
>> if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
>> error_setg(errp, "failed to read the initial flash content");
>> @@ -1259,3 +1264,10 @@ static void m25p80_register_types(void)
>> }
>> type_init(m25p80_register_types)
>> +
>> +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom)
>> +{
>> +Flash *s = M25P80(dev);
>> +
>> +s->storage = memory_region_get_ram_ptr(rom);
>> +}
>> diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
>> index 50ccbbcf1352..9d780f4ae0c9 100644
>> --- a/include/hw/block/flash.h
>> +++ b/include/hw/block/flash.h
>> @@ -61,4 +61,6 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
>> void ecc_reset(ECCState *s);
>> extern VMStateDescription vmstate_ecc_state;
>> +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom);
>> +
>> #endif
>
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
Hello,
On 07/10/2016 01:38 AM, Peter Crosthwaite wrote:
> On Mon, Jul 4, 2016 at 10:57 AM, mar.krzeminski
> wrote:
>>
>>
>> W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze:
>>>
>>> Some SPI controllers, such as the Aspeed AST2400, have a mode in which
>>> accesses to the flash content are no different than doing MMIOs. The
>>> controller generates all the necessary commands to load (or store)
>>> data in memory.
>>>
>>> To emulate such a behavior, we need to have access to the underlying
>>> storage of the flash object. The purpose of the routine is to set the
>>> storage of a preinitialized SPI flash object, which can then be used
>>> by the object itself but also by a SPI controller to handled the
>>> MMIOs.
>>
>> Hi,
>>
>> I am not sure if this approach is correct. I can not find any datasheet
>> to this SPI controller, but as you mentioned in first paragraph, controller
>> generates all commands (probably ones are somehow configurable).
>> In this series you hack this behaviour and you do direct access to file.
>> IMHO you should emulate sending such commands in SPI controller
>> model.
>>
>
> Actually I think this is a good approach for two reasons.
>
> Firstly there is more than one SPI controller that does this, the
> Xilinx Zynq QSPI controller does this for its LQSPI mode. See
> lqspi_read() in hw/ssi/xilinx_spips.c, which already works the way
> Marcin proposes. That one is semi-automatic, in that there are
> software configurable registers that hold the actual command to do the
> read etc. But once setup, the interface is linear-read-only and
> software transparent. A lot of assumptions where made in writing that
> code (the buffering scheme is completely made up) and I think the
> direct approach might be cleaner. This approach can go beyond SPI, in
> that it can work for other protocols and external storage devices.
ok.
The Aspeed SPI controller could follow the SPI approach and initiate
SPI tranfers instead of accessing the underlying storage of the flash
module. This is not strictly necessary for that case.
But, being able to use the SPI flash module as a boot ROM (as you point
out below) brings in extra needs. A rom memory region is required for
that as you need to share the storage with the flash object. Sharing
gives you direct access to the storage and then, generating SPI transfers
becomes a bit superfluous.
I tried a few other approaches, adding aliases, adding a region without
ops behind the flash object but they were not very convincing.
> The more interesting application of this approach though, is emulating
> boot ROMs. Multiple SoCs in tree have pty bootroms that read SD cards
> (or even SPI), mount file-systems and then blob+boot images. Rather
> than emulators having to talk SDHCI through the controller to get
> images, we should be able to go direct, and implement the boot-logic
> off the raw block device. This means that there should be a general
> approach to a machine fishing the backing block-dev out from an
> external storage device, and linear SPI is another good application if
> it.
So if I read you well, the m25p80_set_rom_storage() approach is not the
most hideous thing to do but it needs to be generalized.
Currently, it boils down to :
* create a rom memory region for each flash module needing one :
memory_region_init_rom_device(&fl->mmio, OBJECT(s), &flash_ops,
fl, name, fl->size, &err);
* if needed, storage can be saved for later usage with :
flash->storage = memory_region_get_ram_ptr(&fl->mmio);
* the memory region storage should be passed on to the flash object
using m25p80_set_rom_storage() before realization of the object :
fl->flash = ssi_create_slave_no_init(s->spi, flashtype);
if (dinfo) {
qdev_prop_set_drive(fl->flash, "drive", blk_by_legacy_dinfo(dinfo),
errp);
}
m25p80_set_rom_storage(fl->flash, &fl->mmio);
qdev_init_nofail(fl->flash);
This is very m25p80 centric. A common solution could be an object
gathering the properties of a memory region rom device and a blk
device. This is a bit what the pflash_cfi* objects are proposing
but it is all bundled in a big specific object.
Thanks,
C.
> Regards,
> Peter
>
>> Thanks,
>> Marcin
>>
>>
>>> This is sufficient to support read only accesses. For writes, we would
>>> certainly need to externalize flash_write8() routine.
>>>
>>> Signed-off-by: Cédric Le Goater
>>> ---
>>> hw/block/m25p80.c| 14 +-
>>> include/hw/block/flash.h | 2 ++
>>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>>> index aa964280beab..fec0fd4c2228 100644
>>> --- a/hw/block/m25p80.c
>>> +++ b/hw/block/m25p80.c
>>> @@ -29,6 +29,7 @@
>>> #include "qemu/bitops.h"
>>> #include "qemu/log.h"
>>> #include "qapi/error.h"
>>> +#include "hw/block/flash.h"
>>> #ifndef M25P
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
On Mon, Jul 4, 2016 at 10:57 AM, mar.krzeminski
wrote:
>
>
> W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze:
>>
>> Some SPI controllers, such as the Aspeed AST2400, have a mode in which
>> accesses to the flash content are no different than doing MMIOs. The
>> controller generates all the necessary commands to load (or store)
>> data in memory.
>>
>> To emulate such a behavior, we need to have access to the underlying
>> storage of the flash object. The purpose of the routine is to set the
>> storage of a preinitialized SPI flash object, which can then be used
>> by the object itself but also by a SPI controller to handled the
>> MMIOs.
>
> Hi,
>
> I am not sure if this approach is correct. I can not find any datasheet
> to this SPI controller, but as you mentioned in first paragraph, controller
> generates all commands (probably ones are somehow configurable).
> In this series you hack this behaviour and you do direct access to file.
> IMHO you should emulate sending such commands in SPI controller
> model.
>
Actually I think this is a good approach for two reasons.
Firstly there is more than one SPI controller that does this, the
Xilinx Zynq QSPI controller does this for its LQSPI mode. See
lqspi_read() in hw/ssi/xilinx_spips.c, which already works the way
Marcin proposes. That one is semi-automatic, in that there are
software configurable registers that hold the actual command to do the
read etc. But once setup, the interface is linear-read-only and
software transparent. A lot of assumptions where made in writing that
code (the buffering scheme is completely made up) and I think the
direct approach might be cleaner. This approach can go beyond SPI, in
that it can work for other protocols and external storage devices.
The more interesting application of this approach though, is emulating
boot ROMs. Multiple SoCs in tree have pty bootroms that read SD cards
(or even SPI), mount file-systems and then blob+boot images. Rather
than emulators having to talk SDHCI through the controller to get
images, we should be able to go direct, and implement the boot-logic
off the raw block device. This means that there should be a general
approach to a machine fishing the backing block-dev out from an
external storage device, and linear SPI is another good application if
it.
Regards,
Peter
> Thanks,
> Marcin
>
>
>> This is sufficient to support read only accesses. For writes, we would
>> certainly need to externalize flash_write8() routine.
>>
>> Signed-off-by: Cédric Le Goater
>> ---
>> hw/block/m25p80.c| 14 +-
>> include/hw/block/flash.h | 2 ++
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index aa964280beab..fec0fd4c2228 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -29,6 +29,7 @@
>> #include "qemu/bitops.h"
>> #include "qemu/log.h"
>> #include "qapi/error.h"
>> +#include "hw/block/flash.h"
>> #ifndef M25P80_ERR_DEBUG
>> #define M25P80_ERR_DEBUG 0
>> @@ -1152,7 +1153,11 @@ static void m25p80_realize(SSISlave *ss, Error
>> **errp)
>> if (s->blk) {
>> DB_PRINT_L(0, "Binding to IF_MTD drive\n");
>> -s->storage = blk_blockalign(s->blk, s->size);
>> +
>> +/* using an external storage. see m25p80_create_rom() */
>> +if (!s->storage) {
>> +s->storage = blk_blockalign(s->blk, s->size);
>> +}
>> if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
>> error_setg(errp, "failed to read the initial flash
>> content");
>> @@ -1259,3 +1264,10 @@ static void m25p80_register_types(void)
>> }
>> type_init(m25p80_register_types)
>> +
>> +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom)
>> +{
>> +Flash *s = M25P80(dev);
>> +
>> +s->storage = memory_region_get_ram_ptr(rom);
>> +}
>> diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
>> index 50ccbbcf1352..9d780f4ae0c9 100644
>> --- a/include/hw/block/flash.h
>> +++ b/include/hw/block/flash.h
>> @@ -61,4 +61,6 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
>> void ecc_reset(ECCState *s);
>> extern VMStateDescription vmstate_ecc_state;
>> +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom);
>> +
>> #endif
>
>
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
W dniu 06.07.2016 o 18:30, Cédric Le Goater pisze: On 07/04/2016 08:12 PM, Cédric Le Goater wrote: On 07/04/2016 07:57 PM, mar.krzeminski wrote: W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze: Some SPI controllers, such as the Aspeed AST2400, have a mode in which accesses to the flash content are no different than doing MMIOs. The controller generates all the necessary commands to load (or store) data in memory. To emulate such a behavior, we need to have access to the underlying storage of the flash object. The purpose of the routine is to set the storage of a preinitialized SPI flash object, which can then be used by the object itself but also by a SPI controller to handled the MMIOs. Hi, I am not sure if this approach is correct. I can not find any datasheet to this SPI controller, but as you mentioned in first paragraph, controller generates all commands (probably ones are somehow configurable). yes. see this patch : http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg08229.html /* CEx Control Register */ #define R_CTRL0 (0x10 / 4) #define CTRL_CMD_SHIFT 16 #define CTRL_CMD_MASK0xff In this series you hack this behaviour and you do direct access to file. It is true it is not very respectful of the m25p80 interface. IMHO you should emulate sending such commands in SPI controller model. I will give it a try. I don't think the alternative is a complex change anyhow. So yes, that would work out pretty well. But there is another need that does not show up in the series (I am not splitting the patches correctly I guess) We would like to boot directly from a flash image. For this purpose, the pflash_cfi object uses a memory region of type rom and uses the storage behind the region. m25p80_set_rom_storage() is probably not the right API to share the storage. I am looking for a way to handle this without changing too much m25p80, which does not have a mem region currently. Suggestions welcomed ! :) I do not know if this is a good idea - for sure yet another complication, but we still should process like the HW. The flash is memory mapped to some memory area, so maybe creating alias to this memory area would help with the implementation? Then read will be still done by SPI and m25p80. Some configuration from the board level might be needed to initialise the controller (or maybe defaults are ok - depends how it is done in hw). Regards, Marcin Thanks, C.
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
On 07/04/2016 08:12 PM, Cédric Le Goater wrote: > On 07/04/2016 07:57 PM, mar.krzeminski wrote: >> >> >> W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze: >>> Some SPI controllers, such as the Aspeed AST2400, have a mode in which >>> accesses to the flash content are no different than doing MMIOs. The >>> controller generates all the necessary commands to load (or store) >>> data in memory. >>> >>> To emulate such a behavior, we need to have access to the underlying >>> storage of the flash object. The purpose of the routine is to set the >>> storage of a preinitialized SPI flash object, which can then be used >>> by the object itself but also by a SPI controller to handled the >>> MMIOs. >> Hi, >> >> I am not sure if this approach is correct. I can not find any datasheet >> to this SPI controller, but as you mentioned in first paragraph, controller >> generates all commands (probably ones are somehow configurable). > > yes. see this patch : > > http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg08229.html > > /* CEx Control Register */ > #define R_CTRL0 (0x10 / 4) > #define CTRL_CMD_SHIFT 16 > #define CTRL_CMD_MASK0xff > > >> In this series you hack this behaviour and you do direct access to file. > > It is true it is not very respectful of the m25p80 interface. > >> IMHO you should emulate sending such commands in SPI controller >> model. > > I will give it a try. I don't think the alternative is a complex > change anyhow. So yes, that would work out pretty well. But there is another need that does not show up in the series (I am not splitting the patches correctly I guess) We would like to boot directly from a flash image. For this purpose, the pflash_cfi object uses a memory region of type rom and uses the storage behind the region. m25p80_set_rom_storage() is probably not the right API to share the storage. I am looking for a way to handle this without changing too much m25p80, which does not have a mem region currently. Suggestions welcomed ! :) Thanks, C.
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
W dniu 04.07.2016 o 20:12, Cédric Le Goater pisze:
On 07/04/2016 07:57 PM, mar.krzeminski wrote:
W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze:
Some SPI controllers, such as the Aspeed AST2400, have a mode in which
accesses to the flash content are no different than doing MMIOs. The
controller generates all the necessary commands to load (or store)
data in memory.
To emulate such a behavior, we need to have access to the underlying
storage of the flash object. The purpose of the routine is to set the
storage of a preinitialized SPI flash object, which can then be used
by the object itself but also by a SPI controller to handled the
MMIOs.
Hi,
I am not sure if this approach is correct. I can not find any datasheet
to this SPI controller, but as you mentioned in first paragraph, controller
generates all commands (probably ones are somehow configurable).
yes. see this patch :
http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg08229.html
/* CEx Control Register */
#define R_CTRL0 (0x10 / 4)
#define CTRL_CMD_SHIFT 16
#define CTRL_CMD_MASK0xff
In this series you hack this behaviour and you do direct access to file.
It is true it is not very respectful of the m25p80 interface.
IMHO you should emulate sending such commands in SPI controller
model.
I will give it a try. I don't think the alternative is a complex
change anyhow.
Register logic + few commands, then write is almost copy-paste.
Shouldn’t be complicated, but you have real HW modelling.
Thanks,
Marcin
Thanks,
C.
Thanks,
Marcin
This is sufficient to support read only accesses. For writes, we would
certainly need to externalize flash_write8() routine.
Signed-off-by: Cédric Le Goater
---
hw/block/m25p80.c| 14 +-
include/hw/block/flash.h | 2 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index aa964280beab..fec0fd4c2228 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -29,6 +29,7 @@
#include "qemu/bitops.h"
#include "qemu/log.h"
#include "qapi/error.h"
+#include "hw/block/flash.h"
#ifndef M25P80_ERR_DEBUG
#define M25P80_ERR_DEBUG 0
@@ -1152,7 +1153,11 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
if (s->blk) {
DB_PRINT_L(0, "Binding to IF_MTD drive\n");
-s->storage = blk_blockalign(s->blk, s->size);
+
+/* using an external storage. see m25p80_create_rom() */
+if (!s->storage) {
+s->storage = blk_blockalign(s->blk, s->size);
+}
if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
error_setg(errp, "failed to read the initial flash content");
@@ -1259,3 +1264,10 @@ static void m25p80_register_types(void)
}
type_init(m25p80_register_types)
+
+void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom)
+{
+Flash *s = M25P80(dev);
+
+s->storage = memory_region_get_ram_ptr(rom);
+}
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 50ccbbcf1352..9d780f4ae0c9 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -61,4 +61,6 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
void ecc_reset(ECCState *s);
extern VMStateDescription vmstate_ecc_state;
+void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom);
+
#endif
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
On 07/04/2016 07:57 PM, mar.krzeminski wrote:
>
>
> W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze:
>> Some SPI controllers, such as the Aspeed AST2400, have a mode in which
>> accesses to the flash content are no different than doing MMIOs. The
>> controller generates all the necessary commands to load (or store)
>> data in memory.
>>
>> To emulate such a behavior, we need to have access to the underlying
>> storage of the flash object. The purpose of the routine is to set the
>> storage of a preinitialized SPI flash object, which can then be used
>> by the object itself but also by a SPI controller to handled the
>> MMIOs.
> Hi,
>
> I am not sure if this approach is correct. I can not find any datasheet
> to this SPI controller, but as you mentioned in first paragraph, controller
> generates all commands (probably ones are somehow configurable).
yes. see this patch :
http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg08229.html
/* CEx Control Register */
#define R_CTRL0 (0x10 / 4)
#define CTRL_CMD_SHIFT 16
#define CTRL_CMD_MASK0xff
> In this series you hack this behaviour and you do direct access to file.
It is true it is not very respectful of the m25p80 interface.
> IMHO you should emulate sending such commands in SPI controller
> model.
I will give it a try. I don't think the alternative is a complex
change anyhow.
Thanks,
C.
> Thanks,
> Marcin
>
>> This is sufficient to support read only accesses. For writes, we would
>> certainly need to externalize flash_write8() routine.
>>
>> Signed-off-by: Cédric Le Goater
>> ---
>> hw/block/m25p80.c| 14 +-
>> include/hw/block/flash.h | 2 ++
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index aa964280beab..fec0fd4c2228 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -29,6 +29,7 @@
>> #include "qemu/bitops.h"
>> #include "qemu/log.h"
>> #include "qapi/error.h"
>> +#include "hw/block/flash.h"
>> #ifndef M25P80_ERR_DEBUG
>> #define M25P80_ERR_DEBUG 0
>> @@ -1152,7 +1153,11 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
>> if (s->blk) {
>> DB_PRINT_L(0, "Binding to IF_MTD drive\n");
>> -s->storage = blk_blockalign(s->blk, s->size);
>> +
>> +/* using an external storage. see m25p80_create_rom() */
>> +if (!s->storage) {
>> +s->storage = blk_blockalign(s->blk, s->size);
>> +}
>> if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
>> error_setg(errp, "failed to read the initial flash content");
>> @@ -1259,3 +1264,10 @@ static void m25p80_register_types(void)
>> }
>> type_init(m25p80_register_types)
>> +
>> +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom)
>> +{
>> +Flash *s = M25P80(dev);
>> +
>> +s->storage = memory_region_get_ram_ptr(rom);
>> +}
>> diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
>> index 50ccbbcf1352..9d780f4ae0c9 100644
>> --- a/include/hw/block/flash.h
>> +++ b/include/hw/block/flash.h
>> @@ -61,4 +61,6 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
>> void ecc_reset(ECCState *s);
>> extern VMStateDescription vmstate_ecc_state;
>> +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom);
>> +
>> #endif
>
Re: [Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze:
Some SPI controllers, such as the Aspeed AST2400, have a mode in which
accesses to the flash content are no different than doing MMIOs. The
controller generates all the necessary commands to load (or store)
data in memory.
To emulate such a behavior, we need to have access to the underlying
storage of the flash object. The purpose of the routine is to set the
storage of a preinitialized SPI flash object, which can then be used
by the object itself but also by a SPI controller to handled the
MMIOs.
Hi,
I am not sure if this approach is correct. I can not find any datasheet
to this SPI controller, but as you mentioned in first paragraph, controller
generates all commands (probably ones are somehow configurable).
In this series you hack this behaviour and you do direct access to file.
IMHO you should emulate sending such commands in SPI controller
model.
Thanks,
Marcin
This is sufficient to support read only accesses. For writes, we would
certainly need to externalize flash_write8() routine.
Signed-off-by: Cédric Le Goater
---
hw/block/m25p80.c| 14 +-
include/hw/block/flash.h | 2 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index aa964280beab..fec0fd4c2228 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -29,6 +29,7 @@
#include "qemu/bitops.h"
#include "qemu/log.h"
#include "qapi/error.h"
+#include "hw/block/flash.h"
#ifndef M25P80_ERR_DEBUG
#define M25P80_ERR_DEBUG 0
@@ -1152,7 +1153,11 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
if (s->blk) {
DB_PRINT_L(0, "Binding to IF_MTD drive\n");
-s->storage = blk_blockalign(s->blk, s->size);
+
+/* using an external storage. see m25p80_create_rom() */
+if (!s->storage) {
+s->storage = blk_blockalign(s->blk, s->size);
+}
if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
error_setg(errp, "failed to read the initial flash content");
@@ -1259,3 +1264,10 @@ static void m25p80_register_types(void)
}
type_init(m25p80_register_types)
+
+void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom)
+{
+Flash *s = M25P80(dev);
+
+s->storage = memory_region_get_ram_ptr(rom);
+}
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 50ccbbcf1352..9d780f4ae0c9 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -61,4 +61,6 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
void ecc_reset(ECCState *s);
extern VMStateDescription vmstate_ecc_state;
+void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom);
+
#endif
[Qemu-devel] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine
Some SPI controllers, such as the Aspeed AST2400, have a mode in which
accesses to the flash content are no different than doing MMIOs. The
controller generates all the necessary commands to load (or store)
data in memory.
To emulate such a behavior, we need to have access to the underlying
storage of the flash object. The purpose of the routine is to set the
storage of a preinitialized SPI flash object, which can then be used
by the object itself but also by a SPI controller to handled the
MMIOs.
This is sufficient to support read only accesses. For writes, we would
certainly need to externalize flash_write8() routine.
Signed-off-by: Cédric Le Goater
---
hw/block/m25p80.c| 14 +-
include/hw/block/flash.h | 2 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index aa964280beab..fec0fd4c2228 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -29,6 +29,7 @@
#include "qemu/bitops.h"
#include "qemu/log.h"
#include "qapi/error.h"
+#include "hw/block/flash.h"
#ifndef M25P80_ERR_DEBUG
#define M25P80_ERR_DEBUG 0
@@ -1152,7 +1153,11 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
if (s->blk) {
DB_PRINT_L(0, "Binding to IF_MTD drive\n");
-s->storage = blk_blockalign(s->blk, s->size);
+
+/* using an external storage. see m25p80_create_rom() */
+if (!s->storage) {
+s->storage = blk_blockalign(s->blk, s->size);
+}
if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
error_setg(errp, "failed to read the initial flash content");
@@ -1259,3 +1264,10 @@ static void m25p80_register_types(void)
}
type_init(m25p80_register_types)
+
+void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom)
+{
+Flash *s = M25P80(dev);
+
+s->storage = memory_region_get_ram_ptr(rom);
+}
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 50ccbbcf1352..9d780f4ae0c9 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -61,4 +61,6 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
void ecc_reset(ECCState *s);
extern VMStateDescription vmstate_ecc_state;
+void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom);
+
#endif
--
2.1.4
