Re: [Qemu-ppc] [PATCH 03/19] xive: extend the XiveRouter get_tctx() method with the page offset

[Cédric Le Goater]

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 <address@hidden>
>> ---
>>  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;
>>  
>>          /*
>