Draft: Add new proprieties and wrappers functions#61
Conversation
tmarcero
requested review from
alwalidsalama,
androm3da,
mahmdkamel and
markfoodyburton
tmarcero
changed the title
|
|
||
| switch (ret) { | ||
| case 0: // SUCCESS | ||
| break; |
There was a problem hiding this comment.
seems a little excessive to do the, is there a 'perror' equivalent? Or should we just print the number, Also, is this ARM specific (as the error messages suggest)? this is in CPU.h so it should work for hexagon, riscv, ...?
There was a problem hiding this comment.
This is ARM specific I guess, the functions calls will be arm_set_cpu_on_and_reset and arm_set_cpu_off from qemu, and those functions are returning the values coming from : https://github.com/qualcomm/qemu/blob/d5220094f479931acdc35656b0d35801bb8913a7/target/arm/arm-powerctl.h#L16
Not sure those wrappers functions will work for hexagon or riscv, maybe there is an equivalent in qemu to turn ON/OFF them.
So maybe move this code in more appropriate ARM file ?
- libqemu-v11.0-v0.6: - qct-qtimer: add support for frame_stride property - CortexR52: Add new pmsav8r_hdregion property - Update set_cp15_cbar wrapper function - Add new wrappers functions and export them with getter functions - Add new wrapper function to set IMP_BUILDOPTR register - qct-qtimer: add ticker-ctrl property and remove the start-ticking one (OnOffAuto) Signed-off-by: Marceron Thomas <tmarcero@qti.qualcomm.com>
- Add EL2 property - Add Multi-processor affinity property - Add PMSAv7 and PMSAv8 properties to set the MPU number of supported regions - Add CBAR property Signed-off-by: tmarcero <tmarcero@quicinc.com>
- In the case of some SoC is using multiples GICs, we will need to set this, otherwise we will have this kind of errors : `Assertion `oldreg->type & ARM_CP_OVERRIDE' failed` which means that same GIC registers will be configured multiples times. It's because the same CPU index will be return by `qemu_get_cpu` function except if we use this property. Signed-off-by: tmarcero <tmarcero@quicinc.com>
- Add new getter functions to fetch QEMU properties - Add ARM power control function to power on and unhalt CPUs Signed-off-by: tmarcero <tmarcero@quicinc.com>
| bool value = m_int->exports().object_property_get_bool(m_obj, name, &e); | ||
|
|
||
| if (e != nullptr) { | ||
| throw GetPropertyException("bool", name, m_int->exports().error_get_pretty(e)); |
There was a problem hiding this comment.
I'm not sure throwing is a very good idea here, I think you should SC_REPORT_FATAL.....
Unless you really ARE try/catching these functions?
There was a problem hiding this comment.
Honestly no clue, I just took the logic in get_prop_link which already exist here:
https://github.com/qualcomm/qbox/blob/main/qemu-components/common/src/libqemu-cxx/object.cc#L114
And put the same logic for the other getter
markfoodyburton
left a comment
markfoodyburton
left a comment
There was a problem hiding this comment.
where are the 'getters' used?
This is not used for the moment, it's in case of someone later want to use it, or for debug purpose :-) |
| void write_gpio::set_gpio_method() | ||
| { | ||
| // Let's set GPIOs/FUSES bind with multi_init_socket to 1 | ||
| gpio_sig_socket.async_write_vector({ 1 }); |
There was a problem hiding this comment.
can we expose 1 here to be a CCI parameter ? may be in future this needs to be written as 0 not 1 !
| class write_gpio : public sc_core::sc_module | ||
| { | ||
| SCP_LOGGER(); | ||
| cci::cci_broker_handle m_broker; // Should be a private broker for efficiency, but requires fixes to cci utils to |
There was a problem hiding this comment.
I dot think we need to keep this comment here!
| cci::cci_broker_handle m_broker; // Should be a private broker for efficiency, but requires fixes to cci utils to | ||
| // prevent use of cci_get_broker() | ||
|
|
||
| sc_core::sc_event set_gpio_event; |
There was a problem hiding this comment.
from where is this event notified ?
|
|
||
| void post_init(); | ||
| void register_reset(); | ||
|
|
There was a problem hiding this comment.
The name of the commit is a bit misleading I guess ? add new wrapper functions for what ?!
| cpu.set_prop_int("mp-affinity", p_mp_affinity); | ||
| } | ||
|
|
||
| cpu.set_prop_bool("has_el2", true); |
There was a problem hiding this comment.
I think "has_el2" should be a CCI parameter ?!
- Add new callback to power ON ARM cpus Signed-off-by: tmarcero <tmarcero@quicinc.com>
- This model allows connecting multiple Target Signal Sockets using a multi Initiator socket. It will set all targets connected to the multi initiator socket to '1' by default. This allows defining GPIOs/fuses, for example. Signed-off-by: tmarcero <tmarcero@quicinc.com>
- This register is specific to Cortex-R52, it need to be configured in some specific chips which expect to have Bus protection bit set. Signed-off-by: tmarcero <tmarcero@quicinc.com>
- p_cnttid is now a vector because some SoC can have more than 8 frames, so we will have more than one CNTTID register. - Add ticker_ctrl CCI param to handle if we want always running Qtimers or not. ON: Qtimers will always tick OFF: Qtimers will never tick AUTO: Qtimers will be controlled entirely by SW Signed-off-by: Marceron Thomas <tmarcero@qti.qualcomm.com>
3 participants
This PR follows this one:
qualcomm/qemu#142