On Wed, Jun 14, 2017 at 09:46:20AM -0300, Eduardo Habkost wrote:
> On Wed, Jun 14, 2017 at 12:58:04PM +0300, Roman Kagan wrote:
> > On Tue, Jun 13, 2017 at 03:34:34PM -0300, Eduardo Habkost wrote:
> > > On Tue, Jun 06, 2017 at 09:19:38PM +0300, Roman Kagan wrote:
> > > > +static Property synic_props[] = {
> > > > + DEFINE_PROP_BOOL("enabled", SynICState, enabled, false),
> > > > + DEFINE_PROP_UINT64("msg-page-addr", SynICState, msg_page_addr, 0),
> > > > + DEFINE_PROP_UINT64("evt-page-addr", SynICState, evt_page_addr, 0),
> > >
> > > What do you need the QOM properties for? Are they supposed to be
> > > configurable by the user?
> >
> > No. Actually I wanted to define them as read-only, but I failed to find
> > a way to do so.
> >
> > > Are they supposed to be queried from outside QEMU? On which cases?
> >
> > ATM I only see them as informational fields that may prove useful during
> > debugging.
> >
>
> You can use object_property_add_uint64_ptr() on instance_init to
> add read-only properties.
>
> If the enabled/msg_page_addr/evt_page_addr struct fields exist
> only because of the properties, you can also use
> object_property_add() and write your own getter that will query
> the MSRs only when reading the property. I normally don't like
> custom getters/setters, but it sounds acceptable for a read-only
> property that's only for debugging.
Actually that was what I did at first, but then I decided it was useful
to have the fields directly on SynICState (see followup patches).
> Either way you choose to implement it, I would add a comment
> noting that the properties are there just for debugging.
>
> BTW, would you still want to add this amount of boilerplate code
> just for debugging if we had a generic msr_get HMP command?
> There's a series in the list (submitted on April) adding msr_get
> and msr_set HMP commands, but it was never merged.
Yes I guess it would do. So I'm now tempted to just drop the
properties, and leave the fields invisible through QOM.
Thanks,
Roman.
