[Qemu-devel] [PATCH 0/3] spapr: XIVE and CAS fixes

[Qemu-devel] [PATCH 0/3] spapr: XIVE and CAS fixes

[Cédric Le Goater]

Hello,

Here are a couple of fixes, one for the XIVE exploitation mode option
bit which is clearly specified now and for one for the CAS generated
resets.

Last is a proposal to handle the rebuild of the device tree using a
reset when the XIVE exploitation mode is negotiated with CAS. 

The full tree is here:

  https://github.com/legoater/qemu/commits/xive

XIVE support for Linux is now merged in 4.14.

Thanks,

C.

Cédric Le Goater (3):
  ppc/xive: fix OV5_XIVE_EXPLOIT bits
  spapr: fix CAS-generated reset
  spapr: generate a CAS reset for XIVE exploitation mode

 hw/ppc/spapr.c              |  8 ++++++--
 hw/ppc/spapr_hcall.c        | 13 +++++++++++++
 include/hw/ppc/spapr_ovec.h |  3 ++-
 3 files changed, 21 insertions(+), 3 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH 1/3] ppc/xive: fix OV5_XIVE_EXPLOIT bits

[Cédric Le Goater]

On POWER9, the Client Architecture Support (CAS) negotiation process
determines whether the guest operates in XIVE Legacy compatibility or
in XIVE exploitation mode. Now that we have initial guest support for
the XIVE interrupt controller, let's fix the bits definition which have
evolved in the latest specs.

The platform advertises the XIVE Exploitation Mode support using the
property "ibm,arch-vec-5-platform-support-vec-5", byte 23 bits 0-1 :

 - 0b00 XIVE legacy mode Only
 - 0b01 XIVE exploitation mode Only
 - 0b10 XIVE legacy or exploitation mode

The OS asks for XIVE Exploitation Mode support using the property
"ibm,architecture-vec-5", byte 23 bits 0-1:

 - 0b00 XIVE legacy mode Only
 - 0b01 XIVE exploitation mode Only

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr.c              | 2 +-
 include/hw/ppc/spapr_ovec.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cec441cbf48d..3e3ff1fbc988 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -914,7 +914,7 @@ static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
     PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
 
     char val[2 * 4] = {
-        23, 0x00, /* Xive mode: 0 = legacy (as in ISA 2.7), 1 = Exploitation */
+        23, 0x00, /* Xive mode, filled in below. */
         24, 0x00, /* Hash/Radix, filled in below. */
         25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
         26, 0x40, /* Radix options: GTSE == yes. */
diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
index 9edfa5ff7530..bf25e5d954a1 100644
--- a/include/hw/ppc/spapr_ovec.h
+++ b/include/hw/ppc/spapr_ovec.h
@@ -51,7 +51,8 @@ typedef struct sPAPROptionVector sPAPROptionVector;
 #define OV5_FORM1_AFFINITY      OV_BIT(5, 0)
 #define OV5_HP_EVT              OV_BIT(6, 5)
 #define OV5_HPT_RESIZE          OV_BIT(6, 7)
-#define OV5_XIVE_EXPLOIT        OV_BIT(23, 7)
+#define OV5_XIVE_BOTH           OV_BIT(23, 0)
+#define OV5_XIVE_EXPLOIT        OV_BIT(23, 1) /* 1=exploitation 0=legacy */
 
 /* ISA 3.00 MMU features: */
 #define OV5_MMU_BOTH            OV_BIT(24, 0) /* Radix and hash */
-- 
2.13.5

[Qemu-devel] [PATCH 2/3] spapr: fix CAS-generated reset

[Cédric Le Goater]

The OV5_MMU_RADIX_300 requires special handling in the CAS negotiation
process. It is cleared from the option vector of the guest before
evaluating the changes and re-added later. But, when testing for a
possible CAS reset :

    spapr->cas_reboot = spapr_ovec_diff(ov5_updates,
                                        ov5_cas_old, spapr->ov5_cas);

the bit OV5_MMU_RADIX_300 will each time be seen as removed from the
previous OV5 set, hence generating a reset loop.

Fix this problem by also clearing the same bit in the ov5_cas_old set.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr_hcall.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 07b3da8dc4cd..92f1e21358b8 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1575,6 +1575,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
      * to worry about this for now.
      */
     ov5_cas_old = spapr_ovec_clone(spapr->ov5_cas);
+
+    /* also clear the radix/hash bit from the current ov5_cas bits to
+     * be in sync with the newly ov5 bits. Else the radix bit will be
+     * seen as being removed and this will generate a reset loop
+     */
+    spapr_ovec_clear(ov5_cas_old, OV5_MMU_RADIX_300);
+
     /* full range of negotiated ov5 capabilities */
     spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
     spapr_ovec_cleanup(ov5_guest);
-- 
2.13.5

[Qemu-devel] [RFC PATCH 3/3] spapr: generate a CAS reset for the XIVE exploitation mode

[Cédric Le Goater]

When the platform and the guest agree on using the XIVE exploitation
mode for interrupts, the "interrupt-controller" node needs to reflect
the change and the device tree needs an update.

Reseting the guest after the CAS negotiation makes this change
possible, as the device tree is built at reset time. We use the
'ov5_cas' field to check which interrupt model was negotiated before
reset and populate the tree accordingly.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr.c       | 6 +++++-
 hw/ppc/spapr_hcall.c | 6 ++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3e3ff1fbc988..be467ab61ad0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1073,7 +1073,11 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
 
     /* /interrupt controller */
-    spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
+    if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
+        spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
+    } else {
+        /* populate device tree for XIVE */ ;
+    }
 
     ret = spapr_populate_memory(spapr, fdt);
     if (ret < 0) {
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 92f1e21358b8..ba00b8d3fdd6 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1613,6 +1613,12 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
             (spapr_h_cas_compose_response(spapr, args[1], args[2],
                                           ov5_updates) != 0);
     }
