Re: [PATCH 3/9] ppc/xive: Record the IPB in the associated NVT

[Cédric Le Goater]

On 14/10/2019 07:32, David Gibson wrote:
> On Mon, Oct 07, 2019 at 10:40:56AM +0200, Cédric Le Goater wrote:
>> When an interrupt can not be presented to a vCPU, the XIVE presenter
>> updates the Interrupt Pending Buffer of the XIVE NVT if backlog is
>> activated in the END.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> This commit message doesn't really make it clear what was happening
> before, which certainly looks like an update to the IPB field in the
> NVT, via a different method.
> 
> Is this just a cleanup, or a fix?

This is a small fix. Here is an update changelog :

"
When an interrupt can not be presented to a vCPU, the XIVE presenter
updates the Interrupt Pending Buffer of the XIVE NVT if backlog is
activated in the END.

Fix the current model that also updates the PIPR which is not how HW 
operates and use the XIVE field structure routines to get/set the 
IPB register backup value in the XIVE NVT structure.
"

 
>> ---
>>  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 cbe4ae6c294d..2bf7b4ad7006 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -1600,14 +1600,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);
>>  
>>          /*
>