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

[Alexander Graf <agraf@suse.de>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 18.12.14 06:43, David Gibson wrote:
> On Tue, Dec 16, 2014 at 10:41:16AM +0100, Alexander Graf wrote:
>> 
>> 
>> 
>>> Am 16.12.2014 um 02:24 schrieb David Gibson
>>> <david@gibson.dropbear.id.au>:
>>> 
>>>> 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 <david@gibson.dropbear.id.au> 
>>>>> --- 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).

Well, we could always just leave sysbus completely unused and instead
create our own global spapr root bus. In fact, that's probably what we
should do ;).

If that blows your mind for this patch set, we can make the RTC a
sysbus device too though.

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

I don't think it's that hard. Just create a compat version for version
2 of "spapr" vmstates and have a post_load hook in there that shoves
the rtc offset into the new rtc device via a qom property. You should
even be able to just find the RTC device by path.

Keep in mind that we don't support new -> old migration.


Alex
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJUk2TrAAoJECszeR4D/txglhEP/jey0NvjfAan3LKNxHZhseF+
1gS9/5A648w+tsuR4NzAeTTWVbSsCiDvlS8AmISkUHmMb+Zx1J6NMzuAso8xtEeL
ahMQUrtcskzz+o5b7U4ZDRMRyejRaT4fq5ryYENkxrUXZLjMf82FxbRVSFgYb4s5
89Ct8Ts9ad0OSWQYJXuHYbFRBQCyMKBpFbmW0c5FO275oH+dspsjHpyKjtU9fvaV
XE/4gnF93rXLnZNBhPKNR74jiSZYv364wb1q8Pb8OiuMacKLlBxCby0PKLfcGwEy
o13S7T7v1yrbeB/6/zGT092DpjONQRkkQ5LWZXH9UfwWPcMThg4ByoYRT9ULocUj
PRrwIm8JeXgTyJC6YcZJwxGZhFJAipLJ5SxW56YFqNvAvi2Kdp2SllZFBHy6G1zQ
tmm3CMKkHkQ4DOao48NfRlncZB49UC8MUz80eLq/fUFs2F+SEPJl3s8HN79fo2/r
N8OBnOqSaPRJ28XhZ1Lk6ytkzeE+ymHQWuDJCeiFegJ75G0CeQDG7CrI1N6w4M9V
H/7C0/vooGYDxcnprLSZpeUYdHOR7EqkGZ+ppZ6kCeRWuH7hZS8I5rqMNPeCDZpJ
v1kgmcwhFcbPo3vJ1TXNhkFQe3iU+qh/DMnWogw6NwXXLBrb1XFeslqTFi+lI2Ln
3dC64uWiOQnhllmpHz6Z
=X0rP
-----END PGP SIGNATURE-----