PAPR allows the interrupt controller used on a POWER9 machine (XICS or
XIVE) to be selected by the guest operating system, by using the
ibm,client-architecture-support (CAS) feature negotiation call.

Currently, if the guest selects an interrupt controller different from the
one selected at initial boot, this causes the system to be reset with the
new model and the boot starts again.  This means we run through the SLOF
boot process twice, as well as any other bootloader (e.g. grub) in use
before the OS calls CAS.  This can be confusing and/or inconvenient for
users.

Thanks to two fairly recent changes, we no longer need this reboot.  1) we
now completely regenerate the device tree when CAS is called (meaning we
don't need special case updates for all the device tree changes caused by
the interrupt controller mode change),  2) we now have explicit code paths
to activate and deactivate the different interrupt controllers, rather than
just implicitly calling those at machine reset time.

We can therefore eliminate the reboot for changing irq mode, simply by
putting a called to spapr_irq_update_active_intc() before we call
spapr_h_cas_compose_response() (which gives the updated device tree to the
guest firmware and OS).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_hcall.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 140f05c1c6..05a7ca275b 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1767,21 +1767,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     }
     spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
     spapr_ovec_cleanup(ov1_guest);
-    if (!spapr->cas_reboot) {
-        /* If spapr_machine_reset() did not set up a HPT but one is necessary
-         * (because the guest isn't going to use radix) then set it up here. */
-        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
-            /* legacy hash or new hash: */
-            spapr_setup_hpt_and_vrma(spapr);
-        }
-        spapr->cas_reboot =
-            (spapr_h_cas_compose_response(spapr, args[1], args[2],
-                                          ov5_updates) != 0);
-    }
 
     /*
-     * Ensure the guest asks for an interrupt mode we support; otherwise
-     * terminate the boot.
+     * Ensure the guest asks for an interrupt mode we support;
+     * otherwise terminate the boot.
      */
     if (guest_xive) {
         if (!spapr->irq->xive) {
@@ -1797,14 +1786,18 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
         }
     }
 
-    /*
-     * Generate a machine reset when we have an update of the
-     * interrupt mode. Only required when the machine supports both
-     * modes.
-     */
+    spapr_irq_update_active_intc(spapr);
+
     if (!spapr->cas_reboot) {
-        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
-            && spapr->irq->xics && spapr->irq->xive;
+        /* If spapr_machine_reset() did not set up a HPT but one is necessary
+         * (because the guest isn't going to use radix) then set it up here. */
+        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
+            /* legacy hash or new hash: */
+            spapr_setup_hpt_and_vrma(spapr);
+        }
+        spapr->cas_reboot =
+            (spapr_h_cas_compose_response(spapr, args[1], args[2],
+                                          ov5_updates) != 0);
     }
 
     spapr_ovec_cleanup(ov5_updates);
-- 
2.23.0

On 29/11/2019 06:33, David Gibson wrote:
> PAPR allows the interrupt controller used on a POWER9 machine (XICS or
> XIVE) to be selected by the guest operating system, by using the
> ibm,client-architecture-support (CAS) feature negotiation call.
> 
> Currently, if the guest selects an interrupt controller different from the
> one selected at initial boot, this causes the system to be reset with the
> new model and the boot starts again.  This means we run through the SLOF
> boot process twice, as well as any other bootloader (e.g. grub) in use
> before the OS calls CAS.  This can be confusing and/or inconvenient for
> users.
> 
> Thanks to two fairly recent changes, we no longer need this reboot.  1) we
> now completely regenerate the device tree when CAS is called (meaning we
> don't need special case updates for all the device tree changes caused by
> the interrupt controller mode change),  2) we now have explicit code paths
> to activate and deactivate the different interrupt controllers, rather than
> just implicitly calling those at machine reset time.
> 
> We can therefore eliminate the reboot for changing irq mode, simply by
> putting a called to spapr_irq_update_active_intc() before we call
> spapr_h_cas_compose_response() (which gives the updated device tree to the
> guest firmware and OS).
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Reviewed-by: Cedric Le Goater <clg@fr.ibm.com>

