[for-5.0,v5,01/23] ppc/xive: Record the IPB in the associated NVT
diff mbox series

Message ID 20191115162436.30548-2-clg@kaod.org
State New
Headers show
Series
  • ppc/pnv: add XIVE support for KVM guests
Related show

Commit Message

Cédric Le Goater Nov. 15, 2019, 4:24 p.m. UTC
When an interrupt can not be presented to a vCPU, because it is not
running on any of the HW treads, the XIVE presenter updates the
Interrupt Pending Buffer register of the associated XIVE NVT
structure. This is only done if backlog is activated in the END but
this is generally the case.

The current code assumes that the fields of the NVT structure is
architected with the same layout of the thread interrupt context
registers. Fix this assumption and define an offset for the IPB
register backup value in the NVT.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive_regs.h |  1 +
 hw/intc/xive.c             | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Greg Kurz Nov. 18, 2019, 3:44 p.m. UTC | #1
On Fri, 15 Nov 2019 17:24:14 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> When an interrupt can not be presented to a vCPU, because it is not
> running on any of the HW treads, the XIVE presenter updates the
> Interrupt Pending Buffer register of the associated XIVE NVT
> structure. This is only done if backlog is activated in the END but
> this is generally the case.
> 
> The current code assumes that the fields of the NVT structure is
> architected with the same layout of the thread interrupt context
> registers. Fix this assumption and define an offset for the IPB
> register backup value in the NVT.
> 

Does this fix a visible bug in the powernv machine ? If so,
maybe worth describing the symptoms.

