[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/23] hyperv: qdev-ify SynIC
From: |
Roman Kagan |
Subject: |
Re: [Qemu-devel] [PATCH 13/23] hyperv: qdev-ify SynIC |
Date: |
Wed, 14 Jun 2017 18:11:03 +0300 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
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.
- [Qemu-devel] [PATCH 09/23] hyperv: synic: only setup ack notifier if there's a callback, (continued)
- [Qemu-devel] [PATCH 09/23] hyperv: synic: only setup ack notifier if there's a callback, Roman Kagan, 2017/06/06
- [Qemu-devel] [PATCH 12/23] hyperv: make HvSintRoute reference-counted, Roman Kagan, 2017/06/06
- [Qemu-devel] [PATCH 10/23] hyperv: allow passing arbitrary data to sint ack callback, Roman Kagan, 2017/06/06
- [Qemu-devel] [PATCH 14/23] kvm-all: make async_safe_run_on_cpu safe on kvm too, Roman Kagan, 2017/06/06
- [Qemu-devel] [PATCH 13/23] hyperv: qdev-ify SynIC, Roman Kagan, 2017/06/06
[Qemu-devel] [PATCH 15/23] hyperv: make overlay pages for SynIC, Roman Kagan, 2017/06/06
[Qemu-devel] [PATCH 19/23] hyperv: process SIGNAL_EVENT hypercall, Roman Kagan, 2017/06/06
[Qemu-devel] [PATCH 18/23] hyperv: add synic event flag signaling, Roman Kagan, 2017/06/06
[Qemu-devel] [PATCH 17/23] hyperv: add synic message delivery, Roman Kagan, 2017/06/06