> ---
>  hw/ppc/spapr_hcall.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 140f05c1c6..05a7ca275b 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1767,21 +1767,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      }
>      spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
>      spapr_ovec_cleanup(ov1_guest);
> -    if (!spapr->cas_reboot) {
> -        /* If spapr_machine_reset() did not set up a HPT but one is necessary
> -         * (because the guest isn't going to use radix) then set it up here. */
> -        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
> -            /* legacy hash or new hash: */
> -            spapr_setup_hpt_and_vrma(spapr);
> -        }
> -        spapr->cas_reboot =
> -            (spapr_h_cas_compose_response(spapr, args[1], args[2],
> -                                          ov5_updates) != 0);
> -    }
>  
>      /*
> -     * Ensure the guest asks for an interrupt mode we support; otherwise
> -     * terminate the boot.
> +     * Ensure the guest asks for an interrupt mode we support;
> +     * otherwise terminate the boot.
>       */
>      if (guest_xive) {
>          if (!spapr->irq->xive) {
> @@ -1797,14 +1786,18 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>          }
>      }
>  
> -    /*
> -     * Generate a machine reset when we have an update of the
> -     * interrupt mode. Only required when the machine supports both
> -     * modes.
> -     */
> +    spapr_irq_update_active_intc(spapr);
> +
>      if (!spapr->cas_reboot) {
> -        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> -            && spapr->irq->xics && spapr->irq->xive;
> +        /* If spapr_machine_reset() did not set up a HPT but one is necessary
> +         * (because the guest isn't going to use radix) then set it up here. */
> +        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
> +            /* legacy hash or new hash: */
> +            spapr_setup_hpt_and_vrma(spapr);
> +        }
> +        spapr->cas_reboot =
> +            (spapr_h_cas_compose_response(spapr, args[1], args[2],
> +                                          ov5_updates) != 0);
>      }
>  
>      spapr_ovec_cleanup(ov5_updates);
> 

On Fri, 29 Nov 2019 16:33:53 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> PAPR allows the interrupt controller used on a POWER9 machine (XICS or
> XIVE) to be selected by the guest operating system, by using the
> ibm,client-architecture-support (CAS) feature negotiation call.
> 
> Currently, if the guest selects an interrupt controller different from the
> one selected at initial boot, this causes the system to be reset with the
> new model and the boot starts again.  This means we run through the SLOF
> boot process twice, as well as any other bootloader (e.g. grub) in use
> before the OS calls CAS.  This can be confusing and/or inconvenient for
> users.
> 
> Thanks to two fairly recent changes, we no longer need this reboot.  1) we
> now completely regenerate the device tree when CAS is called (meaning we
> don't need special case updates for all the device tree changes caused by
> the interrupt controller mode change),  2) we now have explicit code paths
> to activate and deactivate the different interrupt controllers, rather than
> just implicitly calling those at machine reset time.
> 
> We can therefore eliminate the reboot for changing irq mode, simply by
> putting a called to spapr_irq_update_active_intc() before we call

putting a call

> spapr_h_cas_compose_response() (which gives the updated device tree to the
> guest firmware and OS).
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Convenient indeed ! :)

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_hcall.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 140f05c1c6..05a7ca275b 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1767,21 +1767,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      }
>      spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
>      spapr_ovec_cleanup(ov1_guest);
> -    if (!spapr->cas_reboot) {
> -        /* If spapr_machine_reset() did not set up a HPT but one is necessary
> -         * (because the guest isn't going to use radix) then set it up here. */
> -        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
> -            /* legacy hash or new hash: */
> -            spapr_setup_hpt_and_vrma(spapr);
> -        }
> -        spapr->cas_reboot =
> -            (spapr_h_cas_compose_response(spapr, args[1], args[2],
> -                                          ov5_updates) != 0);
> -    }
>  
>      /*
> -     * Ensure the guest asks for an interrupt mode we support; otherwise
> -     * terminate the boot.
> +     * Ensure the guest asks for an interrupt mode we support;
> +     * otherwise terminate the boot.
>       */
>      if (guest_xive) {
>          if (!spapr->irq->xive) {
> @@ -1797,14 +1786,18 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>          }
>      }
>  
> -    /*
> -     * Generate a machine reset when we have an update of the
> -     * interrupt mode. Only required when the machine supports both
> -     * modes.
> -     */
> +    spapr_irq_update_active_intc(spapr);
> +
>      if (!spapr->cas_reboot) {
> -        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> -            && spapr->irq->xics && spapr->irq->xive;
> +        /* If spapr_machine_reset() did not set up a HPT but one is necessary
> +         * (because the guest isn't going to use radix) then set it up here. */
> +        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
> +            /* legacy hash or new hash: */
> +            spapr_setup_hpt_and_vrma(spapr);
> +        }
> +        spapr->cas_reboot =
> +            (spapr_h_cas_compose_response(spapr, args[1], args[2],
> +                                          ov5_updates) != 0);
>      }
>  
>      spapr_ovec_cleanup(ov5_updates);

