On Sun, Sep 27, 2015 at 8:38 PM, Peter Crosthwaite
<crosthwaitepe...@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvb...@gmail.com> wrote:
>> Cores with and without MMU have system RAM and ROM at different locations.
>> Also with noMMU cores system IO region is accessible through two physical
>> address ranges.
>>
>> Signed-off-by: Max Filippov <jcmvb...@gmail.com>
>> ---
>> hw/xtensa/xtfpga.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 41 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>> index d4b9afb..b53f40d 100644
>> --- a/hw/xtensa/xtfpga.c
>> +++ b/hw/xtensa/xtfpga.c
>> @@ -199,7 +199,29 @@ static void lx_init(const LxBoardDesc *board,
>> MachineState *machine)
>> const char *kernel_cmdline = qemu_opt_get(machine_opts, "append");
>> const char *dtb_filename = qemu_opt_get(machine_opts, "dtb");
>> const char *initrd_filename = qemu_opt_get(machine_opts, "initrd");
>> + const unsigned system_io_size = 224 * 1024 * 1024;
>> + bool mmu;
>
> You are indexing into an array of configs so it's really an int (or
> better, an enum).
Ok.
>> int n;
>
> Blank line.
Why?
>> + static const struct {
>> + hwaddr ram;
>> + hwaddr rom;
>> + hwaddr io[2];
>> + } base[2] = {
>> + {
>> + .ram = 0x60000000,
>> + .rom = 0x50000000,
>> + .io = {
>> + 0x70000000,
>> + 0x90000000,
>> + },
>> + }, {
>> + .ram = 0,
>> + .rom = 0xfe000000,
>> + .io = {
>> + 0xf0000000,
>> + },
>> + }
>> + };
>>
>> if (!cpu_model) {
>> cpu_model = XTENSA_DEFAULT_CPU_MODEL;
>> @@ -222,16 +244,24 @@ static void lx_init(const LxBoardDesc *board,
>> MachineState *machine)
>> cpu_reset(CPU(cpu));
>> }
>>
>> + mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
>
> This looks backwards, the board should be in charge of itself and the
> CPU config, rather than spying on the CPU setup to rewire the board.
Well, it's an FPGA board and all connections are a part of bitstream.
It's generated that way, I'm just following the specification here.
>> ram = g_malloc(sizeof(*ram));
>> memory_region_init_ram(ram, NULL, "lx60.dram", machine->ram_size,
>> &error_fatal);
>> vmstate_register_ram_global(ram);
>> - memory_region_add_subregion(system_memory, 0, ram);
>> + memory_region_add_subregion(system_memory, base[mmu].ram, ram);
>>
>> system_io = g_malloc(sizeof(*system_io));
>> memory_region_init_io(system_io, NULL, &lx60_io_ops, NULL, "lx60.io",
>> - 224 * 1024 * 1024);
>> - memory_region_add_subregion(system_memory, 0xf0000000, system_io);
>> + system_io_size);
>> + memory_region_add_subregion(system_memory, base[mmu].io[0], system_io);
>> + if (!mmu) {
>
> The boolean switch for whether the alias exists could go in the
> struct. That makes it more robust to add yet more configs in the
> future rather than iffery on the config index.
Ok.
--
Thanks.
-- Max
