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.
thanks
-- PMM
--
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
