Heterogeneous Multicore#135
Conversation
…support upto 8 cores
Small edits to adjust to updated pkg file
* riscbusiness: split buses into front and back sides quartus randomly assigned to all of the split bus signals in each instantiation of the caches causing a large number of build errors. This splits the bus into a front side that connects back_side<->RISCVBusiness and a back side that connects RISCVBusiness<->bus controller. * [misc] fixing failing riscv-tests * component_defines: rename address_translation_enabled to address_translation Helps deal with quartus not properly supporting generate compile time instantiation * misc: various fixes for quartus * [cleanup] renaming supervisor_enabled to supervisor, fixing build issues --------- Co-authored-by: William Cunningham <wrcunnin@purdue.edu>
* [misc] initial commits to fixing aft-dev FPGA build * [misc] fixing riscv-test failures - includes updates to tb_core.cc, changed virtual to debug. - includes updates to riscv-test isa and env patches - multicore (2-harts) now passes integer and multiplication virtual and physical tests, and atomic physical - (virtual atomic tests don't work due to amo_emu.c) * riscv-tests-env.patch: ignore pointer cast warning --------- Co-authored-by: Devin Singh <drsingh2518@icloud.com>
wrcunnin
left a comment
There was a problem hiding this comment.
How exhaustive has your testing been? Just want to make sure that if you max out one core (like 2KB caches, 2 way associative, 4-word blocks, 64 entry BTB) and weaken another (256B caches, 1 way, 2-word block, 16 entry BTB, etc.) that in the package and waveforms this is what actually gets generated.
Can you also link your presentations here?
| - `xlen` | ||
| - Bus type and endianness | ||
| - Cache block sizes | ||
| - Supervisor and address translation settings |
There was a problem hiding this comment.
I think we should loop TLB size into the per-core configuration. It would make more sense so we can put it with the other caching structures. I can help with this.
There was a problem hiding this comment.
@cole-nelson should we merge this with the normal core config script?
There was a problem hiding this comment.
Definitely needs to be merged into old core config
There was a problem hiding this comment.
see my comment on hetero_config_core.py. this may get reverted depending on that.
There was a problem hiding this comment.
Any diagrams to include with this? Would be good to have visuals for an example heterogeneous core layout with clock gating.
| - [ ] 2 stage pipeline RV32EAC little core | ||
|
|
||
| ## Current architecture plan for the Dual-core processor with MESI Coherence | ||
|  |
There was a problem hiding this comment.
@cole-nelson we should get this updated to use the diagram you used for AFTx08 presentation.
|
|
||
| if(num_harts > 1) { | ||
| // write to memory about required extension info | ||
| memory.write(tohost_address + 64, 1 << (extension - 'A'), 0xFF); |
| } | ||
|
|
||
| if(use_tohost_multi) { | ||
| if(tohost_address <= addr <= tohost_address + FROMHOST_OFFSET && (addr - tohost_address) % tohost_stride == 0) { |
There was a problem hiding this comment.
@cole-nelson i'm no c++ whiz, is this functionally sound?
There was a problem hiding this comment.
Nope. Needs to be tohost_address <= addr && addr <= (tohost_address + FROMHOST_OFFSET) && (addr - tohost_address) % tohost_stride == 0
| std::cout << "\t--debug : Allows debug strings to print from cores. Changes tohost-address functionality" << std::endl; | ||
| std::cout << "\t--notrace: Indicate to not generate a waveform file. Speeds up simulation incredibly." << std::endl; | ||
| std::cout << "\t--num-harts : Number of cores for multicore test. Defaults to 1." << std::endl; | ||
| std::cout << "\t--extension : RISC-V extension required for the core to run the test. Running pm env tests require this flag to run correctly." << std::endl; |
There was a problem hiding this comment.
add something like <sim-time> for --max-sim-time. we need to clarify that we have a field that is to come after it.
There was a problem hiding this comment.
delete this, we generally give these files the extension .dump and our .gitignore picks it up.
There was a problem hiding this comment.
Definitely needs to be merged into old core config
There was a problem hiding this comment.
Instead of passing the HART_ID everywhere, could we instead create a struct to pass relevant args through the hierarchy?
Something like
typedef struct {
bool RV32M;
...
} isa_config_t;
typedef struct { ... } cache_config_t;
typedef struct {
int HART_ID;
isa_config_t isa_config;
cache_config_t cache_config;
...
} core_config_t;
const core_config_t HART0_CONFIG = '{
...
};
const core_config_t HART1_CONFIG = '{
...
};
const core_config_t CONFIGS[NUM_HARTS] = {HART0_CONFIG, HART1_CONFIG};
...
RISCVBusiness #(.CONFIG(CONFIGS[HART_ID])) hart (...);Now the caches will take in a cache_config_t to handle configuration. I think it'll create a stronger link between the config and instantiations and hopefully avoid any potential foot guns from not using a core config parameter.
There was a problem hiding this comment.
Yeah should replace component_selection_defines.vh
| .product(product), | ||
| .finished(mul_finished) | ||
| ); | ||
| end |
There was a problem hiding this comment.
I think it's probably cleaner if we use enums to define the possibilities instead of strings. Then it becomes a switch on known values. Could be future work though
| } | ||
|
|
||
| if(use_tohost_multi) { | ||
| if(tohost_address <= addr <= tohost_address + FROMHOST_OFFSET && (addr - tohost_address) % tohost_stride == 0) { |
There was a problem hiding this comment.
Nope. Needs to be tohost_address <= addr && addr <= (tohost_address + FROMHOST_OFFSET) && (addr - tohost_address) % tohost_stride == 0
| // if (addr == FROMHOST_ADDR) { | ||
| // return 1; | ||
| // } |
| std::cout << "\t--debug : Allows debug strings to print from cores. Changes tohost-address functionality" << std::endl; | ||
| std::cout << "\t--notrace: Indicate to not generate a waveform file. Speeds up simulation incredibly." << std::endl; | ||
| std::cout << "\t--num-harts : Number of cores for multicore test. Defaults to 1." << std::endl; | ||
| std::cout << "\t--extension : RISC-V extension required for the core to run the test. Running pm env tests require this flag to run correctly." << std::endl; |
There was a problem hiding this comment.
What is the purpose of --extension?
| .core_clk_en(core_clk_en) | ||
| ); | ||
|
|
||
| assign pipeline_clk = CLK & core_clk_en; |
There was a problem hiding this comment.
Do we even support standalone RISCVBusiness for FPGA?
|
Tests failing for me with updated riscv-tests ( |
This adds heterogeneous multicore configuration feature with extensions and caches, riscv-tests pm environment support from tb_core.cc and run_riscv_tests.py, clock gating, and C multicore tests.
For riscv-tests to be run correctly, pull requests in riscv-test/ and riscv-test/env rvb branch needs to be applied.