On Mon, Dec 02, 2019 at 08:50:09AM +0100, Greg Kurz wrote:
> On Fri, 29 Nov 2019 16:33:53 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > PAPR allows the interrupt controller used on a POWER9 machine (XICS or
> > XIVE) to be selected by the guest operating system, by using the
> > ibm,client-architecture-support (CAS) feature negotiation call.
> > 
> > Currently, if the guest selects an interrupt controller different from the
> > one selected at initial boot, this causes the system to be reset with the
> > new model and the boot starts again.  This means we run through the SLOF
> > boot process twice, as well as any other bootloader (e.g. grub) in use
> > before the OS calls CAS.  This can be confusing and/or inconvenient for
> > users.
> > 
> > Thanks to two fairly recent changes, we no longer need this reboot.  1) we
> > now completely regenerate the device tree when CAS is called (meaning we
> > don't need special case updates for all the device tree changes caused by
> > the interrupt controller mode change),  2) we now have explicit code paths
> > to activate and deactivate the different interrupt controllers, rather than
> > just implicitly calling those at machine reset time.
> > 
> > We can therefore eliminate the reboot for changing irq mode, simply by
> > putting a called to spapr_irq_update_active_intc() before we call
> 
> putting a call

Fixed, thanks.

> > spapr_h_cas_compose_response() (which gives the updated device tree to the
> > guest firmware and OS).
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> 
> Convenient indeed ! :)
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> >  hw/ppc/spapr_hcall.c | 33 +++++++++++++--------------------
> >  1 file changed, 13 insertions(+), 20 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 140f05c1c6..05a7ca275b 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1767,21 +1767,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> >      }
> >      spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
> >      spapr_ovec_cleanup(ov1_guest);
> > -    if (!spapr->cas_reboot) {
> > -        /* If spapr_machine_reset() did not set up a HPT but one is necessary
> > -         * (because the guest isn't going to use radix) then set it up here. */
> > -        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
> > -            /* legacy hash or new hash: */
> > -            spapr_setup_hpt_and_vrma(spapr);
> > -        }
> > -        spapr->cas_reboot =
> > -            (spapr_h_cas_compose_response(spapr, args[1], args[2],
> > -                                          ov5_updates) != 0);
> > -    }
> >  
> >      /*
> > -     * Ensure the guest asks for an interrupt mode we support; otherwise
> > -     * terminate the boot.
> > +     * Ensure the guest asks for an interrupt mode we support;
> > +     * otherwise terminate the boot.
> >       */
> >      if (guest_xive) {
> >          if (!spapr->irq->xive) {
> > @@ -1797,14 +1786,18 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> >          }
> >      }
> >  
> > -    /*
> > -     * Generate a machine reset when we have an update of the
> > -     * interrupt mode. Only required when the machine supports both
> > -     * modes.
> > -     */
> > +    spapr_irq_update_active_intc(spapr);
> > +
> >      if (!spapr->cas_reboot) {
> > -        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> > -            && spapr->irq->xics && spapr->irq->xive;
> > +        /* If spapr_machine_reset() did not set up a HPT but one is necessary
> > +         * (because the guest isn't going to use radix) then set it up here. */
> > +        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
> > +            /* legacy hash or new hash: */
> > +            spapr_setup_hpt_and_vrma(spapr);
> > +        }
> > +        spapr->cas_reboot =
> > +            (spapr_h_cas_compose_response(spapr, args[1], args[2],
> > +                                          ov5_updates) != 0);
> >      }
> >  
> >      spapr_ovec_cleanup(ov5_updates);
> 

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