- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13
          ot_earlgrey: add ePMP configuration machine property with facade feature
          #241
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ot-9.2.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -25,6 +25,7 @@ | |
| */ | ||
|  | ||
| #include "qemu/osdep.h" | ||
| #include <string.h> | ||
| #include "qemu/error-report.h" | ||
| #include "qemu/typedefs.h" | ||
| #include "qapi/error.h" | ||
|  | @@ -197,7 +198,7 @@ enum OtEGBoardDevice { | |
|  | ||
| #define OT_EG_IBEX_WRAPPER_NUM_REGIONS 2u | ||
|  | ||
| static const uint8_t ot_eg_pmp_cfgs[] = { | ||
| static uint8_t ot_eg_pmp_cfgs[] = { | ||
| /* clang-format off */ | ||
| IBEX_PMP_CFG(0, IBEX_PMP_MODE_OFF, 0, 0, 0), | ||
| IBEX_PMP_CFG(0, IBEX_PMP_MODE_OFF, 0, 0, 0), | ||
|  | @@ -218,7 +219,7 @@ static const uint8_t ot_eg_pmp_cfgs[] = { | |
| /* clang-format on */ | ||
| }; | ||
|  | ||
| static const uint32_t ot_eg_pmp_addrs[] = { | ||
| static uint32_t ot_eg_pmp_addrs[] = { | ||
| /* clang-format off */ | ||
| IBEX_PMP_ADDR(0x00000000), | ||
| IBEX_PMP_ADDR(0x00000000), | ||
|  | @@ -1492,6 +1493,10 @@ struct OtEGMachineState { | |
| bool no_epmp_cfg; | ||
| bool ignore_elf_entry; | ||
| bool verilator; | ||
| /* ePMP region specification string */ | ||
| char *epmp_regions; | ||
| /* Whether to redirect an PMP region's CSR reads and writes to the facade */ | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 | ||
| bool pmp_region_masked[MAX_RISCV_PMPS]; | ||
| }; | ||
|  | ||
| struct OtEGMachineClass { | ||
|  | @@ -1553,6 +1558,275 @@ static void ot_eg_soc_flash_ctrl_configure( | |
| } | ||
| } | ||
|  | ||
| /* whether to apply a PMP CSR read/write hook */ | ||
| static RISCVException pmp_predicate(CPURISCVState *env, int csrno) | ||
| { | ||
| if (riscv_cpu_cfg(env)->pmp) { | ||
| if (csrno <= CSR_PMPCFG3) { | ||
| uint32_t reg_index = csrno - CSR_PMPCFG0; | ||
|  | ||
| /* TODO: RV128 restriction check */ | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can remove this: OT is neither RV64 nor RV128 | ||
| if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) { | ||
| return RISCV_EXCP_ILLEGAL_INST; | ||
| } | ||
| } | ||
|  | ||
| return RISCV_EXCP_NONE; | ||
| } | ||
|  | ||
| return RISCV_EXCP_ILLEGAL_INST; | ||
| } | ||
|  | ||
| /* | ||
| * Facades for PMP CSRs - reads and writes to masked regions end up here instead | ||
| * of the effective PMP configuration. | ||
| */ | ||
| static target_ulong reg_pmpcfg_facade[MAX_RISCV_PMPS / 4] = { 0 }; | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to { 0 } since it a static declaration. | ||
| static target_ulong reg_pmpaddr_facade[MAX_RISCV_PMPS] = { 0 }; | ||
|  | ||
| static RISCVException | ||
| read_pmpcfg_masked(CPURISCVState *env, int csrno, target_ulong *val) | ||
| { | ||
| OtEGMachineState *ms = RISCV_OT_EG_MACHINE(qdev_get_machine()); | ||
|  | ||
| uint32_t reg_index = csrno - CSR_PMPCFG0; | ||
|  | ||
| /* each PMPCFG CSR covers four PMP regions; mask the affected parts */ | ||
| target_ulong mask = 0; | ||
| for (unsigned i = 0; i < 4; i++) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit pick: ix, idx, ... avoid single char var. | ||
| if (ms->pmp_region_masked[reg_index * 4 + i]) { | ||
| mask |= (0xff << (8 * i)); | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. u suffix | ||
| } | ||
| } | ||
|  | ||
| /* only read from the PMP if there are unmasked regions in this CSR */ | ||
| if (mask != -1) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. explicit cast or maybe a mask based on  | ||
| *val = pmpcfg_csr_read(env, reg_index) & ~mask; | ||
| } | ||
|  | ||
| /* overlay the facade for masked regions */ | ||
| *val |= (reg_pmpcfg_facade[reg_index] & mask); | ||
|  | ||
| return RISCV_EXCP_NONE; | ||
| } | ||
|  | ||
| static RISCVException | ||
| write_pmpcfg_masked(CPURISCVState *env, int csrno, target_ulong val) | ||
| { | ||
| OtEGMachineState *ms = RISCV_OT_EG_MACHINE(qdev_get_machine()); | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not think you can use such a call here for performace reason. | ||
|  | ||
| uint32_t reg_index = csrno - CSR_PMPCFG0; | ||
|  | ||
| /* each PMPCFG CSR covers four PMP regions; mask the affected parts */ | ||
| target_ulong mask = 0; | ||
| for (unsigned i = 0; i < 4; i++) { | ||
| if (ms->pmp_region_masked[reg_index * 4 + i]) { | ||
| mask |= (0xff << (8 * i)); | ||
| } | ||
| } | ||
|  | ||
| /* only write to the PMP config if there are unmasked regions in this CSR */ | ||
| if (mask != -1) { | ||
| /* combine the current CSR value from the PMP with the facade */ | ||
| target_ulong csr_val = pmpcfg_csr_read(env, reg_index) & mask; | ||
| csr_val |= val & ~mask; | ||
| pmpcfg_csr_write(env, reg_index, csr_val); | ||
| } | ||
|  | ||
| reg_pmpcfg_facade[reg_index] = val; | ||
|  | ||
| return RISCV_EXCP_NONE; | ||
| } | ||
|  | ||
| static RISCVException | ||
| read_pmpaddr_masked(CPURISCVState *env, int csrno, target_ulong *val) | ||
| { | ||
| OtEGMachineState *ms = RISCV_OT_EG_MACHINE(qdev_get_machine()); | ||
|  | ||
| uint32_t reg_index = csrno - CSR_PMPADDR0; | ||
|  | ||
| /* if this region is masked, read from the facade instead of the PMP */ | ||
| if (ms->pmp_region_masked[reg_index]) { | ||
| *val = reg_pmpaddr_facade[reg_index]; | ||
| } else { | ||
| *val = pmpaddr_csr_read(env, reg_index); | ||
| } | ||
|  | ||
| return RISCV_EXCP_NONE; | ||
| } | ||
|  | ||
| static RISCVException | ||
| write_pmpaddr_masked(CPURISCVState *env, int csrno, target_ulong val) | ||
| { | ||
| OtEGMachineState *ms = RISCV_OT_EG_MACHINE(qdev_get_machine()); | ||
|  | ||
| uint32_t reg_index = csrno - CSR_PMPADDR0; | ||
|  | ||
| /* if this region is masked, write to the facade instead of the PMP */ | ||
| if (ms->pmp_region_masked[reg_index]) { | ||
| reg_pmpaddr_facade[reg_index] = val; | ||
| } else { | ||
| pmpaddr_csr_write(env, reg_index, val); | ||
| } | ||
|  | ||
| return RISCV_EXCP_NONE; | ||
| } | ||
|  | ||
| static riscv_csr_operations pmpcfg_masked = { | ||
| .name = "pmpcfg", | ||
| .predicate = pmp_predicate, | ||
| .read = read_pmpcfg_masked, | ||
| .write = write_pmpcfg_masked, | ||
| }; | ||
|  | ||
| static riscv_csr_operations pmpaddr_masked = { | ||
| .name = "pmpaddr", | ||
| .predicate = pmp_predicate, | ||
| .read = read_pmpaddr_masked, | ||
| .write = write_pmpaddr_masked, | ||
| }; | ||
|  | ||
| /* | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would think this is such a large feature that it deserves its own file ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a bit weird that the epmp configuration is defined in the machine itself. where you would define the epmp propery on the epmp device. It would be much cleaner and could be re-used for another SoC. | ||
| * Parse and apply the PMP configuration specification provided as a property. | ||
| * | ||
| * The specification string contains one or more region configurations separated | ||
| * by `#` characters. Each configuration has the following syntax: | ||
| * | ||
| * <region index>:<address>:<mode>:<flags>` | ||
| * | ||
| * - `<region index>` is the zero-based index of the region to configure. | ||
| * - `<address>` is the hexadecimal address field for the region. | ||
| * - `<mode>` is an ePMP region mode: `OFF`, `TOR`, `NA4`, or `NAPOT`. | ||
| * - `<flags>` is a set of uppercase characters denoting thee region's flags. | ||
| * | ||
| * The supported flags are: | ||
| * | ||
| * - `L`: locked | ||
| * - `R`: readable | ||
| * - `W`: writable | ||
| * - `X`: executable | ||
| * - `F`: facade | ||
| * | ||
| * The "facade" flag causes writes to a region's CSRs to have no effect on PMP | ||
| * logic, but can still be read back as if they were successfully set. | ||
| */ | ||
| static void ot_eg_soc_configure_pmp(OtEGMachineState *ms, Error **errp) | ||
| { | ||
| const char *config = ms->epmp_regions; | ||
|  | ||
| /* configure the CSR hooks for all `PMPCFG` and `PMPADDR` CSRs */ | ||
| for (int csr = CSR_PMPCFG0; csr <= CSR_PMPCFG3; csr++) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unsigned | ||
| riscv_set_csr_ops(csr, &pmpcfg_masked); | ||
| } | ||
| for (int csr = CSR_PMPADDR0; csr <= CSR_PMPADDR15; csr++) { | ||
| riscv_set_csr_ops(csr, &pmpaddr_masked); | ||
| } | ||
|  | ||
| /* escape early if config is empty */ | ||
| if (config == NULL || *config == '\0') { | ||
| return; | ||
| } | ||
|  | ||
| while (true) { | ||
| char idx_str[3] = { 0 }; | ||
| char addr_str[9] = { 0 }; | ||
| char mode_str[6] = { 0 }; | ||
| char flags[6] = { 0 }; | ||
| unsigned len; | ||
|  | ||
| /* extract one region configuration from the string */ | ||
| int parsed = sscanf(config, "%2[0-9]:%8[0-9a-f]:%5[^:]:%5[LRWXF]%n", | ||
| idx_str, addr_str, mode_str, flags, &len); | ||
|  | ||
| /* only accept when all parts of the configuration were present */ | ||
| if (parsed != 4) { | ||
| error_setg(errp, "bad epmp format: expected 4 parts, got %d", | ||
| parsed); | ||
| return; | ||
| } | ||
|  | ||
| /* parse the index */ | ||
| unsigned pmp_idx = strtol(idx_str, NULL, 10); | ||
|  | ||
| /* parse the address as hex */ | ||
| target_ulong addr = strtol(addr_str, NULL, 16); | ||
|  | ||
| /* parse the mode */ | ||
| unsigned mode; | ||
| if (strncmp(mode_str, "OFF", 3) == 0) { | ||
| mode = IBEX_PMP_MODE_OFF; | ||
| } else if (strncmp(mode_str, "TOR", 3) == 0) { | ||
| mode = IBEX_PMP_MODE_TOR; | ||
| } else if (strncmp(mode_str, "NA4", 3) == 0) { | ||
| mode = IBEX_PMP_MODE_NA4; | ||
| } else if (strncmp(mode_str, "NAPOT", 5) == 0) { | ||
| mode = IBEX_PMP_MODE_NAPOT; | ||
| } else { | ||
| error_setg(errp, | ||
| "bad mode %s, expected `OFF`, `TOR`, `NA4`, or `NAPOT`", | ||
| mode_str); | ||
| return; | ||
| } | ||
|  | ||
| /* parse the flags */ | ||
| bool l = false, r = false, w = false, x = false, f = false; | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. single char var(s) | ||
| for (unsigned i = 0; flags[i]; i++) { | ||
| switch (flags[i]) { | ||
| case 'L': | ||
| l = true; | ||
| break; | ||
| case 'R': | ||
| r = true; | ||
| break; | ||
| case 'W': | ||
| w = true; | ||
| break; | ||
| case 'X': | ||
| x = true; | ||
| break; | ||
| case 'F': | ||
| f = true; | ||
| break; | ||
| default: | ||
| error_setg(errp, | ||
| "bad flag %c, expected `L`, `R`, `W`, `X`, or `F`", | ||
| flags[i]); | ||
| return; | ||
| } | ||
| } | ||
|  | ||
| /* prepare the `PMPCFG` and `PMPADDR` codes */ | ||
| uint8_t pmpcfg = IBEX_PMP_CFG(l, mode, x, w, r); | ||
| target_ulong pmpaddr = IBEX_PMP_ADDR(addr); | ||
|  | ||
| (void)pmp_idx; | ||
| (void)pmpcfg; | ||
| (void)pmpaddr; | ||
| ot_eg_pmp_cfgs[pmp_idx] = pmpcfg; | ||
| ot_eg_pmp_addrs[pmp_idx] = pmpaddr; | ||
|  | ||
| /* remember if region is masked against the facade */ | ||
| if (f) { | ||
| ms->pmp_region_masked[pmp_idx] = true; | ||
| } | ||
|  | ||
| /* determine whether there are more configurations to parse */ | ||
| if (config[len] == '#') { | ||
| config = config + len + 1; | ||
| continue; | ||
| } | ||
|  | ||
| if (config[len] == '\0') { | ||
| break; | ||
| } | ||
|  | ||
| error_setg(errp, | ||
| "bad region format, expected `,` or end of string, got %c", | ||
| config[len]); | ||
| return; | ||
| } | ||
| } | ||
|  | ||
| static void ot_eg_soc_hart_configure(DeviceState *dev, const IbexDeviceDef *def, | ||
| DeviceState *parent) | ||
| { | ||
|  | @@ -1566,6 +1840,8 @@ static void ot_eg_soc_hart_configure(DeviceState *dev, const IbexDeviceDef *def, | |
| return; | ||
| } | ||
|  | ||
| ot_eg_soc_configure_pmp(ms, &error_fatal); | ||
|  | ||
| pmp_cfg = qlist_new(); | ||
| for (unsigned ix = 0; ix < ARRAY_SIZE(ot_eg_pmp_cfgs); ix++) { | ||
| qlist_append_int(pmp_cfg, ot_eg_pmp_cfgs[ix]); | ||
|  | @@ -2006,6 +2282,29 @@ static void ot_eg_machine_set_verilator(Object *obj, bool value, Error **errp) | |
| s->verilator = value; | ||
| } | ||
|  | ||
| static char *ot_eg_machine_get_epmp_regions(Object *obj, Error **errp) | ||
| { | ||
| OtEGMachineState *s = RISCV_OT_EG_MACHINE(obj); | ||
| (void)errp; | ||
|  | ||
| return s->epmp_regions; | ||
| } | ||
|  | ||
| static void | ||
| ot_eg_machine_set_epmp_regions(Object *obj, const char *value, Error **errp) | ||
| { | ||
| OtEGMachineState *s = RISCV_OT_EG_MACHINE(obj); | ||
| (void)errp; | ||
|  | ||
| if (s->epmp_regions) { | ||
| free(s->epmp_regions); | ||
| } | ||
|  | ||
| size_t len = strlen(value) + 1; | ||
| s->epmp_regions = g_malloc(len); | ||
| strlcpy(s->epmp_regions, value, len); | ||
| } | ||
|  | ||
| static ResettableState *ot_eg_machine_get_reset_state(Object *obj) | ||
| { | ||
| OtEGMachineState *s = RISCV_OT_EG_MACHINE(obj); | ||
|  | @@ -2047,6 +2346,11 @@ static void ot_eg_machine_instance_init(Object *obj) | |
| object_property_add_bool(obj, "verilator", &ot_eg_machine_get_verilator, | ||
| &ot_eg_machine_set_verilator); | ||
| object_property_set_description(obj, "verilator", "Use Verilator clocks"); | ||
| object_property_add_str(obj, "epmp-regions", | ||
| &ot_eg_machine_get_epmp_regions, | ||
| &ot_eg_machine_set_epmp_regions); | ||
| object_property_set_description( | ||
| obj, "epmp-regions", "Set default ePMP memory region configuration"); | ||
| } | ||
|  | ||
| static void ot_eg_machine_init(MachineState *state) | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think this is required.