Re: [Qemu-devel] [PATCH 1/5] pseries: Move sPAPR RTC code into its own file

[David Gibson]

On Tue, Dec 16, 2014 at 10:41:16AM +0100, Alexander Graf wrote:
> 
> 
> 
> > Am 16.12.2014 um 02:24 schrieb David Gibson <address@hidden>:
> > 
> >> On Tue, Dec 16, 2014 at 01:59:01AM +0100, Alexander Graf wrote:
> >> 
> >> 
> >>> On 16.12.14 01:43, David Gibson wrote:
> >>> At the moment the RTAS (firmware/hypervisor) time of day functions are
> >>> implemented in spapr_rtas.c along with a bunch of other things.  Since
> >>> we're going to be expanding these a bit, move the RTAS RTC related code
> >>> out into new file spapr_rtc.c.  Also add its own initialization function,
> >>> spapr_rtc_init() called from the main machine init routine.
> >>> 
> >>> Signed-off-by: David Gibson <address@hidden>
> >>> ---
> >>> hw/ppc/Makefile.objs   |  2 +-
> >>> hw/ppc/spapr.c         |  3 ++
> >>> hw/ppc/spapr_rtas.c    | 49 -----------------------------
> >>> hw/ppc/spapr_rtc.c     | 83 
> >>> ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>> include/hw/ppc/spapr.h |  1 +
> >>> 5 files changed, 88 insertions(+), 50 deletions(-)
> >>> create mode 100644 hw/ppc/spapr_rtc.c
> >>> 
> >>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> >>> index 19d9920..437955d 100644
> >>> --- a/hw/ppc/Makefile.objs
> >>> +++ b/hw/ppc/Makefile.objs
> >>> @@ -3,7 +3,7 @@ obj-y += ppc.o ppc_booke.o
> >>> # IBM pSeries (sPAPR)
> >>> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> >>> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> >>> -obj-$(CONFIG_PSERIES) += spapr_pci.o
> >>> +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o
> >>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> >>> obj-y += spapr_pci_vfio.o
> >>> endif
> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>> index 30de25d..16377a3 100644
> >>> --- a/hw/ppc/spapr.c
> >>> +++ b/hw/ppc/spapr.c
> >>> @@ -1446,6 +1446,9 @@ static void ppc_spapr_init(MachineState *machine)
> >>>     /* Set up EPOW events infrastructure */
> >>>     spapr_events_init(spapr);
> >>> 
> >>> +    /* Set up the RTC RTAS interfaces */
> >>> +    spapr_rtc_init();
> >> 
> >> Do you think we could make it a device instead?
> > 
> > Um.. I guess.  Is there a standard place to put such pseudo-devices in
> > the bus heirarchy?
> 
> How about we combine this with some cleanup work and create a new
> spapr bus that (in the long term) exposes all the details we carry
> around in the spapr struct to its children via the bus?

I've thought about this a bit more.  It really doesn't make sense to
put the rtc device anywhere except the root bus (which optionally
could become an spapr sub-type of the default root bus type).

But, while splitting this into its own device would be cleaner, I
don't think we should delay real behavioural fixes for that cleanup.
Working out how to split out the device without breaking old machine
types, and keeping migration working is making my head hurt.

-- 
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

pgpHQDBlewta_.pgp
Description: PGP signature