On Thu, Jun 30, 2016 at 7:06 AM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
> On 28 June 2016 at 22:44, Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote:
>> Add code to support writing to memory mapped peripherals via
>> cpu_memory_rw_debug(). The code of that function already supports
>> reading from such memory regions, so this commit makes that
>> functionality "symmetric".
>>
>> One use-case for that functionality is setting various registers of a
>> non-running CPU. A concrete example would be starting QEMU emulating
>> Cortex-M with -S, connecting with GDB and modifying the value of Vector
>> Table Offset register.
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
>
>> static uint16_t vring_used_idx(VirtQueue *vq)
>> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
>> index 9f38edf..d5b4966 100644
>> --- a/include/exec/cpu-all.h
>> +++ b/include/exec/cpu-all.h
>> @@ -302,6 +302,6 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf);
>> #endif /* !CONFIG_USER_ONLY */
>>
>> int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
>> - uint8_t *buf, int len, int is_write);
>> + uint8_t *buf, int len, MemTxType type);
>>
>> #endif /* CPU_ALL_H */
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 4ab6800..8fbaf1b 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -14,6 +14,13 @@
>> #ifndef MEMORY_H
>> #define MEMORY_H
>>
>> +typedef enum {
>> + MEMTX_READ,
>> + MEMTX_WRITE,
>> + MEMTX_PROGRAM,
>> +} MemTxType;
>
> We already have an enum for this: MMUAccessType.
> That is currently unhelpfully located in cpu-common.h, but there's a
> patch on list which should get applied soon which moves it to
> include/qom/cpu.h:
> http://patchwork.ozlabs.org/patch/635235/
>
>
>> +
>> #ifndef CONFIG_USER_ONLY
>>
>> #define DIRTY_MEMORY_VGA 0
>> @@ -1240,6 +1247,7 @@ AddressSpace *address_space_init_shareable(MemoryRegion *root,
>> */
>> void address_space_destroy(AddressSpace *as);
>>
>> +
>> /**
>> * address_space_rw: read from or write to an address space.
>> *
>> @@ -1251,11 +1259,11 @@ void address_space_destroy(AddressSpace *as);
>> * @addr: address within that address space
>> * @attrs: memory transaction attributes
>> * @buf: buffer with the data transferred
>> - * @is_write: indicates the transfer direction
>> + * @type: indicates the transfer type
>> */
>> MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
>> MemTxAttrs attrs, uint8_t *buf,
>> - int len, bool is_write);
>> + int len, MemTxType type);
>>
>> /**
>> * address_space_write: write to address space.
>> @@ -1268,10 +1276,11 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
>> * @addr: address within that address space
>> * @attrs: memory transaction attributes
>> * @buf: buffer with the data transferred
>> + * @force: force writing to ROM areas
>> */
>> MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
>> MemTxAttrs attrs,
>> - const uint8_t *buf, int len);
>> + const uint8_t *buf, int len, bool force);
>>
>> /* address_space_ld*: load from an address space
>> * address_space_st*: store to an address space
>
> I think this patch would be easier to review if it was
> split up, something like:
> * a patch which just converts the is_write bool parameter to the
> enum and updates all the callers, with no change in behaviour
> * a patch which makes use of the ability to pass in something other
> than 0 or 1
> * a patch which adds and uses address_space_write_debug(),
> or whatever API we go for
>
> The important bit is splitting the mechanical "convert bool
> to enum" part (which touches lots of files but makes no
> behaviour change) from the part which changes behaviour
> and doesn't touch many files.
OK, sounds good, will update in v2.
Andrey
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
