| Message ID | 20180703163446.9943-1-alistair.francis@wdc.com |
|---|---|
| Headers | show |
| Series | riscv-pull queue | expand |
On 3 July 2018 at 17:34, Alistair Francis <alistair.francis@wdc.com> wrote: > The following changes since commit f988c7e191141e92de2059d04a5f9a9bb01f399c: > > Merge remote-tracking branch 'remotes/shorne/tags/pull-or-20180703' into staging (2018-07-03 16:04:41 +0100) > > are available in the Git repository at: > > git@github.com:alistair23/qemu.git tags/pull-riscv-pull-20180703 > > for you to fetch changes up to 41fd29c43424cb3d660231ba75e9f52246639b08: > > hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device (2018-07-03 09:31:35 -0700) > > ---------------------------------------------------------------- > RISC-V: SoCify SiFive boards and connect GEM > > This series has three tasks: > 1. To convert the SiFive U and E machines into SoCs and boards > 2. To connect the Cadence GEM device to the SiFive U board > 3. Fix some device tree problems with the SiFive U board > > After this series the SiFive E and U boards have their SoCs split into > seperate QEMU objects, which can be used on future boards if desired. > > The RISC-V Virt and Spike boards have not been converted. They haven't > been converted as they aren't physical boards, so it doesn't make a > whole lot of sense to split them into an SoC and board. The only > disadvantage with this is that they now differ to the SiFive boards. > > This series also connect the Cadence GEM device to the SiFive U board. > There are some interrupt line changes requried before this is possible. Hi -- this seems to fail 'make check': TEST: tests/device-introspect-test... (pid=25542) /riscv32/device/introspect/list: OK /riscv32/device/introspect/list-fields: OK /riscv32/device/introspect/none: OK /riscv32/device/introspect/abstract: OK /i386/qom/pc-0.14: OK /riscv32/device/introspect/concrete: RAMBlock "riscv.sifive.u.mrom" already registered, abort! Broken pipe FAIL GTester: last random seed: R02S26c1f5ee936cd6398b20e9983b7c15a8 (pid=25562) /riscv32/device/introspect/abstract-interfaces: OK FAIL: tests/device-introspect-test It looks like riscv_sifive_u_soc_init() is creating a RAM memory region with a NULL parent. This means you can't create more than one of these devices (because they'll have clashing names). Other problems here: you're adding it to the system memory space, which devices really shouldn't do (the system memory space is owned by the board object, and devices shouldn't map themselves into it). You definitely shouldn't be doing that in the init function, because init isn't supposed to cause changes to the system (realize would be ok, I think). My guess is the test failure is some combination of doing this in init rather than realize and not using the right parent pointer for memory_region_init_rom(). Having the device map its regions itself will work, it's just not really the right way round... thanks -- PMM
On Wed, Jul 4, 2018 at 2:37 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 3 July 2018 at 17:34, Alistair Francis <alistair.francis@wdc.com> wrote: >> The following changes since commit f988c7e191141e92de2059d04a5f9a9bb01f399c: >> >> Merge remote-tracking branch 'remotes/shorne/tags/pull-or-20180703' into staging (2018-07-03 16:04:41 +0100) >> >> are available in the Git repository at: >> >> git@github.com:alistair23/qemu.git tags/pull-riscv-pull-20180703 >> >> for you to fetch changes up to 41fd29c43424cb3d660231ba75e9f52246639b08: >> >> hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device (2018-07-03 09:31:35 -0700) >> >> ---------------------------------------------------------------- >> RISC-V: SoCify SiFive boards and connect GEM >> >> This series has three tasks: >> 1. To convert the SiFive U and E machines into SoCs and boards >> 2. To connect the Cadence GEM device to the SiFive U board >> 3. Fix some device tree problems with the SiFive U board >> >> After this series the SiFive E and U boards have their SoCs split into >> seperate QEMU objects, which can be used on future boards if desired. >> >> The RISC-V Virt and Spike boards have not been converted. They haven't >> been converted as they aren't physical boards, so it doesn't make a >> whole lot of sense to split them into an SoC and board. The only >> disadvantage with this is that they now differ to the SiFive boards. >> >> This series also connect the Cadence GEM device to the SiFive U board. >> There are some interrupt line changes requried before this is possible. > > Hi -- this seems to fail 'make check': > > TEST: tests/device-introspect-test... (pid=25542) > /riscv32/device/introspect/list: OK > /riscv32/device/introspect/list-fields: OK > /riscv32/device/introspect/none: OK > /riscv32/device/introspect/abstract: OK > /i386/qom/pc-0.14: OK > /riscv32/device/introspect/concrete: > RAMBlock "riscv.sifive.u.mrom" already registered, abort! > Broken pipe > FAIL > GTester: last random seed: R02S26c1f5ee936cd6398b20e9983b7c15a8 > (pid=25562) > /riscv32/device/introspect/abstract-interfaces: OK > FAIL: tests/device-introspect-test I should have caught this. I had some issues and commented out the test and forgot to un-comment it. > > It looks like riscv_sifive_u_soc_init() is creating a RAM memory > region with a NULL parent. This means you can't create more > than one of these devices (because they'll have clashing > names). Other problems here: you're adding it to the system > memory space, which devices really shouldn't do (the system > memory space is owned by the board object, and devices > shouldn't map themselves into it). You definitely shouldn't > be doing that in the init function, because init isn't > supposed to cause changes to the system (realize would be > ok, I think). > > My guess is the test failure is some combination of doing > this in init rather than realize and not using the right > parent pointer for memory_region_init_rom(). Having the > device map its regions itself will work, it's just not > really the right way round... I fixed this to the device mapping it's memory in the realize. I think the SoC device should map it's own ROM (it lines up more with the physical SoC). I think that is how other SoC devices do it as well. I'm not really sure what other address space to map it to. Alistair > > thanks > -- PMM
On 6 July 2018 at 02:24, Alistair Francis <alistair23@gmail.com> wrote: > I think the SoC device should map it's own ROM (it lines up more with > the physical SoC). > > I think that is how other SoC devices do it as well. I'm not really > sure what other address space to map it to. I generally suggest the approach where the SoC object takes a MemoryRegion as a link QOM property. The board code puts the board-level devices into the system memory, and then passes that up to the SoC. The SoC creates a container MR, puts the thing it got passed by the board in as a background region, and then adds its own devices/ROMs/etc to the container. It also creates the CPU object(s). Then it passes the container to the CPU object. This approach broadly follows what the hardware does -- the SoC controls what goes where for the devices it is dealing with, but doesn't need to care what is outside it. Example in hw/arm/mps2-tz.c and probably others. thanks -- PMM
The following changes since commit f988c7e191141e92de2059d04a5f9a9bb01f399c: Merge remote-tracking branch 'remotes/shorne/tags/pull-or-20180703' into staging (2018-07-03 16:04:41 +0100) are available in the Git repository at: git@github.com:alistair23/qemu.git tags/pull-riscv-pull-20180703 for you to fetch changes up to 41fd29c43424cb3d660231ba75e9f52246639b08: hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device (2018-07-03 09:31:35 -0700) ---------------------------------------------------------------- RISC-V: SoCify SiFive boards and connect GEM This series has three tasks: 1. To convert the SiFive U and E machines into SoCs and boards 2. To connect the Cadence GEM device to the SiFive U board 3. Fix some device tree problems with the SiFive U board After this series the SiFive E and U boards have their SoCs split into seperate QEMU objects, which can be used on future boards if desired. The RISC-V Virt and Spike boards have not been converted. They haven't been converted as they aren't physical boards, so it doesn't make a whole lot of sense to split them into an SoC and board. The only disadvantage with this is that they now differ to the SiFive boards. This series also connect the Cadence GEM device to the SiFive U board. There are some interrupt line changes requried before this is possible. ---------------------------------------------------------------- Alistair Francis (7): hw/riscv/sifive_u: Create a SiFive U SoC object hw/riscv/sifive_e: Create a SiFive E SoC object hw/riscv/sifive_plic: Use gpios instead of irqs hw/riscv/sifive_u: Set the soc device tree node as a simple-bus hw/riscv/sifive_u: Set the interrupt controler number of interrupts hw/riscv/sifive_u: Move the uart device tree node under /soc/ hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device default-configs/riscv32-softmmu.mak | 2 + default-configs/riscv64-softmmu.mak | 2 + hw/riscv/sifive_e.c | 102 +++++++++++++++++------- hw/riscv/sifive_plic.c | 6 +- hw/riscv/sifive_u.c | 151 +++++++++++++++++++++++++++++------- hw/riscv/virt.c | 4 +- include/hw/riscv/sifive_e.h | 16 +++- include/hw/riscv/sifive_plic.h | 1 - include/hw/riscv/sifive_u.h | 25 +++++- 9 files changed, 241 insertions(+), 68 deletions(-)