On Wed, Feb 16, 2022 at 11:50:34AM +1000, Nicholas Piggin wrote:
> Excerpts from David Gibson's message of February 15, 2022 11:10 am:
> > On Mon, Feb 14, 2022 at 09:17:48PM +1000, Nicholas Piggin wrote:
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 222c1b6bbd..5dec056796 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -811,32 +811,35 @@ static target_ulong
> >> h_set_mode_resource_le(PowerPCCPU *cpu,
> >> }
> >>
> >> static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
> >> + SpaprMachineState
> >> *spapr,
> >> target_ulong
> >> mflags,
> >> target_ulong
> >> value1,
> >> target_ulong
> >> value2)
> >> {
> >> - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> >> -
> >> - if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
> >> - return H_P2;
> >> - }
> >> if (value1) {
> >> return H_P3;
> >> }
> >> +
> >> if (value2) {
> >> return H_P4;
> >> }
> >>
> >> - if (mflags == 1) {
> >> - /* AIL=1 is reserved in POWER8/POWER9/POWER10 */
> >> + /* AIL-1 is not architected, and AIL-2 is not supported by QEMU. */
> >> + if (mflags == 1 || mflags == 2) {
> >
> > This test is redundant with the one below, isn't it?
>
> Ah, yes.
>
> >
> >> return H_UNSUPPORTED_FLAG;
> >> }
> >>
> >> - if (mflags == 2 && (pcc->insns_flags2 & PPC2_ISA310)) {
> >> - /* AIL=2 is reserved in POWER10 (ISA v3.1) */
> >> + /* Only 0 and 3 are possible */
> >> + if (mflags != 0 && mflags != 3) {
> >> return H_UNSUPPORTED_FLAG;
> >> }
> >>
> >> + if (mflags == 3) {
> >> + if (!spapr_get_cap(spapr, SPAPR_CAP_AIL_MODE_3)) {
> >> + return H_UNSUPPORTED_FLAG;
> >> + }
> >> + }
> >> +
> >> spapr_set_all_lpcrs(mflags << LPCR_AIL_SHIFT, LPCR_AIL);
> >>
> >> return H_SUCCESS;
> >> @@ -853,7 +856,7 @@ static target_ulong h_set_mode(PowerPCCPU *cpu,
> >> SpaprMachineState *spapr,
> >> ret = h_set_mode_resource_le(cpu, spapr, args[0], args[2],
> >> args[3]);
> >> break;
> >> case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
> >> - ret = h_set_mode_resource_addr_trans_mode(cpu, args[0],
> >> + ret = h_set_mode_resource_addr_trans_mode(cpu, spapr, args[0],
> >> args[2], args[3]);
> >> break;
> >> }
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index ee7504b976..edbf3eeed0 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -77,8 +77,10 @@ typedef enum {
> >> #define SPAPR_CAP_FWNMI 0x0A
> >> /* Support H_RPT_INVALIDATE */
> >> #define SPAPR_CAP_RPT_INVALIDATE 0x0B
> >> +/* Support for AIL modes */
> >> +#define SPAPR_CAP_AIL_MODE_3 0x0C
> >> /* Num Caps */
> >> -#define SPAPR_CAP_NUM (SPAPR_CAP_RPT_INVALIDATE + 1)
> >> +#define SPAPR_CAP_NUM (SPAPR_CAP_AIL_MODE_3 + 1)
> >>
> >> /*
> >> * Capability Values
> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> >> index dc93b99189..128bc530d4 100644
> >> --- a/target/ppc/kvm.c
> >> +++ b/target/ppc/kvm.c
> >> @@ -2563,6 +2563,29 @@ int kvmppc_has_cap_rpt_invalidate(void)
> >> return cap_rpt_invalidate;
> >> }
> >>
> >> +int kvmppc_supports_ail_3(void)
> >
> > Returning bool would make more sense, no?
>
> It would.
>
> >
> >> +{
> >> + 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, but it's not clear how to cover those
> >> + * without disabling the feature for many.
> >> + */
> >> + if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
> >
> > This effectively means that the pseries machine type simply won't
> > start with a !PPC2_ISA207S cpu model. I'm not sure if we support any
> > such CPUs in any case.
>
> Oh, would that at least give an error to disable cap-ail-mode-3?
Yes, it should. Which for something as obscure as POWER7 may be acceptable.
> > If we do, we should probably change the
> > default value for this cap based on cpu model in
> > default_caps_with_cpu().
>
> We allegedly still support POWER7 KVM in Linux. I've never tested it
> and I don't know how much it's used at all. Probably should keep it
> working here if possible. I'll look into default_caps_with_cpu().
>
> 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
