Re: [Qemu-devel] vmbus bridge: machine property or device?

From: Roman Kagan <rkagan@virtuozzo.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Evgeny Yakovlev <eyakovlev@virtuozzo.com>,
	Alexander Graf <agraf@suse.de>,
	qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	"Denis V. Lunev" <den@openvz.org>
Subject: Re: [Qemu-devel] vmbus bridge: machine property or device?
Date: Thu, 13 Apr 2017 20:04:03 +0300	[thread overview]
Message-ID: <20170413170403.GE14312@rkaganb.sw.ru> (raw)
In-Reply-To: <20170413164457.GX32646@thinpad.lan.raisama.net>

On Thu, Apr 13, 2017 at 01:44:57PM -0300, Eduardo Habkost wrote:
> On Thu, Apr 13, 2017 at 06:15:34PM +0300, Roman Kagan wrote:
> > Can you (or anybody else) please help me decide if I need
> > TYPE_SYS_BUS_DEVICE?  Logically the VMBus bridge is "attached directly
> > to the main system bus" as written at the top of include/hw/sysbus.h.
> 
> I think that documentation was written before we supported
> bus-less devices.

I see.  Then we should be good subclassing TYPE_DEVICE.

> > OTOH we use neither mmio nor pio members of SysBusDevice; nor do we
> > currently use any of its *_irq helpers, but we may eventually need to
> > (the guests require VMBus to announce two IRQs in its ACPI description
> > but nothing seems to use them so we just hardcode two (almost) random
> > numbers).
> 
> I'm not sure about the consequences of simply connecting IRQs
> inside ->realize() without using the sysbus *_irq helpers.

We don't connect IRQs; we just compose the appropriate AML fragments
with arbitrarily chosen irq numbers and forget about them.  The guests
refuse to use VMBus if the respective clauses aren't in the ACPI, but
don't seem to care if they point nowhere.

> > Is there an idiom to express that no more than a single vmbus-bridge can
> > be present in the system?  Or the only way to ensure that is with a
> > static or a class variable and checking / setting it in ->realize?
> 
> I wouldn't use a static or class variable. You have some
> alternatives:
> 
> You could check if a TYPE_VMBUS device already exists anywhere in
> the device tree, or always create it at a specific QOM path.
> 
> Or, if your device is 100% specific for PC machines, you could
> add a PCMachineState struct field pointing to the existing vmbus
> device.

Makes sense, thanks!
(And IIUC there's no generic way to express this, like, e.g. with a
class property ->single_instance or ->max_instances=1, right?)

Thanks a lot for the advices, we'll be back with a patchset ;)
Roman.