From: James Hogan <james.hogan@imgtec.com>
To: Aurelien Jarno <aurelien@aurel32.net>
Cc: Sanjay Lal <sanjayl@kymasys.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2] hw/mips: gt64xxx_pci: Add VMStateDescription
Date: Thu, 19 Jun 2014 16:46:03 +0100 [thread overview]
Message-ID: <53A305BB.7030405@imgtec.com> (raw)
In-Reply-To: <20140619152550.GA7398@ohm.rr44.fr>
On 19/06/14 16:25, Aurelien Jarno wrote:
> On Wed, Jun 18, 2014 at 12:10:02AM +0100, James Hogan wrote:
>> From: Sanjay Lal <sanjayl@kymasys.com>
>>
>> Add VMStateDescription for GT64120 PCI emulation used by the Malta
>> platform, to allow it to work with savevm/loadvm and live migration.
>>
>> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
>> [james.hogan@imgtec.com: Convert to VMState]
>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>> This is based on "[Patch 03/12] KVM/MIPS: Add save/restore state APIs
>> for saving/restoring KVM guests."[1].
>>
>> Changes in v2:
>> - Expand commit message
>> - Convert to VMState (Peter Maydell)
>>
>> [1] https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg00195.html
>> ---
>> hw/mips/gt64xxx_pci.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 152 insertions(+)
>>
>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>> index 6398514c99d2..76690ecd6ca4 100644
>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -312,6 +312,156 @@ static void gt64120_pci_mapping(GT64120State *s)
>> }
>> }
>>
>> +static int gt64120_post_load(void *opaque, int version_id)
>> +{
>> + GT64120State *s = opaque;
>> +
>> + gt64120_isd_mapping(s);
>> + gt64120_pci_mapping(s);
>> +
>> + return 0;
>> +}
>> +
>> +static const VMStateDescription vmstate_gt64120 = {
>> + .name = "gt64120",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .post_load = gt64120_post_load,
>> + .fields = (VMStateField[]) {
>> + /* CPU Configuration */
>> + VMSTATE_UINT32(regs[GT_CPU], GT64120State),
>> + VMSTATE_UINT32(regs[GT_MULTI], GT64120State),
>> + /* CPU Address decode */
>> + VMSTATE_UINT32(regs[GT_SCS10LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS10HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS32LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS32HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS20LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS20HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS3BOOTLD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS3BOOTHD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0IOLD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0IOHD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0M0LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0M0HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_ISD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0M1LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0M1HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1IOLD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1IOHD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1M0LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1M0HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1M1LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1M1HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS10AR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS32AR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS20R], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS3BOOTR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0IOREMAP], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0M0REMAP], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0M1REMAP], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1IOREMAP], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1M0REMAP], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1M1REMAP], GT64120State),
>> + /* CPU Error Report */
>> + VMSTATE_UINT32(regs[GT_CPUERR_ADDRLO], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CPUERR_ADDRHI], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CPUERR_DATALO], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CPUERR_DATAHI], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CPUERR_PARITY], GT64120State),
>> + /* CPU Sync Barrier */
>> + VMSTATE_UINT32(regs[GT_PCI0SYNC], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1SYNC], GT64120State),
>> + /* SDRAM and Device Address Decode */
>> + VMSTATE_UINT32(regs[GT_SCS0LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS0HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS1LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS1HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS2LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS2HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS3LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SCS3HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS0LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS0HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS1LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS1HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS2LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS2HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS3LD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_CS3HD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_BOOTLD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_BOOTHD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_ADERR], GT64120State),
>> + /* SDRAM Configuration */
>> + VMSTATE_UINT32(regs[GT_SDRAM_CFG], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SDRAM_OPMODE], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SDRAM_BM], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SDRAM_ADDRDECODE], GT64120State),
>> + /* SDRAM Parameters */
>> + VMSTATE_UINT32(regs[GT_SDRAM_B0], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SDRAM_B1], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SDRAM_B2], GT64120State),
>> + VMSTATE_UINT32(regs[GT_SDRAM_B3], GT64120State),
>> + /* ECC */
>> + VMSTATE_UINT32(regs[GT_ECC_ERRDATALO], GT64120State),
>> + VMSTATE_UINT32(regs[GT_ECC_ERRDATAHI], GT64120State),
>> + VMSTATE_UINT32(regs[GT_ECC_MEM], GT64120State),
>> + VMSTATE_UINT32(regs[GT_ECC_CALC], GT64120State),
>> + VMSTATE_UINT32(regs[GT_ECC_ERRADDR], GT64120State),
>> + /* Device Parameters */
>> + VMSTATE_UINT32(regs[GT_DEV_B0], GT64120State),
>> + VMSTATE_UINT32(regs[GT_DEV_B1], GT64120State),
>> + VMSTATE_UINT32(regs[GT_DEV_B2], GT64120State),
>> + VMSTATE_UINT32(regs[GT_DEV_B3], GT64120State),
>> + VMSTATE_UINT32(regs[GT_DEV_BOOT], GT64120State),
>> + /* DMA registers are all zeroed at reset */
>> + /* Timer/Counter */
>> + VMSTATE_UINT32(regs[GT_TC0], GT64120State),
>> + VMSTATE_UINT32(regs[GT_TC1], GT64120State),
>> + VMSTATE_UINT32(regs[GT_TC2], GT64120State),
>> + VMSTATE_UINT32(regs[GT_TC3], GT64120State),
>> + VMSTATE_UINT32(regs[GT_TC_CONTROL], GT64120State),
>> + /* PCI Internal */
>> + VMSTATE_UINT32(regs[GT_PCI0_CMD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_CMD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_TOR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_BS_SCS10], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_BS_SCS32], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_BS_CS20], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_BS_CS3BT], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_IACK], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_IACK], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_BARE], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_PREFMBR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_SCS10_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_SCS32_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_CS20_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_CS3BT_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_SSCS10_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_SSCS32_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_SCS3BT_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_CMD], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_TOR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_BS_SCS10], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_BS_SCS32], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_BS_CS20], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_BS_CS3BT], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_BARE], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_PREFMBR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_SCS10_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_SCS32_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_CS20_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_CS3BT_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_SSCS10_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_SSCS32_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_SCS3BT_BAR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_CFGADDR], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI1_CFGDATA], GT64120State),
>> + VMSTATE_UINT32(regs[GT_PCI0_CFGADDR], GT64120State),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>
> Disclaimer: my knowledge of VMSTATE is quite limited.
>
> This looks like we are basically saving all registers (which is
> correct), so I do wonder if we shouldn't use a VMSTATE_ARRAY function,
> for example VMSTATE_VARRAY_UINT32.
Yeh, I wondered that too as the device I copied did that for some parts.
However it appeared that the GT64xxx register address space is fairly
sparse. It only saves 118 of the 154 registers #defined, out of an array
of GT_REGS=1024 elements) so I went for a 1:1 conversion of the original
patch. TBH I haven't looked deeply enough yet to know whether the other
registers should be saved.
Should be easy enough to convert to save the whole array if you think
that'd be better/safer though.
Cheers
James
>
>> static void gt64120_writel (void *opaque, hwaddr addr,
>> uint64_t val, unsigned size)
>> {
>> @@ -1174,9 +1324,11 @@ static const TypeInfo gt64120_pci_info = {
>>
>> static void gt64120_class_init(ObjectClass *klass, void *data)
>> {
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
>>
>> sdc->init = gt64120_init;
>> + dc->vmsd = &vmstate_gt64120;
>> }
>>
>> static const TypeInfo gt64120_info = {
>> --
>> 1.9.3
>>
>>
>
next prev parent reply other threads:[~2014-06-19 15:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-17 23:10 [Qemu-devel] [PATCH v2] hw/mips: gt64xxx_pci: Add VMStateDescription James Hogan
2014-06-19 15:25 ` Aurelien Jarno
2014-06-19 15:46 ` James Hogan [this message]
2014-06-19 15:51 ` Aurelien Jarno
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53A305BB.7030405@imgtec.com \
--to=james.hogan@imgtec.com \
--cc=aurelien@aurel32.net \
--cc=qemu-devel@nongnu.org \
--cc=sanjayl@kymasys.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).