+
+    /* We need to rebuild the device tree for XIVE, generate a reset */
+    if (!spapr->cas_reboot) {
+        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT);
+    }
+
     spapr_ovec_cleanup(ov5_updates);
 
     if (spapr->cas_reboot) {
-- 
2.13.5

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] ppc/xive: fix OV5_XIVE_EXPLOIT bits

[Greg Kurz]

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

Shouldn't the patch title mention spapr instead of ppc/xive ?

On Fri,  8 Sep 2017 16:33:42 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> On POWER9, the Client Architecture Support (CAS) negotiation process
> determines whether the guest operates in XIVE Legacy compatibility or
> in XIVE exploitation mode. Now that we have initial guest support for
> the XIVE interrupt controller, let's fix the bits definition which have
> evolved in the latest specs.
> 
> The platform advertises the XIVE Exploitation Mode support using the
> property "ibm,arch-vec-5-platform-support-vec-5", byte 23 bits 0-1 :
> 
>  - 0b00 XIVE legacy mode Only
>  - 0b01 XIVE exploitation mode Only
>  - 0b10 XIVE legacy or exploitation mode
> 
> The OS asks for XIVE Exploitation Mode support using the property
> "ibm,architecture-vec-5", byte 23 bits 0-1:
> 
>  - 0b00 XIVE legacy mode Only
>  - 0b01 XIVE exploitation mode Only
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/spapr.c              | 2 +-
>  include/hw/ppc/spapr_ovec.h | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index cec441cbf48d..3e3ff1fbc988 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -914,7 +914,7 @@ static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
>      PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
>  
>      char val[2 * 4] = {
> -        23, 0x00, /* Xive mode: 0 = legacy (as in ISA 2.7), 1 = Exploitation */
> +        23, 0x00, /* Xive mode, filled in below. */
>          24, 0x00, /* Hash/Radix, filled in below. */
>          25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
>          26, 0x40, /* Radix options: GTSE == yes. */
> diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
> index 9edfa5ff7530..bf25e5d954a1 100644
> --- a/include/hw/ppc/spapr_ovec.h
> +++ b/include/hw/ppc/spapr_ovec.h
> @@ -51,7 +51,8 @@ typedef struct sPAPROptionVector sPAPROptionVector;
>  #define OV5_FORM1_AFFINITY      OV_BIT(5, 0)
>  #define OV5_HP_EVT              OV_BIT(6, 5)
>  #define OV5_HPT_RESIZE          OV_BIT(6, 7)
> -#define OV5_XIVE_EXPLOIT        OV_BIT(23, 7)
> +#define OV5_XIVE_BOTH           OV_BIT(23, 0)
> +#define OV5_XIVE_EXPLOIT        OV_BIT(23, 1) /* 1=exploitation 0=legacy */
>  
>  /* ISA 3.00 MMU features: */
>  #define OV5_MMU_BOTH            OV_BIT(24, 0) /* Radix and hash */


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/3] spapr: XIVE and CAS fixes

