diff mbox series

[03/19] xive: extend the XiveRouter get_tctx() method with the page offset

Message ID 20190128094625.4428-4-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series ppc: support for the baremetal XIVE interrupt controller (POWER9) | expand

Commit Message

Cédric Le Goater Jan. 28, 2019, 9:46 a.m. UTC
The PowerNV machine can perform indirect loads and stores on the TIMA
on behalf of another CPU. The PIR of the CPU is controlled by a set of
4 registers, one per TIMA page. To know which page is being accessed,
we need to inform the controller model of the operation offset.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h |  4 ++--
 hw/intc/spapr_xive.c  |  3 ++-
 hw/intc/xive.c        | 12 +++++++-----
 3 files changed, 11 insertions(+), 8 deletions(-)

Comments

David Gibson Feb. 12, 2019, 4:34 a.m. UTC | #1
On Mon, Jan 28, 2019 at 10:46:09AM +0100, Cédric Le Goater wrote:
> The PowerNV machine can perform indirect loads and stores on the TIMA
> on behalf of another CPU. The PIR of the CPU is controlled by a set of
> 4 registers, one per TIMA page. To know which page is being accessed,
> we need to inform the controller model of the operation offset.

This doesn't seem like the right place to handle this.  Shouldn't you
instead be passing a different CPUState in for "foreign" TCTX
accesses?

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/xive.h |  4 ++--
>  hw/intc/spapr_xive.c  |  3 ++-
>  hw/intc/xive.c        | 12 +++++++-----
>  3 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 04d54e8315f7..a1f5ea2d9143 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -352,7 +352,7 @@ typedef struct XiveRouterClass {
>                     XiveNVT *nvt);
>      int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>                       XiveNVT *nvt, uint8_t word_number);
> -    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
> +    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
>  } XiveRouterClass;
>  
>  void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
> @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>                          XiveNVT *nvt);
>  int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>                            XiveNVT *nvt, uint8_t word_number);
> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs);
> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
>  
>  /*
>   * XIVE END ESBs
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index a0f5ff929447..c41ee96c4c84 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk,
>      g_assert_not_reached();
>  }
>  
> -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
> +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs,
> +                                     hwaddr offset)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>  
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index f5642f2338de..39dff557fadc 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write)
>  static void xive_tm_write(void *opaque, hwaddr offset,
>                            uint64_t value, unsigned size)
>  {
> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
> +    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
> +                                          offset);
>      const XiveTmOp *xto;
>  
>      /*
> @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>  
>  static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>  {
> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
> +    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
> +                                          offset);
>      const XiveTmOp *xto;
>  
>      /*
> @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>     return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
>  }
>  
> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset)
>  {
>      XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>  
> -    return xrc->get_tctx(xrtr, cs);
> +    return xrc->get_tctx(xrtr, cs, offset);
>  }
>  
>  /*
> @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
>       */
>  
>      CPU_FOREACH(cs) {
> -        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
> +        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0);
>          int ring;
>  
>          /*
Cédric Le Goater Feb. 12, 2019, 8:25 a.m. UTC | #2
On 2/12/19 5:34 AM, David Gibson wrote:
> On Mon, Jan 28, 2019 at 10:46:09AM +0100, Cédric Le Goater wrote:
>> The PowerNV machine can perform indirect loads and stores on the TIMA
>> on behalf of another CPU. The PIR of the CPU is controlled by a set of
>> 4 registers, one per TIMA page. To know which page is being accessed,
>> we need to inform the controller model of the operation offset.
> 
> This doesn't seem like the right place to handle this.  Shouldn't you
> instead be passing a different CPUState in for "foreign" TCTX
> accesses?

This patch only adds the page offset required to pick the correct 
register from the XIVE IC. May be I should have introduced this 
page offset extension in the baremetal patch. I might reorganize
the patchset in the next round. 


Anyhow, we pass a different XiveTCTX instead. See the routines in 
PnvXive, xive_tm_indirect_read() and xive_tm_indirect_write().

We don't pass a CPUState because we depend on the machine data  to 
retrieve the XiveTCTX and that would be problematic to do in the 
common XIVE routines.

C. 


> 
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/xive.h |  4 ++--
>>  hw/intc/spapr_xive.c  |  3 ++-
>>  hw/intc/xive.c        | 12 +++++++-----
>>  3 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 04d54e8315f7..a1f5ea2d9143 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -352,7 +352,7 @@ typedef struct XiveRouterClass {
>>                     XiveNVT *nvt);
>>      int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>                       XiveNVT *nvt, uint8_t word_number);
>> -    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
>> +    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
>>  } XiveRouterClass;
>>  
>>  void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
>> @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>                          XiveNVT *nvt);
>>  int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>                            XiveNVT *nvt, uint8_t word_number);
>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs);
>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
>>  
>>  /*
>>   * XIVE END ESBs
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index a0f5ff929447..c41ee96c4c84 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk,
>>      g_assert_not_reached();
>>  }
>>  
>> -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
>> +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs,
>> +                                     hwaddr offset)
>>  {
>>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>>  
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index f5642f2338de..39dff557fadc 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write)
>>  static void xive_tm_write(void *opaque, hwaddr offset,
>>                            uint64_t value, unsigned size)
>>  {
>> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>> +    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
>> +                                          offset);
>>      const XiveTmOp *xto;
>>  
>>      /*
>> @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>>  
>>  static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>>  {
>> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>> +    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
>> +                                          offset);
>>      const XiveTmOp *xto;
>>  
>>      /*
>> @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>     return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
>>  }
>>  
>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset)
>>  {
>>      XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>>  
>> -    return xrc->get_tctx(xrtr, cs);
>> +    return xrc->get_tctx(xrtr, cs, offset);
>>  }
>>  
>>  /*
>> @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
>>       */
>>  
>>      CPU_FOREACH(cs) {
>> -        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
>> +        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0);
>>          int ring;
>>  
>>          /*
>
Cédric Le Goater Feb. 12, 2019, 8:31 p.m. UTC | #3
On 2/12/19 9:25 AM, Cédric Le Goater wrote:
> On 2/12/19 5:34 AM, David Gibson wrote:
>> On Mon, Jan 28, 2019 at 10:46:09AM +0100, Cédric Le Goater wrote:
>>> The PowerNV machine can perform indirect loads and stores on the TIMA
>>> on behalf of another CPU. The PIR of the CPU is controlled by a set of
>>> 4 registers, one per TIMA page. To know which page is being accessed,
>>> we need to inform the controller model of the operation offset.
>>
>> This doesn't seem like the right place to handle this.  Shouldn't you
>> instead be passing a different CPUState in for "foreign" TCTX
>> accesses?
> 
> This patch only adds the page offset required to pick the correct 
> register from the XIVE IC. May be I should have introduced this 
> page offset extension in the baremetal patch. I might reorganize
> the patchset in the next round. 

I had a chat with our hardware designer and the four indirect access
registers of the XIVE IC have no relation with the four pages exposing
the TIMA. It's just four different engines. We can drop this change.  
next patch "ppc/pnv: xive: export the TIMA memory accessors" is still
needed though.

C.


> Anyhow, we pass a different XiveTCTX instead. See the routines in 
> PnvXive, xive_tm_indirect_read() and xive_tm_indirect_write().
> 
> We don't pass a CPUState because we depend on the machine data  to 
> retrieve the XiveTCTX and that would be problematic to do in the 
> common XIVE routines.
> 
> C. 
> 
> 
>>
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>  include/hw/ppc/xive.h |  4 ++--
>>>  hw/intc/spapr_xive.c  |  3 ++-
>>>  hw/intc/xive.c        | 12 +++++++-----
>>>  3 files changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>>> index 04d54e8315f7..a1f5ea2d9143 100644
>>> --- a/include/hw/ppc/xive.h
>>> +++ b/include/hw/ppc/xive.h
>>> @@ -352,7 +352,7 @@ typedef struct XiveRouterClass {
>>>                     XiveNVT *nvt);
>>>      int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>>                       XiveNVT *nvt, uint8_t word_number);
>>> -    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
>>> +    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
>>>  } XiveRouterClass;
>>>  
>>>  void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
>>> @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>>                          XiveNVT *nvt);
>>>  int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>>                            XiveNVT *nvt, uint8_t word_number);
>>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs);
>>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
>>>  
>>>  /*
>>>   * XIVE END ESBs
>>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>>> index a0f5ff929447..c41ee96c4c84 100644
>>> --- a/hw/intc/spapr_xive.c
>>> +++ b/hw/intc/spapr_xive.c
>>> @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk,
>>>      g_assert_not_reached();
>>>  }
>>>  
>>> -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
>>> +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs,
>>> +                                     hwaddr offset)
>>>  {
>>>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>  
>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>>> index f5642f2338de..39dff557fadc 100644
>>> --- a/hw/intc/xive.c
>>> +++ b/hw/intc/xive.c
>>> @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write)
>>>  static void xive_tm_write(void *opaque, hwaddr offset,
>>>                            uint64_t value, unsigned size)
>>>  {
>>> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>>> +    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
>>> +                                          offset);
>>>      const XiveTmOp *xto;
>>>  
>>>      /*
>>> @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>>>  
>>>  static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>>>  {
>>> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>>> +    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
>>> +                                          offset);
>>>      const XiveTmOp *xto;
>>>  
>>>      /*
>>> @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>>     return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
>>>  }
>>>  
>>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
>>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset)
>>>  {
>>>      XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>>>  
>>> -    return xrc->get_tctx(xrtr, cs);
>>> +    return xrc->get_tctx(xrtr, cs, offset);
>>>  }
>>>  
>>>  /*
>>> @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
>>>       */
>>>  
>>>      CPU_FOREACH(cs) {
>>> -        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
>>> +        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0);
>>>          int ring;
>>>  
>>>          /*
>>
>
diff mbox series

Patch

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 04d54e8315f7..a1f5ea2d9143 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -352,7 +352,7 @@  typedef struct XiveRouterClass {
                    XiveNVT *nvt);
     int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                      XiveNVT *nvt, uint8_t word_number);
-    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
+    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
 } XiveRouterClass;
 
 void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
@@ -367,7 +367,7 @@  int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                         XiveNVT *nvt);
 int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                           XiveNVT *nvt, uint8_t word_number);
-XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs);
+XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset);
 
 /*
  * XIVE END ESBs
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index a0f5ff929447..c41ee96c4c84 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -391,7 +391,8 @@  static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk,
     g_assert_not_reached();
 }
 
-static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
+static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs,
+                                     hwaddr offset)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
 
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index f5642f2338de..39dff557fadc 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -320,7 +320,8 @@  static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write)
 static void xive_tm_write(void *opaque, hwaddr offset,
                           uint64_t value, unsigned size)
 {
-    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
+    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
+                                          offset);
     const XiveTmOp *xto;
 
     /*
@@ -358,7 +359,8 @@  static void xive_tm_write(void *opaque, hwaddr offset,
 
 static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
 {
-    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
+    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu,
+                                          offset);
     const XiveTmOp *xto;
 
     /*
@@ -1134,11 +1136,11 @@  int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
    return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
 }
 
-XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
+XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset)
 {
     XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
 
-    return xrc->get_tctx(xrtr, cs);
+    return xrc->get_tctx(xrtr, cs, offset);
 }
 
 /*
@@ -1234,7 +1236,7 @@  static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
      */
 
     CPU_FOREACH(cs) {
-        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
+        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0);
         int ring;
 
         /*