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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3023 bytes --]

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 <clg@kaod.org>
> >> ---
> >>  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.

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.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]