Re: [Qemu-ppc] [PATCH v5 04/36] ppc/xive: introduce the XiveRouter model

[Cédric Le Goater]

On 11/27/18 1:11 AM, David Gibson wrote:
> On Mon, Nov 26, 2018 at 10:39:44AM +0100, Cédric Le Goater wrote:
>> On 11/26/18 6:44 AM, David Gibson wrote:
>>> On Fri, Nov 23, 2018 at 11:28:24AM +0100, Cédric Le Goater wrote:
>>>> On 11/23/18 2:10 AM, David Gibson wrote:
>>>>> On Thu, Nov 22, 2018 at 05:50:07PM +1100, Benjamin Herrenschmidt wrote:
>>>>>> On Thu, 2018-11-22 at 15:44 +1100, David Gibson wrote:
>>>>>>>
>>>>>>> Sorry, didn't think of this in my first reply.
>>>>>>>
>>>>>>> 1) Does the hardware ever actually write back to the EAS?  I know it
>>>>>>> does for the END, but it's not clear why it would need to for the
>>>>>>> EAS.  If not, we don't need the setter.
>>>>>>
>>>>>> Nope, though the PAPR model will via hcalls
>>>>>
>>>>> Right, bit AIUI the set_eas hook is about abstracting PAPR vs bare
>>>>> metal details.  Since the hcall knows it's PAPR it can just update the
>>>>> backing information for the EAS directly, and no need for an
>>>>> abstracted hook.
>>>>
>>>> Indeed, the first versions of the XIVE patchset did not use such hooks, 
>>>> but when discussed we said we wanted abstract methods for the router 
>>>> to validate the overall XIVE model, which is useful for PowerNV. 
>>>>
>>>> We can change again and have the hcalls get/set directly in the EAT
>>>> and ENDT. It would certainly simplify the sPAPR model.
>>>
>>> I think that's the better approach.
>>
>> ok. let's keep that in mind for  : 
>>
>>  [PATCH v5 11/36] spapr/xive: use the VCPU id as a NVT identifier
>>  [PATCH v5 16/36] spapr: add hcalls support for the XIVE exploitation
>>
>> which are using the XiveRouter methods to access the controller EAT 
>> and ENDT. I thought that was good practice to validate the model but 
>> we can use direct sPAPR table accessors or none at all.
> 
> Ok.  Consistency is good as a general rule, but I don't think it makes
> sense to force the EAT and the ENDT into the same model.  

What do you mean by model ? the QEMU machine IC model ?

> The EAT is
> pure configuration, whereas the the ENDT has both configuration and
> status.  Or to look at it another way, the EAT is purely software
> controlled, whereas the ENDT is at least partially hardware
> controlled.

yes but the EAT and the ENDT are XIVE internal tables of the same XIVE 
sub-engine, the IVRE, Interrupt Virtualization Routing Engine, formely 
known as VC, for Virtualization Controller. the EAS is just an entry 
point to the ENDT. I don't see why we would use different models for 
them.

> (I realize that gets a bit fuzzy when considering PAPR, but I think
> from the point of view of the XIVE model it makes sense to treat the
> PAPR hypervisor logic as "software", even though it's "hardware" from the
> guest point of view).

That I agree but the resulting code is too ugly in the hcalls. Tell me 
when you reach patch 11, which links the machine IC model sPAPRXive to 
the generic XiveRouter and also check patch 16 introducing the hcalls, 
which update the XIVE internal tables.

Thanks,

C. 

 
>>
>>
>> I think these prereq patches could be merged now :
>>
>>  [PATCH v5 12/36] spapr: initialize VSMT before initializing the IRQ
>>  [PATCH v5 13/36] spapr: introduce a spapr_irq_init() routine
>>  [PATCH v5 14/36] spapr: modify the irq backend 'init' method
>>
>> This one also :
>>
>>  [PATCH v5 21/36] spapr: extend the sPAPR IRQ backend for XICS
>>
>> Thanks,
>>
>> C. 
>>
>