[Cédric Le Goater]

On 09/08/2017 04:33 PM, Cédric Le Goater wrote:
> Hello,
> 
> Here are a couple of fixes, one for the XIVE exploitation mode option
> bit which is clearly specified now and for one for the CAS generated
> resets.
> 
> Last is a proposal to handle the rebuild of the device tree using a
> reset when the XIVE exploitation mode is negotiated with CAS. 

I should have copied Sam on this topic for some insights. 

Thanks,

C. 


> The full tree is here:
> 
>   https://github.com/legoater/qemu/commits/xive
> 
> XIVE support for Linux is now merged in 4.14.
> 
> Thanks,
> 
> C.
> 
> Cédric Le Goater (3):
>   ppc/xive: fix OV5_XIVE_EXPLOIT bits
>   spapr: fix CAS-generated reset
>   spapr: generate a CAS reset for XIVE exploitation mode
> 
>  hw/ppc/spapr.c              |  8 ++++++--
>  hw/ppc/spapr_hcall.c        | 13 +++++++++++++
>  include/hw/ppc/spapr_ovec.h |  3 ++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 

Re: [Qemu-devel] [PATCH 1/3] ppc/xive: fix OV5_XIVE_EXPLOIT bits

[David Gibson]

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

On Fri, Sep 08, 2017 at 04:33:42PM +0200, Cédric Le Goater wrote:
> On POWER9, the Client Architecture Support (CAS) negotiation process
> determines whether the guest operates in XIVE Legacy compatibility or
> in XIVE exploitation mode. Now that we have initial guest support for
> the XIVE interrupt controller, let's fix the bits definition which have
> evolved in the latest specs.
> 
> The platform advertises the XIVE Exploitation Mode support using the
> property "ibm,arch-vec-5-platform-support-vec-5", byte 23 bits 0-1 :
> 
>  - 0b00 XIVE legacy mode Only
>  - 0b01 XIVE exploitation mode Only
>  - 0b10 XIVE legacy or exploitation mode
> 
> The OS asks for XIVE Exploitation Mode support using the property
> "ibm,architecture-vec-5", byte 23 bits 0-1:
> 
>  - 0b00 XIVE legacy mode Only
>  - 0b01 XIVE exploitation mode Only
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-2.11, thanks.

