From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40146) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4tYV-0006Pd-M3 for qemu-devel@nongnu.org; Thu, 10 Nov 2016 12:54:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4tYS-00047Z-Id for qemu-devel@nongnu.org; Thu, 10 Nov 2016 12:54:31 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:36580) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c4tYS-00046x-A0 for qemu-devel@nongnu.org; Thu, 10 Nov 2016 12:54:28 -0500 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uAAHsQtY095985 for ; Thu, 10 Nov 2016 12:54:26 -0500 Received: from e33.co.us.ibm.com (e33.co.us.ibm.com [32.97.110.151]) by mx0a-001b2d01.pphosted.com with ESMTP id 26mwc69ppe-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 10 Nov 2016 12:54:26 -0500 Received: from localhost by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 10 Nov 2016 10:54:22 -0700 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1477825928-10803-11-git-send-email-david@gibson.dropbear.id.au> References: <1477825928-10803-1-git-send-email-david@gibson.dropbear.id.au> <1477825928-10803-11-git-send-email-david@gibson.dropbear.id.au> Date: Thu, 10 Nov 2016 11:54:13 -0600 Message-Id: <20161110175413.32249.77477@loki> Subject: Re: [Qemu-devel] [RFC 10/17] pseries: Rewrite CAS PVR compatibility logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , nikunj@linux.vnet.ibm.com, aik@ozlabs.ru Cc: thuth@redhat.com, lvivier@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org Quoting David Gibson (2016-10-30 06:12:01) > During boot, PAPR guests negotiate CPU model support with the > ibm,client-architecture-support mechanism. The logic to implement this in > qemu is very convoluted. This cleans it up to be cleaner, using the new > ppc_check_compat() call. > = > The new logic for choosing a compatibility mode is: > 1. If the guest lists the CPU's real PVR as supported *AND* no > maximum compatibility mode has been requested on the command line > then we use "raw" mode - the CPU acts with full capabilities. > 2. Otherwise, we pick the most recent compatibility mode which is > both supported by the CPU, and is advertised as supported by the > guest. > I think the original code approximated the same thing, but it's hard to be > sure, and I think it had some weird edge cases. > = > Signed-off-by: David Gibson > --- > hw/ppc/spapr_hcall.c | 107 +++++++++++++++++----------------------------= ------ > hw/ppc/trace-events | 2 +- > 2 files changed, 37 insertions(+), 72 deletions(-) > = > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index d93f580..3bd6d06 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -895,98 +895,63 @@ static void do_set_compat(CPUState *cs, void *arg) > ppc_set_compat(cpu, s->compat_pvr, &s->err); > } > = > -#define get_compat_level(cpuver) ( \ > - ((cpuver) =3D=3D CPU_POWERPC_LOGICAL_2_05) ? 2050 : \ > - ((cpuver) =3D=3D CPU_POWERPC_LOGICAL_2_06) ? 2060 : \ > - ((cpuver) =3D=3D CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \ > - ((cpuver) =3D=3D CPU_POWERPC_LOGICAL_2_07) ? 2070 : 0) > - > -static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr, > - unsigned max_lvl, unsigned *compat_lvl, > - unsigned *compat_pvr) > -{ > - unsigned lvl =3D get_compat_level(pvr); > - bool is205, is206, is207; > - > - if (!lvl) { > - return; > - } > - > - /* If it is a logical PVR, try to determine the highest level */ > - is205 =3D (pcc->pcr_supported & PCR_COMPAT_2_05) && > - (lvl =3D=3D get_compat_level(CPU_POWERPC_LOGICAL_2_05)); > - is206 =3D (pcc->pcr_supported & PCR_COMPAT_2_06) && > - ((lvl =3D=3D get_compat_level(CPU_POWERPC_LOGICAL_2_06)) || > - (lvl =3D=3D get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)= )); > - is207 =3D (pcc->pcr_supported & PCR_COMPAT_2_07) && > - (lvl =3D=3D get_compat_level(CPU_POWERPC_LOGICAL_2_07)); > - > - if (is205 || is206 || is207) { > - if (!max_lvl) { > - /* User did not set the level, choose the highest */ > - if (*compat_lvl <=3D lvl) { > - *compat_lvl =3D lvl; > - *compat_pvr =3D pvr; > - } > - } else if (max_lvl >=3D lvl) { > - /* User chose the level, don't set higher than this */ > - *compat_lvl =3D lvl; > - *compat_pvr =3D pvr; > - } > - } > -} > - > -static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, > +static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > sPAPRMachineState *spa= pr, > target_ulong opcode, > target_ulong *args) > { > target_ulong list =3D ppc64_phys_to_real(args[0]); > target_ulong ov_table; > - PowerPCCPUClass *pcc =3D POWERPC_CPU_GET_CLASS(cpu_); > CPUState *cs; > - bool cpu_match =3D false; > - unsigned old_compat_pvr =3D cpu_->compat_pvr; > - unsigned compat_lvl =3D 0, compat_pvr =3D 0; > - unsigned max_lvl =3D get_compat_level(cpu_->max_compat); > - int counter; > + bool explicit_match =3D false; /* Matched the CPU's real PVR */ > + uint32_t max_compat =3D cpu->max_compat; > + uint32_t best_compat =3D 0; > + int i; > sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates; > = > - /* Parse PVR list */ > - for (counter =3D 0; counter < 512; ++counter) { > + /* > + * We scan the supplied table of PVRs looking for two things > + * 1. Is our real CPU PVR in the list? > + * 2. What's the "best" listed logical PVR > + */ > + for (i =3D 0; i < 512; ++i) { > uint32_t pvr, pvr_mask; > = > - pvr_mask =3D ldl_be_phys(&address_space_memory, list); > - list +=3D 4; > pvr =3D ldl_be_phys(&address_space_memory, list); > - list +=3D 4; > - > - trace_spapr_cas_pvr_try(pvr); > - if (!max_lvl && > - ((cpu_->env.spr[SPR_PVR] & pvr_mask) =3D=3D (pvr & pvr_mask)= )) { > - cpu_match =3D true; > - compat_pvr =3D 0; > - } else if (pvr =3D=3D cpu_->compat_pvr) { > - cpu_match =3D true; > - compat_pvr =3D cpu_->compat_pvr; > - } else if (!cpu_match) { > - cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &compa= t_pvr); > - } > - /* Terminator record */ > + pvr_mask =3D ldl_be_phys(&address_space_memory, list + 4); > + list +=3D 8; > + > if (~pvr_mask & pvr) { > - break; > + break; /* Terminator record */ > } > + > + if ((cpu->env.spr[SPR_PVR] & pvr_mask) =3D=3D (pvr & pvr_mask)) { > + explicit_match =3D true; The previous code only did this check if max_lvl/max_compat was set. I'm not sure if it's possible in practice, but in theory this PVR might also happen to be the next best_compat PVR, but since we test positive here we may not not set best_compat accordingly. > + } else { > + if (ppc_check_compat(cpu, pvr, best_compat, max_compat)) { > + best_compat =3D pvr; > + } > + } > + } > + > + if (!max_compat && explicit_match) { > + /* If the guest explicitly supports the CPU, *and* user hasn't > + * requested a compatibility mode, use "raw" mode */ > + best_compat =3D 0; > + } else if (best_compat =3D=3D 0) { > + /* Didn't find any supported compat modes */ > + /* FIXME: what's the right error here? */ > + return H_HARDWARE; > } > = > /* Parsing finished */ > - trace_spapr_cas_pvr(cpu_->compat_pvr, cpu_match, > - compat_pvr, pcc->pcr_mask); > + trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat); > = > /* Update CPUs */ > - if (old_compat_pvr !=3D compat_pvr) { > + if (cpu->compat_pvr !=3D best_compat) { > CPU_FOREACH(cs) { > SetCompatState s =3D { > - .compat_pvr =3D compat_pvr, > + .compat_pvr =3D best_compat, > .err =3D NULL, > }; > = > diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events > index 2297ead..604ac92 100644 > --- a/hw/ppc/trace-events > +++ b/hw/ppc/trace-events > @@ -15,7 +15,7 @@ spapr_cas_continue(unsigned long n) "Copy changes to th= e guest: %ld bytes" > = > # hw/ppc/spapr_hcall.c > spapr_cas_pvr_try(uint32_t pvr) "%x" > -spapr_cas_pvr(uint32_t cur_pvr, bool cpu_match, uint32_t new_pvr, uint64= _t pcr) "current=3D%x, cpu_match=3D%u, new=3D%x, compat flags=3D%"PRIx64 > +spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "= current=3D%x, explicit_match=3D%u, new=3D%x" > = > # hw/ppc/spapr_iommu.c > spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t re= t) "liobn=3D%"PRIx64" ioba=3D0x%"PRIx64" tce=3D0x%"PRIx64" ret=3D%"PRId64 > -- = > 2.7.4 >=20