Re: [Qemu-ppc] [PATCH v6 06/37] ppc/xive: add support for the END Event State buffers

[Cédric Le Goater]

On 12/7/18 3:05 AM, David Gibson wrote:
> On Thu, Dec 06, 2018 at 07:30:34AM +0100, Cédric Le Goater wrote:
>> On 12/6/18 5:09 AM, David Gibson wrote:
>>> On Thu, Dec 06, 2018 at 12:22:20AM +0100, Cédric Le Goater wrote:
> [snip]
>>>> +/*
>>>> + * END ESB MMIO loads
>>>> + */
>>>> +static uint64_t xive_end_source_read(void *opaque, hwaddr addr, unsigned 
>>>> size)
>>>> +{
>>>> +    XiveENDSource *xsrc = XIVE_END_SOURCE(opaque);
>>>> +    XiveRouter *xrtr = xsrc->xrtr;
>>>> +    uint32_t offset = addr & 0xFFF;
>>>> +    uint8_t end_blk;
>>>> +    uint32_t end_idx;
>>>> +    XiveEND end;
>>>> +    uint32_t end_esmask;
>>>> +    uint8_t pq;
>>>> +    uint64_t ret = -1;
>>>> +
>>>> +    end_blk = xrtr->chip_id;
>>>
>>> .. instead I think it makes more sense to just configure the end_blk
>>> directly on the end_source, rather than reaching into another object
>>> to 
>>
>> Ah. That's what I was asking in an email. I missed the answer maybe.
>> Let's drop it and sPAPRXive block will be 0.
> 
> Actually, I think I might not have answered earlier, because it was
> far enough into the thread that I'd forgotten the original context too
> much to answer easily.
>
 
> Anyway, proposed course of action sounds fine.

Let's be clear. 

I will add a "block" property to the XiveENDSource model, which will be set 
to a fixed value (0) on sPAPRXive and to the chip_id on PowerNV. ok ? 

> 
> [snip]
>>>> +static void xive_source_end_reset(void *dev)
>>>> +{
>>>> +    /* TODO: Loop on all ENDs and mask off the ESn and ESe */
>>>
>>> IIUC you're talking about actually writing the (potentially in memory)
>>> END structures.  I don't think that makes sense for the END source
>>> hardware model.  I'm guessing you want this for PAPR, where the ENDs
>>> are in virtual hardware 
>>
>> That's done already.
>>
>>> rather than guest memory, but in that case I
>>> think the reset handling should be in the PAPR specific Xive object,
>>> not the end_source itself.
>>
>> I will remove the TODO, it's obsolete.
> 
> Ok, and the whole function with it, yes?  Or is there something else
> that will really belong here?

Nothing I would say. 

The XiveENDSource are backed by the END structures, which are software 
entities. So it's up to the PowerNV firmware, or the sPAPR IC model to 
do the cleanup.

C.