On 3/7/19 7:57 AM, Peter Maydell wrote:
diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 5f0ab2f..c697047 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -140,6 +140,7 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr ddr_base,
uint64_t entry, low, high;
uint32_t base32;
int big_endian = 0;
+ int kernel_space = 0;
#ifdef TARGET_WORDS_BIGENDIAN
big_endian = 1;
@@ -155,10 +156,12 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr ddr_base,
translate_kernel_address, NULL,
&entry, NULL, NULL,
big_endian, EM_ALTERA_NIOS2, 0, 0);
+ kernel_space = 1;
}
/* Always boot into physical ram. */
- boot_info.bootstrap_pc = ddr_base + 0xc0000000 + (entry & 0x07ffffff);
+ boot_info.bootstrap_pc = ddr_base + (kernel_space ? 0xc0000000 : 0)
+ + (entry & 0x07ffffff);
It's not clear to me what's going on here, or why an
entry address of 0xc000_0000 is special, but I don't
know anything about NiosII -- maybe it's clear if you do?
The processor reference guide documents that the kernel is placed at
virtual memory address 0xc0000000.
https://www.intel.com/content/www/us/en/programmable/documentation/iga1409256728501.html#iga1409332620358
The problem the patch to boot.c is trying to solve is that we found
things like unwind info were screwed up when using -kernel to load
executables with an entry address other than 0xc0000000.
Why isn't the bootstrap_pc just always the entry address ?
Some comments on what is being done here and the use cases
being addressed might assist. I wasn't able to work out what
the remarks in the commit message meant, I'm afraid.
Looking at the code, I don't think that the second call to
load_elf() will return a different entry address to the first
one (ie translate_kernel_address() is not applied to &entry).
That means that kernel_space is only true if entry == 0xc0000000,
and
(kernel_space ? 0xc0000000 : 0) + (entry & 0x07ffffff);
is almost the same thing as just 'entry'.
It seems like these remarks are directed more at the existing code than
the patch.... :-S TBH, I don't know why it was done that way originally.
+static void nios2_generic_nommu_init(MachineState *machine)
+{
+ Nios2CPU *cpu;
+ DeviceState *dev;
+ MemoryRegion *address_space_mem = get_system_memory();
+ MemoryRegion *phys_tcm = g_new(MemoryRegion, 1);
+ MemoryRegion *phys_tcm_alias = g_new(MemoryRegion, 1);
+ MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
+ MemoryRegion *phys_ram_alias = g_new(MemoryRegion, 1);
+ ram_addr_t tcm_base = 0x0;
+ ram_addr_t tcm_size = 0x1000; /* 1 kiB, but QEMU limit is 4 kiB */
+ ram_addr_t ram_base = 0x10000000;
+ ram_addr_t ram_size = 0x08000000;
+ qemu_irq *cpu_irq, irq[32];
+ int i;
The description says this is "generic", but it appears to
be almost identical to the existing 10M50 board model,
including having exactly the same devices at the same
apparently arbitrary addresses.
Could we instead add a machine parameter to the existing
board so you could say "-machine 10m50-ghrd,mmu=no"
(and drop the other changes -- it's not clear what they're
needed for) ?
If it really ought to be a separate board model, perhaps
it should be in the same source file and share the common
code.
I didn't write this code, but the intent was actually to allow
executables linked for the 3c120 devboards we'd been using for hardware
testing to run in this emulation; not to emulate a mmu-less 10m50 board.
The BSP that we contributed to libgloss locates the reset vector, etc
appropriately for this emulation.
I can't comment on whether the peripherals copied from the 10m50
emulation are actually necessary or useful for anything; we certainly
don't intend to support manipulating them from the program being loaded,
but maybe other parts of QEMU expect certain devices to be present (I've
seen that on other targets like ARM). Andrew, do you have any state on
this left?
-Sandra