Anyway, this seems conforment to the XIVE spec I have, so FWIW:

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

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/xive_regs.h |  1 +
>  hw/intc/xive.c             | 11 +++++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index 55307cd1533c..530f232b04f8 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -255,6 +255,7 @@ typedef struct XiveNVT {
>          uint32_t        w2;
>          uint32_t        w3;
>          uint32_t        w4;
> +#define NVT_W4_IPB               PPC_BITMASK32(16, 23)
>          uint32_t        w5;
>          uint32_t        w6;
>          uint32_t        w7;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 3d472e29c858..177663d2b43e 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1607,14 +1607,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
>       * - logical server : forward request to IVPE (not supported)
>       */
>      if (xive_end_is_backlog(&end)) {
> +        uint8_t ipb;
> +
>          if (format == 1) {
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "XIVE: END %x/%x invalid config: F1 & backlog\n",
>                            end_blk, end_idx);
>              return;
>          }
> -        /* Record the IPB in the associated NVT structure */
> -        ipb_update((uint8_t *) &nvt.w4, priority);
> +        /*
> +         * Record the IPB in the associated NVT structure for later
> +         * use. The presenter will resend the interrupt when the vCPU
> +         * is dispatched again on a HW thread.
> +         */
> +        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
> +        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
>          xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
>  
>          /*
Cédric Le Goater Nov. 18, 2019, 3:57 p.m. UTC | #2
On 18/11/2019 16:44, Greg Kurz wrote:
> On Fri, 15 Nov 2019 17:24:14 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> When an interrupt can not be presented to a vCPU, because it is not
>> running on any of the HW treads, the XIVE presenter updates the
>> Interrupt Pending Buffer register of the associated XIVE NVT
>> structure. This is only done if backlog is activated in the END but
>> this is generally the case.
>>
>> The current code assumes that the fields of the NVT structure is
>> architected with the same layout of the thread interrupt context
>> registers. Fix this assumption and define an offset for the IPB
>> register backup value in the NVT.
>>
> 
> Does this fix a visible bug in the powernv machine ? If so,
> maybe worth describing the symptoms.

no. this is a preliminary work for escalation suppport and to dump 
the contents of the NVT table.
 
> Anyway, this seems conforment to the XIVE spec I have, so FWIW:

FYI, the NVT structure evolves in the XIVE2 specs.

C.
 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/xive_regs.h |  1 +
>>  hw/intc/xive.c             | 11 +++++++++--
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
>> index 55307cd1533c..530f232b04f8 100644
>> --- a/include/hw/ppc/xive_regs.h
>> +++ b/include/hw/ppc/xive_regs.h
>> @@ -255,6 +255,7 @@ typedef struct XiveNVT {
>>          uint32_t        w2;
>>          uint32_t        w3;
>>          uint32_t        w4;
>> +#define NVT_W4_IPB               PPC_BITMASK32(16, 23)
>>          uint32_t        w5;
>>          uint32_t        w6;
>>          uint32_t        w7;
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 3d472e29c858..177663d2b43e 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -1607,14 +1607,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
>>       * - logical server : forward request to IVPE (not supported)
>>       */
>>      if (xive_end_is_backlog(&end)) {
>> +        uint8_t ipb;
>> +
>>          if (format == 1) {
>>              qemu_log_mask(LOG_GUEST_ERROR,
>>                            "XIVE: END %x/%x invalid config: F1 & backlog\n",
>>                            end_blk, end_idx);
>>              return;
>>          }
>> -        /* Record the IPB in the associated NVT structure */
>> -        ipb_update((uint8_t *) &nvt.w4, priority);
>> +        /*
>> +         * Record the IPB in the associated NVT structure for later
>> +         * use. The presenter will resend the interrupt when the vCPU
>> +         * is dispatched again on a HW thread.
>> +         */
>> +        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
>> +        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
>>          xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
>>  
>>          /*
>
David Gibson Nov. 19, 2019, 3:18 a.m. UTC | #3
On Fri, Nov 15, 2019 at 05:24:14PM +0100, Cédric Le Goater wrote:
> When an interrupt can not be presented to a vCPU, because it is not
> running on any of the HW treads, the XIVE presenter updates the
> Interrupt Pending Buffer register of the associated XIVE NVT
> structure. This is only done if backlog is activated in the END but
> this is generally the case.
> 
> The current code assumes that the fields of the NVT structure is
> architected with the same layout of the thread interrupt context
> registers. Fix this assumption and define an offset for the IPB
> register backup value in the NVT.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-5.0, thanks.

> ---
>  include/hw/ppc/xive_regs.h |  1 +
>  hw/intc/xive.c             | 11 +++++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index 55307cd1533c..530f232b04f8 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -255,6 +255,7 @@ typedef struct XiveNVT {
>          uint32_t        w2;
>          uint32_t        w3;
>          uint32_t        w4;
> +#define NVT_W4_IPB               PPC_BITMASK32(16, 23)
>          uint32_t        w5;
>          uint32_t        w6;
>          uint32_t        w7;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 3d472e29c858..177663d2b43e 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1607,14 +1607,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
>       * - logical server : forward request to IVPE (not supported)
>       */
>      if (xive_end_is_backlog(&end)) {
> +        uint8_t ipb;
> +
>          if (format == 1) {
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "XIVE: END %x/%x invalid config: F1 & backlog\n",
>                            end_blk, end_idx);
>              return;
>          }
> -        /* Record the IPB in the associated NVT structure */
> -        ipb_update((uint8_t *) &nvt.w4, priority);
> +        /*
> +         * Record the IPB in the associated NVT structure for later
> +         * use. The presenter will resend the interrupt when the vCPU
> +         * is dispatched again on a HW thread.
> +         */
> +        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
> +        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
>          xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
>  
>          /*

Patch
diff mbox series

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 55307cd1533c..530f232b04f8 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -255,6 +255,7 @@  typedef struct XiveNVT {
         uint32_t        w2;
         uint32_t        w3;
         uint32_t        w4;
+#define NVT_W4_IPB               PPC_BITMASK32(16, 23)
         uint32_t        w5;
         uint32_t        w6;
         uint32_t        w7;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 3d472e29c858..177663d2b43e 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1607,14 +1607,21 @@  static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
      * - logical server : forward request to IVPE (not supported)
      */
     if (xive_end_is_backlog(&end)) {
+        uint8_t ipb;
+
         if (format == 1) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "XIVE: END %x/%x invalid config: F1 & backlog\n",
                           end_blk, end_idx);
             return;
         }
-        /* Record the IPB in the associated NVT structure */
-        ipb_update((uint8_t *) &nvt.w4, priority);
+        /*
+         * Record the IPB in the associated NVT structure for later
+         * use. The presenter will resend the interrupt when the vCPU
+         * is dispatched again on a HW thread.
+         */
+        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
+        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
         xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
 
         /*