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?
> 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
