Re: [PATCH v3 09/14] ppc/xive2: Change context/ring specific functions to be generic

[Mike Kowal <kowal@linux.ibm.com>]

On 9/13/2024 8:10 AM, Cédric Le Goater wrote:
> On 9/12/24 22:50, Michael Kowal wrote:
>> Some the functions that have been created are specific to a ring or 
>> context. Some
>> of these same functions are being changed to operate on any 
>> ring/context. This  will
>> simplify the next patch sets that are adding additional ring/context 
>> operations.
>>
>> Signed-off-by: Michael Kowal <kowal@linux.ibm.com>
>> ---
>>   include/hw/ppc/xive.h |  2 +-
>>   hw/intc/xive.c        |  6 +++---
>>   hw/intc/xive2.c       | 23 ++++++++++++-----------
>>   3 files changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 28c181faa2..31242f0406 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -533,7 +533,7 @@ Object *xive_tctx_create(Object *cpu, 
>> XivePresenter *xptr, Error **errp);
>>   void xive_tctx_reset(XiveTCTX *tctx);
>>   void xive_tctx_destroy(XiveTCTX *tctx);
>>   void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
>> -void xive_tctx_reset_os_signal(XiveTCTX *tctx);
>> +void xive_tctx_reset_signal(XiveTCTX *tctx, uint8_t ring);
>>     /*
>>    * KVM XIVE device helpers
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 5b66a3aec5..f1d007d9a6 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -114,7 +114,7 @@ static void xive_tctx_notify(XiveTCTX *tctx, 
>> uint8_t ring)
>>       }
>>   }
>>   -void xive_tctx_reset_os_signal(XiveTCTX *tctx)
>> +void xive_tctx_reset_signal(XiveTCTX *tctx, uint8_t ring)
>>   {
>>       /*
>>        * Lower the External interrupt. Used when pulling an OS
>> @@ -122,7 +122,7 @@ void xive_tctx_reset_os_signal(XiveTCTX *tctx)
>>        * context. It should be raised again when re-pushing the OS
>>        * context.
>>        */
>> -    qemu_irq_lower(xive_tctx_output(tctx, TM_QW1_OS));
>> +    qemu_irq_lower(xive_tctx_output(tctx, ring));
>>   }
>>     static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t ring, 
>> uint8_t cppr)
>> @@ -424,7 +424,7 @@ static uint64_t xive_tm_pull_os_ctx(XivePresenter 
>> *xptr, XiveTCTX *tctx,
>>       qw1w2_new = xive_set_field32(TM_QW1W2_VO, qw1w2, 0);
>>       xive_tctx_set_os_cam(tctx, qw1w2_new);
>>   -    xive_tctx_reset_os_signal(tctx);
>> +    xive_tctx_reset_signal(tctx, TM_QW1_OS);
>>       return qw1w2;
>>   }
>>   diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
>> index 8d3d69a0db..ad8b8729ed 100644
>> --- a/hw/intc/xive2.c
>> +++ b/hw/intc/xive2.c
>> @@ -270,13 +270,14 @@ static void xive2_end_enqueue(Xive2End *end, 
>> uint32_t data)
>>    *     the NVP by changing the H bit while the context is enabled
>>    */
>>   -static void xive2_tctx_save_os_ctx(Xive2Router *xrtr, XiveTCTX *tctx,
>> -                                   uint8_t nvp_blk, uint32_t nvp_idx)
>> +static void xive2_tctx_save_ctx(Xive2Router *xrtr, XiveTCTX *tctx,
>> +                                uint8_t nvp_blk, uint32_t nvp_idx,
>> +                                uint8_t ring)
>>   {
>>       CPUPPCState *env = &POWERPC_CPU(tctx->cs)->env;
>>       uint32_t pir = env->spr_cb[SPR_PIR].default_value;
>>       Xive2Nvp nvp;
>> -    uint8_t *regs = &tctx->regs[TM_QW1_OS];
>> +    uint8_t *regs = &tctx->regs[ring];
>>         if (xive2_router_get_nvp(xrtr, nvp_blk, nvp_idx, &nvp)) {
>>           qemu_log_mask(LOG_GUEST_ERROR, "XIVE: No NVP %x/%x\n",
>> @@ -321,13 +322,13 @@ static void xive2_tctx_save_os_ctx(Xive2Router 
>> *xrtr, XiveTCTX *tctx,
>>       xive2_router_write_nvp(xrtr, nvp_blk, nvp_idx, &nvp, 1);
>>   }
>>   -static void xive2_os_cam_decode(uint32_t cam, uint8_t *nvp_blk,
>> -                                uint32_t *nvp_idx, bool *vo, bool *ho)
>> +static void xive2_cam_decode(uint32_t cam, uint8_t *nvp_blk,
>> +                             uint32_t *nvp_idx, bool *valid, bool *hw)
>>   {
>>       *nvp_blk = xive2_nvp_blk(cam);
>>       *nvp_idx = xive2_nvp_idx(cam);
>> -    *vo = !!(cam & TM2_QW1W2_VO);
>> -    *ho = !!(cam & TM2_QW1W2_HO);
>> +    *valid = !!(cam & TM2_QW1W2_VO);
>> +    *hw = !!(cam & TM2_QW1W2_HO);
>
> This change belongs to another patch. Anyhow,
>
>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
> Thanks,
>
> C.
>

Shoot, sorry about that.  I must have messed up the rebase.   The 
changes are in patch set 10.

MAK


>
>
>>   }
>>     @@ -363,7 +364,7 @@ uint64_t xive2_tm_pull_os_ctx(XivePresenter 
>> *xptr, XiveTCTX *tctx,
>>       bool vo;
>>       bool do_save;
>>   -    xive2_os_cam_decode(cam, &nvp_blk, &nvp_idx, &vo, &do_save);
>> +    xive2_cam_decode(cam, &nvp_blk, &nvp_idx, &vo, &do_save);
>>         if (!vo) {
>>           qemu_log_mask(LOG_GUEST_ERROR, "XIVE: pulling invalid NVP 
>> %x/%x !?\n",
>> @@ -375,10 +376,10 @@ uint64_t xive2_tm_pull_os_ctx(XivePresenter 
>> *xptr, XiveTCTX *tctx,
>>       memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2_new, 4);
>>         if (xive2_router_get_config(xrtr) & XIVE2_VP_SAVE_RESTORE && 
>> do_save) {
>> -        xive2_tctx_save_os_ctx(xrtr, tctx, nvp_blk, nvp_idx);
>> +        xive2_tctx_save_ctx(xrtr, tctx, nvp_blk, nvp_idx, TM_QW1_OS);
>>       }
>>   -    xive_tctx_reset_os_signal(tctx);
>> +    xive_tctx_reset_signal(tctx, TM_QW1_OS);
>>       return qw1w2;
>>   }
>>   @@ -573,7 +574,7 @@ void xive2_tm_push_os_ctx(XivePresenter *xptr, 
>> XiveTCTX *tctx,
>>       bool vo;
>>       bool do_restore;
>>   -    xive2_os_cam_decode(cam, &nvp_blk, &nvp_idx, &vo, &do_restore);
>> +    xive2_cam_decode(cam, &nvp_blk, &nvp_idx, &vo, &do_restore);
>>         /* First update the thead context */
>>       memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
>