Re: [Qemu-devel] [PATCH 2/4] spapr: Enable DABRX special register

[Tom Musta <tommusta@gmail.com>]

On 4/3/2014 8:14 AM, Alexey Kardashevskiy wrote:
> This advertises Data Address Breakpoint Register Extension (DABRX) to
> the guest via hyperrtas list and enables it to migrate.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr.c              | 1 +
>  target-ppc/translate_init.c | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a11e121..451c473 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -307,6 +307,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>      uint32_t start_prop = cpu_to_be32(initrd_base);
>      uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
>      char hypertas_prop[] = "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
> +        "\0hcall-xdabr"
>          "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk\0hcall-set-mode";
>      char qemu_hypertas_prop[] = "hcall-memop1";
>      uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};


It isn't clear to me what is enabled with this.  Alexey: can you provide a little explanation
of what adding this string actually does?  And I assume the spelling of "xdabr" (versus "dabrx")
is intentional?

> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index d07e186..1627bb0 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7010,6 +7010,10 @@ static void init_proc_POWER7 (CPUPPCState *env)
>                       SPR_NOACCESS, SPR_NOACCESS,
>                       &spr_read_generic, &spr_write_generic,
>                       KVM_REG_PPC_PMC6, 0x00000000);
> +    spr_register_kvm(env, SPR_DABRX, "DABRX",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     KVM_REG_PPC_DABRX, 0x00000000);
>  #endif /* !CONFIG_USER_ONLY */
>      gen_spr_amr(env);
>      /* XXX : not implemented */
> 

I see a problem with this and it is caused by some of the P8 code that I had added a while back.
The P8 init code  (init_proc_POWER8) calls this init_proc_POWER7 routine.  So by adding DABRX
to the P7 code means P8 gets it for free.  Unfortunately, P8 doesn't have DABRX ... it supports
the new debug facilities (DAWR[X]).  The DABR and IABR registers are in the same boat.   I think
the init_proc_POWER8 code needs to become a self-sufficienct version.

I can fix and send to you or you can fix yourself ... let me know.