> ---
>  hw/ppc/spapr.c              | 2 +-
>  include/hw/ppc/spapr_ovec.h | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index cec441cbf48d..3e3ff1fbc988 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -914,7 +914,7 @@ static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
>      PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
>  
>      char val[2 * 4] = {
> -        23, 0x00, /* Xive mode: 0 = legacy (as in ISA 2.7), 1 = Exploitation */
> +        23, 0x00, /* Xive mode, filled in below. */
>          24, 0x00, /* Hash/Radix, filled in below. */
>          25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
>          26, 0x40, /* Radix options: GTSE == yes. */
> diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
> index 9edfa5ff7530..bf25e5d954a1 100644
> --- a/include/hw/ppc/spapr_ovec.h
> +++ b/include/hw/ppc/spapr_ovec.h
> @@ -51,7 +51,8 @@ typedef struct sPAPROptionVector sPAPROptionVector;
>  #define OV5_FORM1_AFFINITY      OV_BIT(5, 0)
>  #define OV5_HP_EVT              OV_BIT(6, 5)
>  #define OV5_HPT_RESIZE          OV_BIT(6, 7)
> -#define OV5_XIVE_EXPLOIT        OV_BIT(23, 7)
> +#define OV5_XIVE_BOTH           OV_BIT(23, 0)
> +#define OV5_XIVE_EXPLOIT        OV_BIT(23, 1) /* 1=exploitation 0=legacy */
>  
>  /* ISA 3.00 MMU features: */
>  #define OV5_MMU_BOTH            OV_BIT(24, 0) /* Radix and hash */

-- 
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: 833 bytes --]

Re: [Qemu-devel] [PATCH 2/3] spapr: fix CAS-generated reset

[David Gibson]

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

On Fri, Sep 08, 2017 at 04:33:43PM +0200, Cédric Le Goater wrote:
> The OV5_MMU_RADIX_300 requires special handling in the CAS negotiation
> process. It is cleared from the option vector of the guest before
> evaluating the changes and re-added later. But, when testing for a
> possible CAS reset :
> 
>     spapr->cas_reboot = spapr_ovec_diff(ov5_updates,
>                                         ov5_cas_old, spapr->ov5_cas);
> 
> the bit OV5_MMU_RADIX_300 will each time be seen as removed from the
> previous OV5 set, hence generating a reset loop.
> 
> Fix this problem by also clearing the same bit in the ov5_cas_old set.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Kind of an ugly hack, but probably the easiest fix for now. Applied to ppc-for-2.11.

> ---
>  hw/ppc/spapr_hcall.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 07b3da8dc4cd..92f1e21358b8 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1575,6 +1575,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>       * to worry about this for now.
>       */
>      ov5_cas_old = spapr_ovec_clone(spapr->ov5_cas);
> +
> +    /* also clear the radix/hash bit from the current ov5_cas bits to
> +     * be in sync with the newly ov5 bits. Else the radix bit will be
> +     * seen as being removed and this will generate a reset loop
> +     */
> +    spapr_ovec_clear(ov5_cas_old, OV5_MMU_RADIX_300);
> +
>      /* full range of negotiated ov5 capabilities */
>      spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
>      spapr_ovec_cleanup(ov5_guest);

-- 
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: 833 bytes --]

Re: [Qemu-devel] [RFC PATCH 3/3] spapr: generate a CAS reset for the XIVE exploitation mode

[David Gibson]

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

On Fri, Sep 08, 2017 at 04:33:44PM +0200, Cédric Le Goater wrote:
> When the platform and the guest agree on using the XIVE exploitation
> mode for interrupts, the "interrupt-controller" node needs to reflect
> the change and the device tree needs an update.
> 
> Reseting the guest after the CAS negotiation makes this change
> possible, as the device tree is built at reset time. We use the
> 'ov5_cas' field to check which interrupt model was negotiated before
> reset and populate the tree accordingly.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Looks reasonable, though I'm not going to apply it until we actually
have something to go..

