From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Corey Minyard <cminyard@mvista.com>,
Corey Minyard <minyard@acm.org>,
QEMU Developers <qemu-devel@nongnu.org>,
Paolo Bonzini <pbonzini@redhat.com>,
"Michael S . Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom
Date: Mon, 12 Nov 2018 17:38:31 +0000 [thread overview]
Message-ID: <20181112173830.GH2293@work-vm> (raw)
In-Reply-To: <CAFEAcA9=UK2P7J_8yvvf38Jvyr-v96uvjVEY-oBcOS-VF5DFZg@mail.gmail.com>
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 9 November 2018 at 17:19, Corey Minyard <cminyard@mvista.com> wrote:
> > On 11/9/18 9:02 AM, Peter Maydell wrote:
> >> The data provided by the caller is only the initialization
> >> data. So I think the device should create its own memory
> >> to copy that init data into, and migrate that ram, not
> >> wherever the initialization data lives. (Currently
> >> this "copy the data into our own ram" happens in the
> >> smbus_eeprom_init() wrapper, but we should do it in the
> >> device realize function instead.)
> >
> >
> > That's what I would like, but should I get rid of the "DEFINE_PROP_PTR"?
> > I don't know the value of creating a properly like this, since the user
> > can't set it and can't see it. Plus the comments in the code say that
> > it should be gotten rid of at some point.
> >
> > But if I store off the initialization data and keep the actual data in
> > a separate memory created by the realize function, that should work
> > find.
>
> Well, you still have to pass the init data to the device
> somehow, so I think a pointer property is not a terrible
> way to do that.
>
> >> Also there seem to be unresolved questions about what happens
> >> on reset -- should the EEPROM revert back to the initial
> >> contents? We don't do that at the moment, but this breaks
> >> the usual QEMU pattern that reset is as if you'd just
> >> completely restarted QEMU.
> >
> >
> > I would consider this part of the hardware, like data on a disk drive,
> > so it seem better to leave it alone after a reset. But it's not quite
> > the same. So I'm not sure.
>
> That would require us to support backing it properly with a block
> device, like the pflash flash devices, I think. (This would
> be the long term way to be able to dump the pointer property,
> in favour of a block backend property.)
There's a number of separate questions:
a) Can the guest write to the EEPROM or are we just treating it as a
ROM?
b) If a guest writes to the EEPROM and then resets does the data stay
there [I'd expect so, it's an EEPROM]
c) It the guest writes to the EEPROM and then quits qemu and restarts
does the data stay there?
d) If the guest is migrated does it keep the data it's written - I'd say
it must because otherwise it would get confused.
(c) is where it starts looking like a pflash - which does get a bit
messy.
Dave
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
next prev parent reply other threads:[~2018-11-12 17:38 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-07 15:54 [Qemu-devel] [PATCH 0/3] Fix/add vmstate handling in some I2C code minyard
2018-11-07 15:54 ` [Qemu-devel] [PATCH 1/3] i2c:pm_smbus: Fix state transfer minyard
2018-11-12 17:47 ` Dr. David Alan Gilbert
2018-11-12 20:32 ` Corey Minyard
2018-11-07 15:54 ` [Qemu-devel] [PATCH 2/3] i2c: Add an SMBus vmstate structure minyard
2018-11-08 14:23 ` Philippe Mathieu-Daudé
2018-11-08 14:40 ` Peter Maydell
2018-11-08 14:48 ` Philippe Mathieu-Daudé
2018-11-07 15:54 ` [Qemu-devel] [PATCH 3/3] i2c: Add vmstate handling to the smbus eeprom minyard
2018-11-08 14:08 ` Peter Maydell
2018-11-08 17:58 ` Corey Minyard
2018-11-08 18:03 ` Peter Maydell
2018-11-09 14:56 ` Corey Minyard
2018-11-09 15:02 ` Peter Maydell
2018-11-09 17:19 ` Corey Minyard
2018-11-09 17:53 ` Peter Maydell
2018-11-12 17:38 ` Dr. David Alan Gilbert [this message]
2018-11-12 17:41 ` Peter Maydell
2018-11-12 17:28 ` Dr. David Alan Gilbert
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=20181112173830.GH2293@work-vm \
--to=dgilbert@redhat.com \
--cc=cminyard@mvista.com \
--cc=minyard@acm.org \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/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).