On 15/09/2016 16:13, Brijesh Singh wrote:
> 1) something like this
>
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index a9d8aef..6322265 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1379,13 +1379,22 @@ void x86_cpu_exec_exit(CPUState *cs)
> }
>
> #ifndef CONFIG_USER_ONLY
> +static inline MemTxAttrs get_mem_debug_attrs(CPUX86State *env)
> +{
> + MemTxAttrs attrs = cpu_get_mem_attrs(env);
> +
> + attrs.debug = MEMTXATTRS_DEBUG;
> +
> + return attrs;
> +}
> +
> uint8_t x86_ldub_phys(CPUState *cs, hwaddr addr)
> {
> X86CPU *cpu = X86_CPU(cs);
> CPUX86State *env = &cpu->env;
>
> return address_space_ldub(cs->as, addr,
> - cpu_get_mem_attrs(env),
> + get_mem_debug_attrs(env),
> NULL);
> }
This changes the semantics of x86_ld*_phys so it's not acceptable.
You need new exec.c functions that wrap
cpu_physical_memory_rw_debug_internal so that they end up calling your
hooks. Like this:
x86_cpu_get_phys_page_debug
-> calls ldl_phys_debug
-> calls cpu_physical_memory_rw_debug_internal
uint32_t ldl_phys_debug(CPUState *cs, hwaddr addr)
{
/* This really should be a little more complex to support
* SMRAM, but this is enough for now. I'll provide you with a
* diff when you post v2.
*/
MemTxAttrs attrs = { .debug = 1 };
int asidx = cpu_asidx_from_attrs(cpu, attrs);
uint32_t val;
cpu_physical_memory_rw_debug_internal(cpu->cpu_ases[asidx].as,
addr, (void *) &val,
4, READ_DATA);
return tswap32(val);
}
/* and the same for ldq_phys_debug */
These two new functions can replace x86_ld*_phys in
x86_cpu_get_phys_page_debug.
Technically I'm not even sure you need the new .debug attribute now that
the hooks are never called by the device emulation DMA path. Still,
asserting something like assert(attrs->debug || sev_info.state ==
SEV_LAUNCH_START) in your hooks is probably a good idea. Even if you
only need the new attribute for that assertion, keep it. It's just a
couple lines of code.
Paolo