> ---
>  hw/ppc/spapr.c       | 6 +++++-
>  hw/ppc/spapr_hcall.c | 6 ++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3e3ff1fbc988..be467ab61ad0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1073,7 +1073,11 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>      _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
>  
>      /* /interrupt controller */
> -    spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> +    if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +        spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> +    } else {
> +        /* populate device tree for XIVE */ ;

.. here.

> +    }
>  
>      ret = spapr_populate_memory(spapr, fdt);
>      if (ret < 0) {
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 92f1e21358b8..ba00b8d3fdd6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1613,6 +1613,12 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>              (spapr_h_cas_compose_response(spapr, args[1], args[2],
>                                            ov5_updates) != 0);
>      }
> +
> +    /* We need to rebuild the device tree for XIVE, generate a reset */
> +    if (!spapr->cas_reboot) {
> +        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT);
> +    }

At some point we may well want to make XIVE exploitation the default
for POWER9 guests, but that's a detail easily adjusted later.

>      spapr_ovec_cleanup(ov5_updates);
>  
>      if (spapr->cas_reboot) {

-- 
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: 833 bytes --]

Re: [Qemu-devel] [PATCH 2/3] spapr: fix CAS-generated reset

[Cédric Le Goater]

On 09/10/2017 05:17 AM, David Gibson wrote:
> On Fri, Sep 08, 2017 at 04:33:43PM +0200, Cédric Le Goater wrote:
>> The OV5_MMU_RADIX_300 requires special handling in the CAS negotiation
>> process. It is cleared from the option vector of the guest before
>> evaluating the changes and re-added later. But, when testing for a
>> possible CAS reset :
>>
>>     spapr->cas_reboot = spapr_ovec_diff(ov5_updates,
>>                                         ov5_cas_old, spapr->ov5_cas);
>>
>> the bit OV5_MMU_RADIX_300 will each time be seen as removed from the
>> previous OV5 set, hence generating a reset loop.
>>
>> Fix this problem by also clearing the same bit in the ov5_cas_old set.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Kind of an ugly hack,  

yes. I lack context to fully understand why the guest radix option is
handled that way. 

> but probably the easiest fix for now. Applied to ppc-for-2.11.

Thanks,

C.

> 
>> ---
>>  hw/ppc/spapr_hcall.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 07b3da8dc4cd..92f1e21358b8 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1575,6 +1575,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>>       * to worry about this for now.
>>       */
>>      ov5_cas_old = spapr_ovec_clone(spapr->ov5_cas);
>> +
>> +    /* also clear the radix/hash bit from the current ov5_cas bits to
>> +     * be in sync with the newly ov5 bits. Else the radix bit will be
>> +     * seen as being removed and this will generate a reset loop
>> +     */
>> +    spapr_ovec_clear(ov5_cas_old, OV5_MMU_RADIX_300);
>> +
>>      /* full range of negotiated ov5 capabilities */
>>      spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
>>      spapr_ovec_cleanup(ov5_guest);
> 

Re: [Qemu-devel] [RFC PATCH 3/3] spapr: generate a CAS reset for the XIVE exploitation mode

[Cédric Le Goater]

On 09/10/2017 05:19 AM, David Gibson wrote:
> On Fri, Sep 08, 2017 at 04:33:44PM +0200, Cédric Le Goater wrote:
>> When the platform and the guest agree on using the XIVE exploitation
>> mode for interrupts, the "interrupt-controller" node needs to reflect
>> the change and the device tree needs an update.
>>
>> Reseting the guest after the CAS negotiation makes this change
>> possible, as the device tree is built at reset time. We use the
>> 'ov5_cas' field to check which interrupt model was negotiated before
>> reset and populate the tree accordingly.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Looks reasonable, though I'm not going to apply it until we actually
> have something to go..

Yes. I agree. A simplified XIVE patchset for sPAPR should be ready 
soon for comments. Please see below for one question.

