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

[Cédric Le Goater]

On 03/15/2016 11:49 AM, Thomas Huth wrote:
> 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?

yes. Here is my plan for the next patchset :

01/17 - ppc: Update SPR definitions (take) 
02/17 - ppc: Add macros to register hypervisor mode SPRs (needs a fix)
03/17 - ppc: Add a bunch of hypervisor SPRs to Book3s  (extract DAWR*)
04/17 - ppc: Add number of threads per core to the processor definition (drop 
for 2.6)
05/17 - ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV (drop 
for 2.6)
06/17 - ppc: Create cpu_ppc_set_papr() helper (take. needed by 11/17)
07/17 - ppc: Better figure out if processor has HV mode  (take)
08/17 - ppc: Add placeholder SPRs for DPDES and DHDES on P8 (take) 
09/17 - ppc: SPURR & PURR are HV writeable and privileged (drop for 2.6)
10/17 - ppc: Add dummy SPR_IC for POWER8  (take) 
11/17 - ppc: Initialize AMOR in PAPR mode  (take) 
12/17 - ppc: Fix writing to AMR/UAMOR (move hunk to 13)
13/17 - ppc: Add POWER8 IAMR register (rework with above)
14/17 - ppc: Add dummy write to VTB (drop for 2.6)
15/17 - ppc: Add dummy POWER8 MPPR register (drop for 2.6)
16/17 - ppc: Add dummy CIABR SPR (take) 
17/17 - ppc: A couple more dummy POWER8 Book4 regs (take) 

C.