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:
> > Make Hyper-V SynIC a device which is attached as a child to X86CPU. For
> > now it only makes SynIC visibile in the qom hierarchy and exposes a few
> > properties which are maintained in sync with the respecitve msrs of the
> > parent cpu.
[...]
> > +static SynICState *get_synic(X86CPU *cpu)
> > +{
> > + SynICState *synic =
> > + SYNIC(object_resolve_path_component(OBJECT(cpu), "synic"));
> > + assert(synic);
> > + return synic;
>
> How often this will run? I think a new X86CPU struct field would
> be acceptable to avoid resolving a QOM path on every hyperv exit.
It's only used at the points where synic state is updated: at
realize/save/load/unrealize and when the guest configures synics via
msrs (i.e. a few times per cpu at guest startup). None of those is
performance-critical.
> Do you even need QOM for this?
I need to reach the synic from the cpu on slow paths, and IMHO the
pointer on X86CPU is not justified here. Besides there are two memory
regions associated with every synic (in a followup patch) and I thought
it made sense to have a dedicated parent QOM node for them.
> > +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.
Roman.
