Philippe Mathieu-Daudé <phi...@linaro.org> writes:
> On 3/10/23 20:30, Richard Henderson wrote:
>> From: "Vanderson M. do Rosario" <vanderson...@gmail.com>
>> Introduce a MonitorDisasSpace to replace the current is_physical
>> boolean argument to monitor_disas. Generate an error if we attempt
>> to read past the end of a single RAMBlock.
>> Signed-off-by: Vanderson M. do Rosario <vanderson...@gmail.com>
>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>> Signed-off-by: Fei Wu <fei2...@intel.com>
>> [rth: Split out of a larger patch; validate the RAMBlock size]
>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> include/disas/disas.h | 8 +++++++-
>> disas/disas-mon.c | 32 ++++++++++++++++++++++++++++++--
>> monitor/hmp-cmds-target.c | 27 ++++++++++++++++-----------
>> 3 files changed, 53 insertions(+), 14 deletions(-)
>> diff --git a/include/disas/disas.h b/include/disas/disas.h
>> index 176775eff7..cd99b0ccd0 100644
>> --- a/include/disas/disas.h
>> +++ b/include/disas/disas.h
>> @@ -5,8 +5,14 @@
>> void disas(FILE *out, const void *code, size_t size);
>> void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size);
>> +typedef enum {
>> + MON_DISAS_GVA, /* virtual */
>> + MON_DISAS_GPA, /* physical */
>> + MON_DISAS_GRA, /* ram_addr_t */
>> +} MonitorDisasSpace;
>
>
> Obviously I'd rather see MonitorDisasSpace = {MON_DISAS_GVA/GPA}
> introduced in a preliminary patch, but just saying.
>
>> static void memory_dump(Monitor *mon, int count, int format, int wsize,
>> - hwaddr addr, int is_physical)
>> + hwaddr addr, MonitorDisasSpace space)
>> {
>> int l, line_size, i, max_digits, len;
>> uint8_t buf[16];
>> uint64_t v;
>> CPUState *cs = mon_get_cpu(mon);
>> - if (!cs && (format == 'i' || !is_physical)) {
>
> Why is the '!cs' check removed? Otherwise LGTM.
Yeah it breaks the monitor I think with should be:
if (!cs && (space == MON_DISAS_GVA || format == 'i'))
>
>> + if (space == MON_DISAS_GVA || format == 'i') {
>> monitor_printf(mon, "Can not dump without CPU\n");
>> return;
>> }
>> if (format == 'i') {
>> - monitor_disas(mon, cs, addr, count, is_physical);
>> - return;
>> + monitor_disas(mon, cs, addr, count, space);
>> }
>> len = wsize * count;
>> @@ -163,15 +162,21 @@ static void memory_dump(Monitor *mon, int count, int
>> format, int wsize,
>> }
>> while (len > 0) {
>> - if (is_physical) {
>> - monitor_printf(mon, HWADDR_FMT_plx ":", addr);
>> - } else {
>> + switch (space) {
>> + case MON_DISAS_GVA:
>> monitor_printf(mon, TARGET_FMT_lx ":", (target_ulong)addr);
>> + break;
>> + case MON_DISAS_GPA:
>> + monitor_printf(mon, HWADDR_FMT_plx ":", addr);
>> + break;
>> + default:
>> + g_assert_not_reached();
>> }
>> l = len;
>> - if (l > line_size)
>> + if (l > line_size) {
>> l = line_size;
>> - if (is_physical) {
>> + }
>> + if (space == MON_DISAS_GPA) {
>> AddressSpace *as = cs ? cs->as : &address_space_memory;
>> MemTxResult r = address_space_read(as, addr,
>> MEMTXATTRS_UNSPECIFIED,
>> buf, l);
>> @@ -235,7 +240,7 @@ void hmp_memory_dump(Monitor *mon, const QDict *qdict)
>> int size = qdict_get_int(qdict, "size");
>> target_long addr = qdict_get_int(qdict, "addr");
>> - memory_dump(mon, count, format, size, addr, 0);
>> + memory_dump(mon, count, format, size, addr, MON_DISAS_GVA);
>> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