>> ---
>>  hw/ppc/spapr.c       | 6 +++++-
>>  hw/ppc/spapr_hcall.c | 6 ++++++
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 3e3ff1fbc988..be467ab61ad0 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1073,7 +1073,11 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>>      _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
>>  
>>      /* /interrupt controller */
>> -    spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
>> +    if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
>> +        spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
>> +    } else {
>> +        /* populate device tree for XIVE */ ;
> 
> .. here.

How should we handle the PHANDLE_XICP for xive ? as it is done
for xics ? 

The DT node is now :	"interrupt-controller@6030203180000"

>> +    }
>>  
>>      ret = spapr_populate_memory(spapr, fdt);
>>      if (ret < 0) {
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 92f1e21358b8..ba00b8d3fdd6 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1613,6 +1613,12 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>>              (spapr_h_cas_compose_response(spapr, args[1], args[2],
>>                                            ov5_updates) != 0);
>>      }
>> +
>> +    /* We need to rebuild the device tree for XIVE, generate a reset */
>> +    if (!spapr->cas_reboot) {
>> +        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT);
>> +    }
> 
> At some point we may well want to make XIVE exploitation the default
> for POWER9 guests, but that's a detail easily adjusted later.

ah yes. we can do that.

Thanks,

C. 
 
>>      spapr_ovec_cleanup(ov5_updates);
>>  
>>      if (spapr->cas_reboot) {
> 

Re: [Qemu-devel] [RFC PATCH 3/3] spapr: generate a CAS reset for the XIVE exploitation mode

[David Gibson]

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

On Sun, Sep 10, 2017 at 09:30:18PM +0200, Cédric Le Goater wrote:
> On 09/10/2017 05:19 AM, David Gibson wrote:
> > On Fri, Sep 08, 2017 at 04:33:44PM +0200, Cédric Le Goater wrote:
> >> When the platform and the guest agree on using the XIVE exploitation
> >> mode for interrupts, the "interrupt-controller" node needs to reflect
> >> the change and the device tree needs an update.
> >>
> >> Reseting the guest after the CAS negotiation makes this change
> >> possible, as the device tree is built at reset time. We use the
> >> 'ov5_cas' field to check which interrupt model was negotiated before
> >> reset and populate the tree accordingly.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > 
> > Looks reasonable, though I'm not going to apply it until we actually
> > have something to go..
> 
> Yes. I agree. A simplified XIVE patchset for sPAPR should be ready 
> soon for comments. Please see below for one question.
> 
> >> ---
> >>  hw/ppc/spapr.c       | 6 +++++-
> >>  hw/ppc/spapr_hcall.c | 6 ++++++
> >>  2 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 3e3ff1fbc988..be467ab61ad0 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -1073,7 +1073,11 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >>      _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
> >>  
> >>      /* /interrupt controller */
> >> -    spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> >> +    if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> >> +        spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> >> +    } else {
> >> +        /* populate device tree for XIVE */ ;
> > 
> > .. here.
> 
> How should we handle the PHANDLE_XICP for xive ? as it is done
> for xics ? 
> 
> The DT node is now :	"interrupt-controller@6030203180000"

Is there just one node that needs to be referenced?  If so, then yes,
the XICS approach of just using a well known phandle for it should be
fine.


> 
> >> +    }
> >>  
> >>      ret = spapr_populate_memory(spapr, fdt);
> >>      if (ret < 0) {
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 92f1e21358b8..ba00b8d3fdd6 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1613,6 +1613,12 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> >>              (spapr_h_cas_compose_response(spapr, args[1], args[2],
> >>                                            ov5_updates) != 0);
> >>      }
> >> +
> >> +    /* We need to rebuild the device tree for XIVE, generate a reset */
> >> +    if (!spapr->cas_reboot) {
> >> +        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT);
> >> +    }
> > 
> > At some point we may well want to make XIVE exploitation the default
> > for POWER9 guests, but that's a detail easily adjusted later.
> 
> ah yes. we can do that.
> 
> Thanks,
> 
> C. 
>  
> >>      spapr_ovec_cleanup(ov5_updates);
> >>  
> >>      if (spapr->cas_reboot) {
> > 
> 

-- 
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: 833 bytes --]