Re: [Qemu-devel] [PATCH v6 04/37] ppc/xive: introduce the XiveRouter model

[Cédric Le Goater]

On 12/7/18 2:57 AM, David Gibson wrote:
> On Thu, Dec 06, 2018 at 07:22:54AM +0100, Cédric Le Goater wrote:
>> On 12/6/18 4:41 AM, David Gibson wrote:
>>> On Thu, Dec 06, 2018 at 12:22:18AM +0100, Cédric Le Goater wrote:
>>>> The XiveRouter models the second sub-engine of the XIVE architecture :
>>>> the Interrupt Virtualization Routing Engine (IVRE).
>>>>
>>>> The IVRE handles event notifications of the IVSE and performs the
>>>> interrupt routing process. For this purpose, it uses a set of tables
>>>> stored in system memory, the first of which being the Event Assignment
>>>> Structure (EAS) table.
>>>>
>>>> The EAT associates an interrupt source number with an Event Notification
>>>> Descriptor (END) which will be used in a second phase of the routing
>>>> process to identify a Notification Virtual Target.
>>>>
>>>> The XiveRouter is an abstract class which needs to be inherited from
>>>> to define a storage for the EAT, and other upcoming tables.
>>>>
>>>> Signed-off-by: Cédric Le Goater <address@hidden>
>>>> ---
>>>>  include/hw/ppc/xive.h      | 31 ++++++++++++++++
>>>>  include/hw/ppc/xive_regs.h | 50 +++++++++++++++++++++++++
>>>>  hw/intc/xive.c             | 76 ++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 157 insertions(+)
>>>>  create mode 100644 include/hw/ppc/xive_regs.h
>>>>
>>>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>>>> index 6770cffec67d..57ec9f84f527 100644
>>>> --- a/include/hw/ppc/xive.h
>>>> +++ b/include/hw/ppc/xive.h
>>>> @@ -141,6 +141,8 @@
>>>>  #define PPC_XIVE_H
>>>>  
>>>>  #include "hw/qdev-core.h"
>>>> +#include "hw/sysbus.h"
>>>> +#include "hw/ppc/xive_regs.h"
>>>>  
>>>>  /*
>>>>   * XIVE Fabric (Interface between Source and Router)
>>>> @@ -297,4 +299,33 @@ static inline void xive_source_irq_set(XiveSource 
>>>> *xsrc, uint32_t srcno,
>>>>      }
>>>>  }
>>>>  
>>>> +/*
>>>> + * XIVE Router
>>>> + */
>>>> +
>>>> +typedef struct XiveRouter {
>>>> +    SysBusDevice    parent;
>>>
>>> I thought the plan was to make XiveRouter as well as XiveSource a
>>> TYPE_DEVICE descendent rather than a SysBusDevice?
>>
>> We start talking about that, indeed, but then :
>>
>>      https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg06407.html
>>
>> I thought we concluded that it was going to get too complex.
>>
>> Also, sPAPRXive is a direct descendant of XiveRouter and we want sPAPRXive 
>> on SysBus.
> 
> Ah, good point.  So, to clarify my thinking here - I think from a
> theoretical point of view, having XiveRouter not be sysbus and
> including it by composition is probably the "correct" approach.

One possible solution would be to transform the XiveRouter in a QOM 
interface, this will be possible when I have removed the chip_id field,
and define the VST accessors as we do today. I am not sure how QOM 
interfaces are considered, but I think they are more in the composition 
pattern than inheritance. That way, we could have sPAPRXive directly 
inherit from SysBusDevice.

I can give it a try for v7, and you could merge the small XiveRouter 
changes in the current XiveRouter patch.

> But I can also see that that will be a bit of a pain in practice.  So
> yes, keeping it as a SysBusDevice is ok, at least as long as any
> migration stuff is in the "outermost" / most specific type, which I
> believe it is.

By this sentence, you mean that we don't rely on the XiveRouter model 
to capture the sPAPRXive state ? 

Thanks,

C.