[Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: Stop parsing pvr list in H_CAS when exact match found

[Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: Stop parsing pvr list in H_CAS when exact match found

[Suraj Jitindar Singh]

The pvr-list passed in H_Client_Architecture_Support is used to
communicate the supported pvrs of the client program. When an
exact match is found you are allowed to stop parsing the list and continue
the boot process.

Currently while explicit_match is set when we find an exact match, we still
set a compat mode based on best_compat irrespective of whether an exact
match was found or not. This is wrong since it means we can only ever run
in an architected state, not a raw state since we always set a compat mode.
We are basically ignoring the case were we find an exact match.

Fix the code to stop parsing the pvr list when an exact match is found.
This means that best_compat will always be zero in the case of an exact
match which means we will not set a compat mode an thus run in raw mode,
which is the desired functionality when we have an exact match.

Fixes: 152ef803ceb1 ("pseries: Rewrite CAS PVR compatibility logic")

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

---
 hw/ppc/spapr_hcall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 590105a..215c385 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1028,6 +1028,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
 
         if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask)) {
             explicit_match = true;
+            break;
         } else {
             if (ppc_check_compat(cpu, pvr, best_compat, max_compat)) {
                 best_compat = pvr;
-- 
2.5.5

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: Stop parsing pvr list in H_CAS when exact match found

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2234 bytes --]

On Mon, Feb 13, 2017 at 02:30:27PM +1100, Suraj Jitindar Singh wrote:
> The pvr-list passed in H_Client_Architecture_Support is used to
> communicate the supported pvrs of the client program. When an
> exact match is found you are allowed to stop parsing the list and continue
> the boot process.
> 
> Currently while explicit_match is set when we find an exact match, we still
> set a compat mode based on best_compat irrespective of whether an exact
> match was found or not. This is wrong since it means we can only ever run
> in an architected state, not a raw state since we always set a compat mode.
> We are basically ignoring the case were we find an exact match.
> 
> Fix the code to stop parsing the pvr list when an exact match is found.
> This means that best_compat will always be zero in the case of an exact
> match which means we will not set a compat mode an thus run in raw mode,
> which is the desired functionality when we have an exact match.
> 
> Fixes: 152ef803ceb1 ("pseries: Rewrite CAS PVR compatibility logic")
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Nack.  That change was deliberately intended to prefer compatibility
modes, only ever using a "raw" mode if there's no matching
compatibility mode.

Using compatibility modes as often as possible makes migration across
different host types substantially easier to handle and more likely to
succeed.

> 
> ---
>  hw/ppc/spapr_hcall.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 590105a..215c385 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1028,6 +1028,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>  
>          if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask)) {
>              explicit_match = true;
> +            break;
>          } else {
>              if (ppc_check_compat(cpu, pvr, best_compat, max_compat)) {
>                  best_compat = pvr;

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: Stop parsing pvr list in H_CAS when exact match found

[Suraj Jitindar Singh]

On Mon, 2017-02-13 at 15:10 +1100, David Gibson wrote:
> On Mon, Feb 13, 2017 at 02:30:27PM +1100, Suraj Jitindar Singh wrote:
> > 
> > The pvr-list passed in H_Client_Architecture_Support is used to
> > communicate the supported pvrs of the client program. When an
> > exact match is found you are allowed to stop parsing the list and
> > continue
> > the boot process.
> > 
> > Currently while explicit_match is set when we find an exact match,
> > we still
> > set a compat mode based on best_compat irrespective of whether an
> > exact
> > match was found or not. This is wrong since it means we can only
> > ever run
> > in an architected state, not a raw state since we always set a
> > compat mode.
> > We are basically ignoring the case were we find an exact match.
> > 
> > Fix the code to stop parsing the pvr list when an exact match is
> > found.
> > This means that best_compat will always be zero in the case of an
> > exact
> > match which means we will not set a compat mode an thus run in raw
> > mode,
> > which is the desired functionality when we have an exact match.
> > 
> > Fixes: 152ef803ceb1 ("pseries: Rewrite CAS PVR compatibility
> > logic")
> > 
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Nack.  That change was deliberately intended to prefer compatibility
> modes, only ever using a "raw" mode if there's no matching
> compatibility mode.
> 
> Using compatibility modes as often as possible makes migration across
> different host types substantially easier to handle and more likely
> to
> succeed.
Ok cool, didn't realise that was an intended effect. Guess I'll just
add power9 to the compat table.
> 
> > 
> > 
> > ---
> >  hw/ppc/spapr_hcall.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 590105a..215c385 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1028,6 +1028,7 @@ static target_ulong
> > h_client_architecture_support(PowerPCCPU *cpu,
> >  
> >          if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr &
> > pvr_mask)) {
> >              explicit_match = true;
> > +            break;
> >          } else {
> >              if (ppc_check_compat(cpu, pvr, best_compat,
> > max_compat)) {
> >                  best_compat = pvr;