Re: [PATCH v2 2/3] spapr: Add SPAPR_CAP_AIL_MODE_3 for AIL mode 3 support for H_SET_MODE hcall

[David Gibson]

On Wed, Feb 23, 2022 at 05:42:06PM +1000, Nicholas Piggin wrote:
> Excerpts from David Gibson's message of February 17, 2022 10:17 am:
> > On Wed, Feb 16, 2022 at 04:39:02PM +1000, Nicholas Piggin wrote:
> >> The behaviour of the Address Translation Mode on Interrupt resource is
> >> not consistently supported by all CPU versions or all KVM versions:
> >> KVM-HV does not support mode 2, and does not support mode 3 on POWER7 or
> >> early POWER9 processesors. KVM PR only supports mode 0. TCG supports all
> >> modes (0, 2, 3). This leads to inconsistencies in guest behaviour and
> >> could cause problems migrating guests.
> >> 
> >> This was not noticable for Linux guests for a long time because the
> >> kernel only uses modes 0 and 3, and it used to consider AIL-3 to be
> >> advisory in that it would always keep the AIL-0 vectors around. Recent
> >> Linux guests depend on the AIL mode working as specified in order to
> >> support the SCV facility interrupt. If AIL-3 can not be provided, then
> >> Linux must be given an error so it can disable the SCV facility, rather
> >> than silently failing.
> >> 
> >> Add the ail-mode-3 capability to specify that AIL-3 is supported. AIL-0
> >> is implied as the baseline, and AIL-2 is no longer supported by spapr.
> >> AIL-2 is not known to be used by any software, but support in TCG could
> >> be restored with an ail-mode-2 capability quite easily if a regression
> >> is reported.
> >> 
> >> Modify the H_SET_MODE Address Translation Mode on Interrupt resource
> >> handler to check capabilities and correctly return error if not
> >> supported.
> >> 
> >> A heuristic is added for KVM to determine AIL-3 support before the
> >> introduction of a new KVM CAP, because blanket disabling AIL-3 has too
> >> much performance cost.
> >> 
> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > 
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > [snip]
> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> >> index dc93b99189..1338c41f8f 100644
> >> --- a/target/ppc/kvm.c
> >> +++ b/target/ppc/kvm.c
> >> @@ -2563,6 +2563,35 @@ int kvmppc_has_cap_rpt_invalidate(void)
> >>      return cap_rpt_invalidate;
> >>  }
> >>  
> >> +bool kvmppc_supports_ail_3(void)
> >> +{
> >> +    PowerPCCPUClass *pcc = kvm_ppc_get_host_cpu_class();
> >> +
> >> +    /*
> >> +     * KVM PR only supports AIL-0
> >> +     */
> >> +    if (kvmppc_is_pr(kvm_state)) {
> >> +        return 0;
> >> +    }
> >> +
> >> +    /*
> >> +     * KVM HV hosts support AIL-3 on POWER8 and above, except for radix
> >> +     * mode on some early POWER9s.
> >> +     */
> >> +    if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
> >> +        return 0;
> >> +    }
> >> +
> >> +    /* These tests match the CPU_FTR_P9_RADIX_PREFETCH_BUG flag in Linux 
> >> */
> >> +    if (((pcc->pvr & 0xffffff00) == CPU_POWERPC_POWER9_DD1) ||
> >> +        ((pcc->pvr & 0xffffff00) == CPU_POWERPC_POWER9_DD20) ||
> >> +        ((pcc->pvr & 0xffffff00) == CPU_POWERPC_POWER9_DD21)) {
> >> +        return 0;
> >> +    }
> > 
> > Deducing what KVM supports rather than getting it to tell us
> > explicitly with a cap is usually frowned upon.  However, given the
> > earlier discussion, I'm satisfied that this is the least bad available
> > option, at least for now.
> 
> BTW this particular test doesn't work as I hoped because we only have
> a power9 dd2.0 model.

Oh.. yes... IIRC dd2.0 is barely present in the wild, since it doesn't
have vital Spectre mitigations....

> Adding a 2.2 or 2.3 would be possible. Maybe overkill.  I'll change
> the test just to catch all POWER9 for now.

... no, given the above, I think we should definitely add the newer
models.  Maybe even remove dd2.0 and replace it with the newer ones.

> KVM cap has been allocated in the upstream kvm tree now though, so I'll
> re-send soon.
> 
> Thanks,
> Nick
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature