Re: [Qemu-devel] [PATCH 03/17] ppc: Add a bunch of hypervisor SPRs to Book3s

[Thomas Huth]

On 15.03.2016 10:43, David Gibson wrote:
> 
> On Mon, Mar 14, 2016 at 08:14:59PM +0100, Thomas Huth wrote:
>> On 14.03.2016 17:56, Cédric Le Goater wrote:
>>> From: Benjamin Herrenschmidt <address@hidden>
>>>
>>> We don't give them a KVM reg number to most of the registers yet as no
>>> current KVM version supports HV mode. For DAWR and DAWRX, the KVM reg
>>> number is needed since this register can be set by the guest via the
>>> H_SET_MODE hypercall.
>>>
>>> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
>>> [clg: squashed in patch 'ppc: Add KVM numbers to some P8 SPRs' and
>>>       changed the commit log with a proposal of Thomas Huth ]
>>> Signed-off-by: Cédric Le Goater <address@hidden>
>>> ---
>>>  target-ppc/translate_init.c | 140 
>>> +++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 137 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>> index 6a11b41206e5..43c6e524a6bc 100644
>>> --- a/target-ppc/translate_init.c
>>> +++ b/target-ppc/translate_init.c
>>> @@ -1105,6 +1105,11 @@ static void gen_spr_amr (CPUPPCState *env)
>>>                       SPR_NOACCESS, SPR_NOACCESS,
>>>                       &spr_read_generic, &spr_write_generic,
>>>                       KVM_REG_PPC_UAMOR, 0);
>>> +    spr_register_hv(env, SPR_AMOR, "AMOR",
>>> +                    SPR_NOACCESS, SPR_NOACCESS,
>>> +                    SPR_NOACCESS, SPR_NOACCESS,
>>> +                    &spr_read_generic, &spr_write_generic,
>>> +                    0);
>>>  #endif /* !CONFIG_USER_ONLY */
>>>  }
>>>  #endif /* TARGET_PPC64 */
>>> @@ -7491,6 +7496,20 @@ static void gen_spr_book3s_dbg(CPUPPCState *env)
>>>                       KVM_REG_PPC_DABRX, 0x00000000);
>>>  }
>>>  
>>> +static void gen_spr_book3s_207_dbg(CPUPPCState *env)
>>> +{
>>> +    spr_register_kvm_hv(env, SPR_DAWR, "DAWR",
>>> +                        SPR_NOACCESS, SPR_NOACCESS,
>>> +                        SPR_NOACCESS, SPR_NOACCESS,
>>> +                        &spr_read_generic, &spr_write_generic,
>>> +                        KVM_REG_PPC_DAWR, 0x00000000);
>>> +    spr_register_kvm_hv(env, SPR_DAWRX, "DAWRX",
>>> +                        SPR_NOACCESS, SPR_NOACCESS,
>>> +                        SPR_NOACCESS, SPR_NOACCESS,
>>> +                        &spr_read_generic, &spr_write_generic,
>>> +                        KVM_REG_PPC_DAWRX, 0x00000000);
>>> +}
>>> +
>>>  static void gen_spr_970_dbg(CPUPPCState *env)
>>>  {
>>>      /* Breakpoints */
>>> @@ -7683,15 +7702,116 @@ static void gen_spr_power5p_lpar(CPUPPCState *env)
>>>      spr_register_kvm(env, SPR_LPCR, "LPCR",
>>>                       SPR_NOACCESS, SPR_NOACCESS,
>>>                       &spr_read_generic, &spr_write_generic,
>>> -                     KVM_REG_PPC_LPCR, 0x00000000);
>>> +                     KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
>>
>> Could we please postpone that hunk to a later, separate patch (after
>> QEMU 2.6 has been released)? It looks like it could maybe cause some
>> trouble with some emulated boards (e.g. there is some code in
>> target-ppc/excp_helper.c for example - which is currently disabled, but
>> I'm not sure whether there are other spots like this somewhere else).
> 
> I think this whole patch needs to wait until after 2.6, I'm not seeing
> a good rationale for squeezing it into 2.6 at this stage.

Well, this patch registers DAWR and DAWRX registers with KVM - so
without this patch, the hardware breakpoints will be lost during
migration. I haven't tested it, but I think that when somebody uses
hardware breakpoints in gdb in a KVM guest, and migrates it, then the
breakpoints won't be triggered anymore after migration without this patch.

Cédric, maybe you could send a patch that adds at least the DAWR and
DAWRX registers if David does not want to have the full patch for 2.6?

 Thomas

signature.asc
Description: OpenPGP digital signature