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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.