| Message ID | 1477825928-10803-11-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
On 30/10/16 22:12, David Gibson wrote: > 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 <david@gibson.dropbear.id.au> > --- > 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) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \ > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \ > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \ > - ((cpuver) == 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 = get_compat_level(pvr); > - bool is205, is206, is207; > - > - if (!lvl) { > - return; > - } > - > - /* If it is a logical PVR, try to determine the highest level */ > - is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) && > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05)); > - is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) && > - ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) || > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS))); > - is207 = (pcc->pcr_supported & PCR_COMPAT_2_07) && > - (lvl == 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 <= lvl) { > - *compat_lvl = lvl; > - *compat_pvr = pvr; > - } > - } else if (max_lvl >= lvl) { > - /* User chose the level, don't set higher than this */ > - *compat_lvl = lvl; > - *compat_pvr = pvr; > - } > - } > -} > - > -static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, > +static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > sPAPRMachineState *spapr, > target_ulong opcode, > target_ulong *args) > { > target_ulong list = ppc64_phys_to_real(args[0]); > target_ulong ov_table; > - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_); > CPUState *cs; > - bool cpu_match = false; > - unsigned old_compat_pvr = cpu_->compat_pvr; > - unsigned compat_lvl = 0, compat_pvr = 0; > - unsigned max_lvl = get_compat_level(cpu_->max_compat); > - int counter; > + bool explicit_match = false; /* Matched the CPU's real PVR */ > + uint32_t max_compat = cpu->max_compat; > + uint32_t best_compat = 0; > + int i; > sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates; > > - /* Parse PVR list */ > - for (counter = 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 = 0; i < 512; ++i) { > uint32_t pvr, pvr_mask; > > - pvr_mask = ldl_be_phys(&address_space_memory, list); > - list += 4; > pvr = ldl_be_phys(&address_space_memory, list); LoPAPR: List-entry = 4-byte-mask : 4-byte-PVR-value This patch changes the order and "cas" fails. > - list += 4; > - > - trace_spapr_cas_pvr_try(pvr); > - if (!max_lvl && > - ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) { > - cpu_match = true; > - compat_pvr = 0; > - } else if (pvr == cpu_->compat_pvr) { > - cpu_match = true; > - compat_pvr = cpu_->compat_pvr; > - } else if (!cpu_match) { > - cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &compat_pvr); > - } > - /* Terminator record */ > + pvr_mask = ldl_be_phys(&address_space_memory, list + 4); > + list += 8; > + > if (~pvr_mask & pvr) { > - break; > + break; /* Terminator record */ > } > + > + if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask)) { > + explicit_match = true; > + } else { > + if (ppc_check_compat(cpu, pvr, best_compat, max_compat)) { > + best_compat = 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 = 0; > + } else if (best_compat == 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 != compat_pvr) { > + if (cpu->compat_pvr != best_compat) { > CPU_FOREACH(cs) { > SetCompatState s = { > - .compat_pvr = compat_pvr, > + .compat_pvr = best_compat, > .err = 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 the 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=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64 > +spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=%x, explicit_match=%u, new=%x" > > # hw/ppc/spapr_iommu.c > spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64 >
On Mon, Oct 31, 2016 at 04:00:14PM +1100, Alexey Kardashevskiy wrote: > On 30/10/16 22:12, David Gibson wrote: > > 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 <david@gibson.dropbear.id.au> > > --- > > 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) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \ > > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \ > > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \ > > - ((cpuver) == 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 = get_compat_level(pvr); > > - bool is205, is206, is207; > > - > > - if (!lvl) { > > - return; > > - } > > - > > - /* If it is a logical PVR, try to determine the highest level */ > > - is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) && > > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05)); > > - is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) && > > - ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) || > > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS))); > > - is207 = (pcc->pcr_supported & PCR_COMPAT_2_07) && > > - (lvl == 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 <= lvl) { > > - *compat_lvl = lvl; > > - *compat_pvr = pvr; > > - } > > - } else if (max_lvl >= lvl) { > > - /* User chose the level, don't set higher than this */ > > - *compat_lvl = lvl; > > - *compat_pvr = pvr; > > - } > > - } > > -} > > - > > -static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, > > +static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > > sPAPRMachineState *spapr, > > target_ulong opcode, > > target_ulong *args) > > { > > target_ulong list = ppc64_phys_to_real(args[0]); > > target_ulong ov_table; > > - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_); > > CPUState *cs; > > - bool cpu_match = false; > > - unsigned old_compat_pvr = cpu_->compat_pvr; > > - unsigned compat_lvl = 0, compat_pvr = 0; > > - unsigned max_lvl = get_compat_level(cpu_->max_compat); > > - int counter; > > + bool explicit_match = false; /* Matched the CPU's real PVR */ > > + uint32_t max_compat = cpu->max_compat; > > + uint32_t best_compat = 0; > > + int i; > > sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates; > > > > - /* Parse PVR list */ > > - for (counter = 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 = 0; i < 512; ++i) { > > uint32_t pvr, pvr_mask; > > > > - pvr_mask = ldl_be_phys(&address_space_memory, list); > > - list += 4; > > pvr = ldl_be_phys(&address_space_memory, list); > > > LoPAPR: List-entry = 4-byte-mask : 4-byte-PVR-value > > This patch changes the order and "cas" fails. Oops, yes, thanks for the catch.
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 <david@gibson.dropbear.id.au> > --- > 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) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \ > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \ > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \ > - ((cpuver) == 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 = get_compat_level(pvr); > - bool is205, is206, is207; > - > - if (!lvl) { > - return; > - } > - > - /* If it is a logical PVR, try to determine the highest level */ > - is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) && > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05)); > - is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) && > - ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) || > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS))); > - is207 = (pcc->pcr_supported & PCR_COMPAT_2_07) && > - (lvl == 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 <= lvl) { > - *compat_lvl = lvl; > - *compat_pvr = pvr; > - } > - } else if (max_lvl >= lvl) { > - /* User chose the level, don't set higher than this */ > - *compat_lvl = lvl; > - *compat_pvr = pvr; > - } > - } > -} > - > -static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, > +static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > sPAPRMachineState *spapr, > target_ulong opcode, > target_ulong *args) > { > target_ulong list = ppc64_phys_to_real(args[0]); > target_ulong ov_table; > - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_); > CPUState *cs; > - bool cpu_match = false; > - unsigned old_compat_pvr = cpu_->compat_pvr; > - unsigned compat_lvl = 0, compat_pvr = 0; > - unsigned max_lvl = get_compat_level(cpu_->max_compat); > - int counter; > + bool explicit_match = false; /* Matched the CPU's real PVR */ > + uint32_t max_compat = cpu->max_compat; > + uint32_t best_compat = 0; > + int i; > sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates; > > - /* Parse PVR list */ > - for (counter = 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 = 0; i < 512; ++i) { > uint32_t pvr, pvr_mask; > > - pvr_mask = ldl_be_phys(&address_space_memory, list); > - list += 4; > pvr = ldl_be_phys(&address_space_memory, list); > - list += 4; > - > - trace_spapr_cas_pvr_try(pvr); > - if (!max_lvl && > - ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) { > - cpu_match = true; > - compat_pvr = 0; > - } else if (pvr == cpu_->compat_pvr) { > - cpu_match = true; > - compat_pvr = cpu_->compat_pvr; > - } else if (!cpu_match) { > - cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &compat_pvr); > - } > - /* Terminator record */ > + pvr_mask = ldl_be_phys(&address_space_memory, list + 4); > + list += 8; > + > if (~pvr_mask & pvr) { > - break; > + break; /* Terminator record */ > } > + > + if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask)) { > + explicit_match = 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 = 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 = 0; > + } else if (best_compat == 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 != compat_pvr) { > + if (cpu->compat_pvr != best_compat) { > CPU_FOREACH(cs) { > SetCompatState s = { > - .compat_pvr = compat_pvr, > + .compat_pvr = best_compat, > .err = 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 the 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=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64 > +spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=%x, explicit_match=%u, new=%x" > > # hw/ppc/spapr_iommu.c > spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64 > -- > 2.7.4 >
On Thu, Nov 10, 2016 at 11:54:13AM -0600, Michael Roth wrote: > 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 <david@gibson.dropbear.id.au> > > --- > > 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) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \ > > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \ > > - ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \ > > - ((cpuver) == 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 = get_compat_level(pvr); > > - bool is205, is206, is207; > > - > > - if (!lvl) { > > - return; > > - } > > - > > - /* If it is a logical PVR, try to determine the highest level */ > > - is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) && > > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05)); > > - is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) && > > - ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) || > > - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS))); > > - is207 = (pcc->pcr_supported & PCR_COMPAT_2_07) && > > - (lvl == 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 <= lvl) { > > - *compat_lvl = lvl; > > - *compat_pvr = pvr; > > - } > > - } else if (max_lvl >= lvl) { > > - /* User chose the level, don't set higher than this */ > > - *compat_lvl = lvl; > > - *compat_pvr = pvr; > > - } > > - } > > -} > > - > > -static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, > > +static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > > sPAPRMachineState *spapr, > > target_ulong opcode, > > target_ulong *args) > > { > > target_ulong list = ppc64_phys_to_real(args[0]); > > target_ulong ov_table; > > - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_); > > CPUState *cs; > > - bool cpu_match = false; > > - unsigned old_compat_pvr = cpu_->compat_pvr; > > - unsigned compat_lvl = 0, compat_pvr = 0; > > - unsigned max_lvl = get_compat_level(cpu_->max_compat); > > - int counter; > > + bool explicit_match = false; /* Matched the CPU's real PVR */ > > + uint32_t max_compat = cpu->max_compat; > > + uint32_t best_compat = 0; > > + int i; > > sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates; > > > > - /* Parse PVR list */ > > - for (counter = 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 = 0; i < 512; ++i) { > > uint32_t pvr, pvr_mask; > > > > - pvr_mask = ldl_be_phys(&address_space_memory, list); > > - list += 4; > > pvr = ldl_be_phys(&address_space_memory, list); > > - list += 4; > > - > > - trace_spapr_cas_pvr_try(pvr); > > - if (!max_lvl && > > - ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) { > > - cpu_match = true; > > - compat_pvr = 0; > > - } else if (pvr == cpu_->compat_pvr) { > > - cpu_match = true; > > - compat_pvr = cpu_->compat_pvr; > > - } else if (!cpu_match) { > > - cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &compat_pvr); > > - } > > - /* Terminator record */ > > + pvr_mask = ldl_be_phys(&address_space_memory, list + 4); > > + list += 8; > > + > > if (~pvr_mask & pvr) { > > - break; > > + break; /* Terminator record */ > > } > > + > > + if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask)) { > > + explicit_match = 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. I guess it's theoretically possible. But that would only happen if the guest advertised a pvr+mask which covered both a valid real PVR and a valid compatibility PVR. Since the logical PVRs are in a wholly different range and don't look like real PVR values at all, I think that's vanishingly unlikely. > > > + } else { > > + if (ppc_check_compat(cpu, pvr, best_compat, max_compat)) { > > + best_compat = 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 = 0; > > + } else if (best_compat == 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 != compat_pvr) { > > + if (cpu->compat_pvr != best_compat) { > > CPU_FOREACH(cs) { > > SetCompatState s = { > > - .compat_pvr = compat_pvr, > > + .compat_pvr = best_compat, > > .err = 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 the 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=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64 > > +spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=%x, explicit_match=%u, new=%x" > > > > # hw/ppc/spapr_iommu.c > > spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64 >
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) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \ - ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \ - ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \ - ((cpuver) == 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 = get_compat_level(pvr); - bool is205, is206, is207; - - if (!lvl) { - return; - } - - /* If it is a logical PVR, try to determine the highest level */ - is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) && - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05)); - is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) && - ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) || - (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS))); - is207 = (pcc->pcr_supported & PCR_COMPAT_2_07) && - (lvl == 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 <= lvl) { - *compat_lvl = lvl; - *compat_pvr = pvr; - } - } else if (max_lvl >= lvl) { - /* User chose the level, don't set higher than this */ - *compat_lvl = lvl; - *compat_pvr = pvr; - } - } -} - -static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, +static target_ulong h_client_architecture_support(PowerPCCPU *cpu, sPAPRMachineState *spapr, target_ulong opcode, target_ulong *args) { target_ulong list = ppc64_phys_to_real(args[0]); target_ulong ov_table; - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_); CPUState *cs; - bool cpu_match = false; - unsigned old_compat_pvr = cpu_->compat_pvr; - unsigned compat_lvl = 0, compat_pvr = 0; - unsigned max_lvl = get_compat_level(cpu_->max_compat); - int counter; + bool explicit_match = false; /* Matched the CPU's real PVR */ + uint32_t max_compat = cpu->max_compat; + uint32_t best_compat = 0; + int i; sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates; - /* Parse PVR list */ - for (counter = 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 = 0; i < 512; ++i) { uint32_t pvr, pvr_mask; - pvr_mask = ldl_be_phys(&address_space_memory, list); - list += 4; pvr = ldl_be_phys(&address_space_memory, list); - list += 4; - - trace_spapr_cas_pvr_try(pvr); - if (!max_lvl && - ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) { - cpu_match = true; - compat_pvr = 0; - } else if (pvr == cpu_->compat_pvr) { - cpu_match = true; - compat_pvr = cpu_->compat_pvr; - } else if (!cpu_match) { - cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &compat_pvr); - } - /* Terminator record */ + pvr_mask = ldl_be_phys(&address_space_memory, list + 4); + list += 8; + if (~pvr_mask & pvr) { - break; + break; /* Terminator record */ } + + if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask)) { + explicit_match = true; + } else { + if (ppc_check_compat(cpu, pvr, best_compat, max_compat)) { + best_compat = 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 = 0; + } else if (best_compat == 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 != compat_pvr) { + if (cpu->compat_pvr != best_compat) { CPU_FOREACH(cs) { SetCompatState s = { - .compat_pvr = compat_pvr, + .compat_pvr = best_compat, .err = 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 the 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=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64 +spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=%x, explicit_match=%u, new=%x" # hw/ppc/spapr_iommu.c spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
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 <david@gibson.dropbear.id.au> --- hw/ppc/spapr_hcall.c | 107 +++++++++++++++++---------------------------------- hw/ppc/trace-events | 2 +- 2 files changed, 37 insertions(+), 72 deletions(-)