Re: [PATCH for-5.0 v5 02/23] ppc/xive: Introduce helpers for the NVT id

[Cédric Le Goater]

On 19/11/2019 15:04, Greg Kurz wrote:
> On Fri, 15 Nov 2019 17:24:15 +0100
> Cédric Le Goater <address@hidden> wrote:
> 
>> Each vCPU in the system is identified with an NVT identifier which is
>> pushed in the OS CAM line (QW1W2) of the HW thread interrupt context
>> register when the vCPU is dispatched on a HW thread. This identifier
>> is used by the presenter subengine to find a matching target to notify
>> of an event. It is also used to fetch the associate NVT structure
>> which may contain pending interrupts that need a resend.
>>
>> Add a couple of helpers for the NVT ids. The NVT space is 19 bits
>> wide, giving a maximum of 512K per chip.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  include/hw/ppc/xive.h      |  5 -----
>>  include/hw/ppc/xive_regs.h | 21 +++++++++++++++++++++
>>  2 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 8fd439ec9bba..fa7adf87feb2 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -418,11 +418,6 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, 
>> Error **errp);
>>  void xive_tctx_reset(XiveTCTX *tctx);
>>  void xive_tctx_destroy(XiveTCTX *tctx);
>>  
>> -static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> -{
>> -    return (nvt_blk << 19) | nvt_idx;
>> -}
>> -
>>  /*
>>   * KVM XIVE device helpers
>>   */
>> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
>> index 530f232b04f8..1a5622f8ded8 100644
>> --- a/include/hw/ppc/xive_regs.h
>> +++ b/include/hw/ppc/xive_regs.h
>> @@ -272,4 +272,25 @@ typedef struct XiveNVT {
>>  
>>  #define xive_nvt_is_valid(nvt)    (be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
>>  
>> +/*
>> + * The VP number space in a block is defined by the END_W6_NVT_INDEX
>> + * field of the XIVE END
>> + */
>> +#define XIVE_NVT_SHIFT                19
>> +
>> +static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> +{
>> +    return (nvt_blk << XIVE_NVT_SHIFT) | nvt_idx;
> 
> Shouldn't we ensure nvt_idx fits in the 19 bits ?

yes. We should use the END_W6_NVT_INDEX mask. We are fine today because
the NVT index is extracted from the end w6 using xive_get_field32() or 
computed from the PIR using the appropriate mask.

Something to improve. 

> 
>> +}
>> +
>> +static inline uint32_t xive_nvt_idx(uint32_t cam_line)
>> +{
>> +    return cam_line & ((1 << XIVE_NVT_SHIFT) - 1);
>> +}
>> +
>> +static inline uint32_t xive_nvt_blk(uint32_t cam_line)
>> +{
>> +    return (cam_line >> XIVE_NVT_SHIFT) & 0xf;
>> +}
>> +
>>  #endif /* PPC_XIVE_REGS_H */
>