Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[Cédric Le Goater]

On 12/07/2019 03:15, David Gibson wrote:
> On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
>> On 03/07/2019 04:07, David Gibson wrote:
>>> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
>>>> This is to perform lookups in the NVT table when a vCPU is dispatched
>>>> and possibly resend interrupts.
>>>
>>> I'm slightly confused by this one.  Aren't there multiple router
>>> objects, each of which can deliver to any thread?  In which case what
>>> router object is associated with a specific TCTX?
>>
>> when a vCPU is dispatched on a HW thread, the hypervisor does a store 
>> on the CAM line to store the VP id. At that time, it checks the IPB in 
>> the associated NVT structure and notifies the thread if an interrupt is 
>> pending. 
>>
>> We need to do a NVT lookup, just like the presenter in HW, hence the 
>> router pointer. You should look at the following patch which clarifies 
>> the resend sequence.
> 
> Hm, ok.
> 
>>>> Future XIVE chip will use a different class for the model of the
>>>> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
>>>
>>> This seems odd to me, shouldn't it be an interface pointer or
>>> something in that case?
>>
>> I have duplicated most of the XIVE models for P10 because the internal 
>> structures have changed. I managed to keep the XiveSource and XiveTCTX 
>> but we now have a Xive10Router, this is the reason why.
> 
> Right, but XiveRouter and Xive10Router must have something in common
> if they can both be used here.  Usually that's expressed as a shared
> QOM interface - in which case you can use a pointer to the interface,
> rathe than using Object * which kind of implies *anything* can go
> here.

Yeah. I also think it would be better to have a common base object but
the class don't have much in common. Here is what I have for now :

P9:

typedef struct XiveRouterClass {
    SysBusDeviceClass parent;

    /* XIVE table accessors */
    int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
                   XiveEAS *eas);
    int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
                   XiveEND *end);
    int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
                     XiveEND *end, uint8_t word_number);
    int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                   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);
    uint8_t (*get_block_id)(XiveRouter *xrtr);
} XiveRouterClass;

and P10:

typedef struct Xive10RouterClass {
    SysBusDeviceClass parent;

    /* XIVE table accessors */
    int (*get_eas)(Xive10Router *xrtr, uint8_t eas_blk, uint32_t eas_idx,
                   Xive10EAS *eas);
    int (*get_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
                   Xive10END *end);
    int (*write_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
                     Xive10END *end, uint8_t word_number);
    int (*get_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                   Xive10NVP *nvt);
    int (*write_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                     Xive10NVP *nvt, uint8_t word_number);
    XiveTCTX *(*get_tctx)(Xive10Router *xrtr, CPUState *cs);
    uint8_t (*get_block_id)(XiveRouter *xrtr);
    uint32_t (*get_config)(Xive10Router *xrtr);
} Xive10RouterClass;

Only get_tctx() is common. 

The XIVE structures (END, NV*) used by the routing algo have changed a lot.
Even the presenter has changed, because all the CAM lines have a slightly 
different format.   

C.