All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] vmbus bridge: machine property or device?
@ 2017-04-11 20:58 Roman Kagan
  2017-04-12 15:18 ` Markus Armbruster
  0 siblings, 1 reply; 11+ messages in thread
From: Roman Kagan @ 2017-04-11 20:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Evgeny Yakovlev, Denis V. Lunev

While hammering out the VMBus / storage series, we've been struggling to
figure out the best practices solution to the following problem:

VMBus is provided by a vmbus bridge; it appears the most natural to have
it subclassed from SysBusDevice.  There can only be one VMBus in the
VM.

Now the question is how to add it to the system:

1) with a boolean machine property "vmbus" that would trigger the
   creation of the VMBus bridge; its class would have
   ->cannot_instantiate_with_device_add_yet = true


2) with a regular -device option; this would require setting
   ->has_dynamic_sysbus = true for i440fx machines (q35 already have it)


3) anything else


So far we went with 1) but since it's essentially the API the management
layer would have to use we'd like to get it right from the beginning.

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-11 20:58 [Qemu-devel] vmbus bridge: machine property or device? Roman Kagan
@ 2017-04-12 15:18 ` Markus Armbruster
  2017-04-12 20:07   ` Eduardo Habkost
  2017-04-13 14:08   ` Roman Kagan
  0 siblings, 2 replies; 11+ messages in thread
From: Markus Armbruster @ 2017-04-12 15:18 UTC (permalink / raw)
  To: Roman Kagan
  Cc: qemu-devel, Evgeny Yakovlev, Denis V. Lunev, Eduardo Habkost,
	Peter Maydell, Alexander Graf, Paolo Bonzini

Cc'ing a few more people who might have a reasoned opinion.

Roman Kagan <rkagan@virtuozzo.com> writes:

> While hammering out the VMBus / storage series, we've been struggling to
> figure out the best practices solution to the following problem:
>
> VMBus is provided by a vmbus bridge; it appears the most natural to have
> it subclassed from SysBusDevice.  There can only be one VMBus in the
> VM.

TYPE_DEVICE unless you actually need something TYPE_SYS_BUS_DEVICE
provides.

> Now the question is how to add it to the system:
>
> 1) with a boolean machine property "vmbus" that would trigger the
>    creation of the VMBus bridge; its class would have
>    ->cannot_instantiate_with_device_add_yet = true

This makes it an optional onboard device.  Similar ones exist already,
e.g. various optional onboard USB host controllers controlled by machine
property "usb".

> 2) with a regular -device option; this would require setting
>    ->has_dynamic_sysbus = true for i440fx machines (q35 already have it)

This makes it a pluggable sysbus device.

I'd be tempted to leave old i400FX rot in peace, but your use case may
not allow that.

>
> 3) anything else
>
>
> So far we went with 1) but since it's essentially the API the management
> layer would have to use we'd like to get it right from the beginning.

Asking for advice here is a good idea.

Anyone?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-12 15:18 ` Markus Armbruster
@ 2017-04-12 20:07   ` Eduardo Habkost
  2017-04-13 15:15     ` Roman Kagan
  2017-04-13 14:08   ` Roman Kagan
  1 sibling, 1 reply; 11+ messages in thread
From: Eduardo Habkost @ 2017-04-12 20:07 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Roman Kagan, Peter Maydell, Evgeny Yakovlev, Alexander Graf,
	qemu-devel, Paolo Bonzini, Denis V. Lunev

On Wed, Apr 12, 2017 at 05:18:51PM +0200, Markus Armbruster wrote:
> Cc'ing a few more people who might have a reasoned opinion.
> 
> Roman Kagan <rkagan@virtuozzo.com> writes:
> 
> > While hammering out the VMBus / storage series, we've been struggling to
> > figure out the best practices solution to the following problem:
> >
> > VMBus is provided by a vmbus bridge; it appears the most natural to have
> > it subclassed from SysBusDevice.  There can only be one VMBus in the
> > VM.
> 
> TYPE_DEVICE unless you actually need something TYPE_SYS_BUS_DEVICE
> provides.
> 
> > Now the question is how to add it to the system:
> >
> > 1) with a boolean machine property "vmbus" that would trigger the
> >    creation of the VMBus bridge; its class would have
> >    ->cannot_instantiate_with_device_add_yet = true
> 
> This makes it an optional onboard device.  Similar ones exist already,
> e.g. various optional onboard USB host controllers controlled by machine
> property "usb".
> 
> > 2) with a regular -device option; this would require setting
> >    ->has_dynamic_sysbus = true for i440fx machines (q35 already have it)
> 
> This makes it a pluggable sysbus device.
> 
> I'd be tempted to leave old i400FX rot in peace, but your use case may
> not allow that.

I have sent a RFC some time ago that replaces the all-or-nothing
has_dynamic_sysbus flag with an explicit sysbus device whitelist,
so i440fx wouldn't be a big problem. But as you noted above, if
you don't need TYPE_SYS_BUS_DEVIC, you can just use TYPE_DEVICE.

> 
> >
> > 3) anything else
> >
> >
> > So far we went with 1) but since it's essentially the API the management
> > layer would have to use we'd like to get it right from the beginning.
> 
> Asking for advice here is a good idea.
> 
> Anyone?
> 

I would go with (2) instead of (1): it allows more flexibility in
case the device needs additional arguments, and will
automatically benefit from (present and future) mechanisms for
reporting available device-types and buses. Asking QEMU if
"-device FOO" is supported is easy and reliable; the mechanisms
for asking QEMU about supported "-machine" options are obscure
and probably not well-tested.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-12 15:18 ` Markus Armbruster
  2017-04-12 20:07   ` Eduardo Habkost
@ 2017-04-13 14:08   ` Roman Kagan
  1 sibling, 0 replies; 11+ messages in thread
From: Roman Kagan @ 2017-04-13 14:08 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Evgeny Yakovlev, Denis V. Lunev, Eduardo Habkost,
	Peter Maydell, Alexander Graf, Paolo Bonzini

On Wed, Apr 12, 2017 at 05:18:51PM +0200, Markus Armbruster wrote:
> Roman Kagan <rkagan@virtuozzo.com> writes:
> > VMBus is provided by a vmbus bridge; it appears the most natural to have
> > it subclassed from SysBusDevice.  There can only be one VMBus in the
> > VM.
> 
> TYPE_DEVICE unless you actually need something TYPE_SYS_BUS_DEVICE
> provides.

I thought VMBus looked similar to an ISA or PCI bus in its relation to
the system, and those bridges were TYPE_SYS_BUS_DEVICE.

If it's subclassed directly from TYPE_DEVICE how is it connected to the
rest of the QOM tree?

> > Now the question is how to add it to the system:
> >
> > 1) with a boolean machine property "vmbus" that would trigger the
> >    creation of the VMBus bridge; its class would have
> >    ->cannot_instantiate_with_device_add_yet = true
> 
> This makes it an optional onboard device.  Similar ones exist already,
> e.g. various optional onboard USB host controllers controlled by machine
> property "usb".
> 
> > 2) with a regular -device option; this would require setting
> >    ->has_dynamic_sysbus = true for i440fx machines (q35 already have it)
> 
> This makes it a pluggable sysbus device.

VMBus can't be hot-plugged into the system.  Besides, there can be no
more than one VMBus in the system, so multiple -device vmbus-bridge
options would not be allowed; I thought this semantics was clearer with
a machine option.

> I'd be tempted to leave old i400FX rot in peace, but your use case may
> not allow that.

It doesn't ;)  Besides, we've had no specal cases for i440fx vs q35 in
our VMBus code so far, so I don't see any reason to not to support both.

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-12 20:07   ` Eduardo Habkost
@ 2017-04-13 15:15     ` Roman Kagan
  2017-04-13 16:44       ` Eduardo Habkost
  0 siblings, 1 reply; 11+ messages in thread
From: Roman Kagan @ 2017-04-13 15:15 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Markus Armbruster, Peter Maydell, Evgeny Yakovlev,
	Alexander Graf, qemu-devel, Paolo Bonzini, Denis V. Lunev

On Wed, Apr 12, 2017 at 05:07:20PM -0300, Eduardo Habkost wrote:
> On Wed, Apr 12, 2017 at 05:18:51PM +0200, Markus Armbruster wrote:
> > Roman Kagan <rkagan@virtuozzo.com> writes:
> > > VMBus is provided by a vmbus bridge; it appears the most natural to have
> > > it subclassed from SysBusDevice.  There can only be one VMBus in the
> > > VM.
> > 
> > TYPE_DEVICE unless you actually need something TYPE_SYS_BUS_DEVICE
> > provides.
> > 
> > > Now the question is how to add it to the system:
> > >
> > > 1) with a boolean machine property "vmbus" that would trigger the
> > >    creation of the VMBus bridge; its class would have
> > >    ->cannot_instantiate_with_device_add_yet = true
> > 
> > This makes it an optional onboard device.  Similar ones exist already,
> > e.g. various optional onboard USB host controllers controlled by machine
> > property "usb".
> > 
> > > 2) with a regular -device option; this would require setting
> > >    ->has_dynamic_sysbus = true for i440fx machines (q35 already have it)
> > 
> > This makes it a pluggable sysbus device.
> > 
> > I'd be tempted to leave old i400FX rot in peace, but your use case may
> > not allow that.
> 
> I have sent a RFC some time ago that replaces the all-or-nothing
> has_dynamic_sysbus flag with an explicit sysbus device whitelist,
> so i440fx wouldn't be a big problem.

I looked at the patchset, and it seems to adress this very nicely,
indeed.

> But as you noted above, if
> you don't need TYPE_SYS_BUS_DEVIC, you can just use TYPE_DEVICE.

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.
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).

> > > 3) anything else
> > >
> > >
> > > So far we went with 1) but since it's essentially the API the management
> > > layer would have to use we'd like to get it right from the beginning.
> > 
> > Asking for advice here is a good idea.
> > 
> > Anyone?
> > 
> 
> I would go with (2) instead of (1): it allows more flexibility in
> case the device needs additional arguments, and will
> automatically benefit from (present and future) mechanisms for
> reporting available device-types and buses. Asking QEMU if
> "-device FOO" is supported is easy and reliable; the mechanisms
> for asking QEMU about supported "-machine" options are obscure
> and probably not well-tested.

Good point in favor of -device, thanks.

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?

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-13 15:15     ` Roman Kagan
@ 2017-04-13 16:44       ` Eduardo Habkost
  2017-04-13 16:58         ` Peter Maydell
  2017-04-13 17:04         ` Roman Kagan
  0 siblings, 2 replies; 11+ messages in thread
From: Eduardo Habkost @ 2017-04-13 16:44 UTC (permalink / raw)
  To: Roman Kagan, Markus Armbruster, Peter Maydell, Evgeny Yakovlev,
	Alexander Graf, qemu-devel, Paolo Bonzini, Denis V. Lunev

On Thu, Apr 13, 2017 at 06:15:34PM +0300, Roman Kagan wrote:
> On Wed, Apr 12, 2017 at 05:07:20PM -0300, Eduardo Habkost wrote:
> > On Wed, Apr 12, 2017 at 05:18:51PM +0200, Markus Armbruster wrote:
> > > Roman Kagan <rkagan@virtuozzo.com> writes:
> > > > VMBus is provided by a vmbus bridge; it appears the most natural to have
> > > > it subclassed from SysBusDevice.  There can only be one VMBus in the
> > > > VM.
> > > 
> > > TYPE_DEVICE unless you actually need something TYPE_SYS_BUS_DEVICE
> > > provides.
> > > 
> > > > Now the question is how to add it to the system:
> > > >
> > > > 1) with a boolean machine property "vmbus" that would trigger the
> > > >    creation of the VMBus bridge; its class would have
> > > >    ->cannot_instantiate_with_device_add_yet = true
> > > 
> > > This makes it an optional onboard device.  Similar ones exist already,
> > > e.g. various optional onboard USB host controllers controlled by machine
> > > property "usb".
> > > 
> > > > 2) with a regular -device option; this would require setting
> > > >    ->has_dynamic_sysbus = true for i440fx machines (q35 already have it)
> > > 
> > > This makes it a pluggable sysbus device.
> > > 
> > > I'd be tempted to leave old i400FX rot in peace, but your use case may
> > > not allow that.
> > 
> > I have sent a RFC some time ago that replaces the all-or-nothing
> > has_dynamic_sysbus flag with an explicit sysbus device whitelist,
> > so i440fx wouldn't be a big problem.
> 
> I looked at the patchset, and it seems to adress this very nicely,
> indeed.
> 
> > But as you noted above, if
> > you don't need TYPE_SYS_BUS_DEVIC, you can just use TYPE_DEVICE.
> 
> 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.

> 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. I hope
others can clarify this.

> 
> > > > 3) anything else
> > > >
> > > >
> > > > So far we went with 1) but since it's essentially the API the management
> > > > layer would have to use we'd like to get it right from the beginning.
> > > 
> > > Asking for advice here is a good idea.
> > > 
> > > Anyone?
> > > 
> > 
> > I would go with (2) instead of (1): it allows more flexibility in
> > case the device needs additional arguments, and will
> > automatically benefit from (present and future) mechanisms for
> > reporting available device-types and buses. Asking QEMU if
> > "-device FOO" is supported is easy and reliable; the mechanisms
> > for asking QEMU about supported "-machine" options are obscure
> > and probably not well-tested.
> 
> Good point in favor of -device, thanks.
> 
> 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.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-13 16:44       ` Eduardo Habkost
@ 2017-04-13 16:58         ` Peter Maydell
  2017-04-13 18:15           ` Eduardo Habkost
  2017-04-13 21:10           ` Roman Kagan
  2017-04-13 17:04         ` Roman Kagan
  1 sibling, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2017-04-13 16:58 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Roman Kagan, Markus Armbruster, Evgeny Yakovlev, Alexander Graf,
	QEMU Developers, Paolo Bonzini, Denis V. Lunev

On 13 April 2017 at 17:44, Eduardo Habkost <ehabkost@redhat.com> 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.

The major question to ask to determine whether you need to be
a sysbus device is how does the guest interact with this thing?
In particular, does it have memory mapped registers (and is it
not part of some other more specific interface like a PCI device) ?

> I'm not sure about the consequences of simply connecting IRQs
> inside ->realize() without using the sysbus *_irq helpers. I hope
> others can clarify this.

Connecting what IRQs to what? If your device has an IRQ
then it isn't the device's job to connect it up -- it is
the job of the machine model, because only the machine
model knows what the IRQ controller is, whether the IRQ
needs to be advertised via ACPI or device tree, and so on.
You can do that without sysbus though, by using the core
DeviceState's qdev_init_gpio_* APIs. (NB that if you do
this then you're by necessity not a device creatable
with -device, since the board code has to wire you up.)

If you're not using sysbus then watch out for reset:
all sysbus devices get automatically reset on QEMU
system reset; if you're directly using the DeviceState
baseclass then you have to arrange reset for yourself
somehow.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-13 16:44       ` Eduardo Habkost
  2017-04-13 16:58         ` Peter Maydell
@ 2017-04-13 17:04         ` Roman Kagan
  1 sibling, 0 replies; 11+ messages in thread
From: Roman Kagan @ 2017-04-13 17:04 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Markus Armbruster, Peter Maydell, Evgeny Yakovlev,
	Alexander Graf, qemu-devel, Paolo Bonzini, Denis V. Lunev

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-13 16:58         ` Peter Maydell
@ 2017-04-13 18:15           ` Eduardo Habkost
  2017-04-13 21:57             ` Peter Maydell
  2017-04-13 21:10           ` Roman Kagan
  1 sibling, 1 reply; 11+ messages in thread
From: Eduardo Habkost @ 2017-04-13 18:15 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Roman Kagan, Markus Armbruster, Evgeny Yakovlev, Alexander Graf,
	QEMU Developers, Paolo Bonzini, Denis V. Lunev

On Thu, Apr 13, 2017 at 05:58:51PM +0100, Peter Maydell wrote:
> On 13 April 2017 at 17:44, Eduardo Habkost <ehabkost@redhat.com> 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.
> 
> The major question to ask to determine whether you need to be
> a sysbus device is how does the guest interact with this thing?
> In particular, does it have memory mapped registers (and is it
> not part of some other more specific interface like a PCI device) ?
> 
> > I'm not sure about the consequences of simply connecting IRQs
> > inside ->realize() without using the sysbus *_irq helpers. I hope
> > others can clarify this.
> 
> Connecting what IRQs to what? If your device has an IRQ
> then it isn't the device's job to connect it up -- it is
> the job of the machine model, because only the machine
> model knows what the IRQ controller is, whether the IRQ
> needs to be advertised via ACPI or device tree, and so on.

I can't say it's a good idea, but it looks like there are devices
that do that (probably they are machine-specific, so they know
which machine they are being connected to?).

I looked for realize functions that call sysbus_mmio_map() or
get_system_memory() directly, and I've found a few:

 13  static void fsl_imx6_realize(DeviceState *dev, Error **errp)
 11  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
 11  static void fsl_imx25_realize(DeviceState *dev, Error **errp)
 10  static void fsl_imx31_realize(DeviceState *dev, Error **errp)
 10  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
  7  static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
  4  static void aw_a10_realize(DeviceState *dev, Error **errp)
  2  static void spapr_phb_realize(DeviceState *dev, Error **errp)
  2  static void raven_realize(PCIDevice *d, Error **errp)
  2  static void digic_realize(DeviceState *dev, Error **errp)
  2  static void bcm2836_realize(DeviceState *dev, Error **errp)
  1  static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
  1  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
  1  static void vtd_realize(DeviceState *dev, Error **errp)
  1  static void vt82c686b_realize(PCIDevice *d, Error **errp)
  1  static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
  1  static void rs6000mc_realize(DeviceState *dev, Error **errp)
  1  static void realview_mpcore_realize(DeviceState *dev, Error **errp)
  1  static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
  1  static void prep_systemio_realize(DeviceState *dev, Error **errp)
  1  static void pnv_chip_realize(DeviceState *dev, Error **errp)
  1  static void piix3_realize(PCIDevice *dev, Error **errp)
  1  static void pci_ebus_realize(PCIDevice *pci_dev, Error **errp)
  1  static void ich9_lpc_realize(PCIDevice *d, Error **errp)
  1  static void i82378_realize(PCIDevice *pci, Error **errp)
  1  static void bonito_realize(PCIDevice *dev, Error **errp)
  1  static void amdvi_realize(DeviceState *dev, Error **err)

And the ones that call sysbus_connect_irq() directly:

  9  static void fsl_imx6_realize(DeviceState *dev, Error **errp)
  8  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
  8  static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
  7  static void fsl_imx25_realize(DeviceState *dev, Error **errp)
  6  static void fsl_imx31_realize(DeviceState *dev, Error **errp)
  5  static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
  4  static void aw_a10_realize(DeviceState *dev, Error **errp)
  4  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
  1  static void realview_mpcore_realize(DeviceState *dev, Error **errp)
  1  static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
  1  static void macio_oldworld_realize(PCIDevice *d, Error **errp)
  1  static void macio_newworld_realize(PCIDevice *d, Error **errp)
  1  static void bcm2836_realize(DeviceState *dev, Error **errp)
  1  static void armv7m_realize(DeviceState *dev, Error **errp)
  1  static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
  1  static void a9mp_priv_realize(DeviceState *dev, Error **errp)

> You can do that without sysbus though, by using the core
> DeviceState's qdev_init_gpio_* APIs. (NB that if you do
> this then you're by necessity not a device creatable
> with -device, since the board code has to wire you up.)

Isn't "the board code has to wire you up" true for both sysbus
and bus-less devices?

> 
> If you're not using sysbus then watch out for reset:
> all sysbus devices get automatically reset on QEMU
> system reset; if you're directly using the DeviceState
> baseclass then you have to arrange reset for yourself
> somehow.
> 
> thanks
> -- PMM

-- 
Eduardo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-13 16:58         ` Peter Maydell
  2017-04-13 18:15           ` Eduardo Habkost
@ 2017-04-13 21:10           ` Roman Kagan
  1 sibling, 0 replies; 11+ messages in thread
From: Roman Kagan @ 2017-04-13 21:10 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Eduardo Habkost, Markus Armbruster, Evgeny Yakovlev,
	Alexander Graf, QEMU Developers, Paolo Bonzini, Denis V. Lunev

On Thu, Apr 13, 2017 at 05:58:51PM +0100, Peter Maydell wrote:
> On 13 April 2017 at 17:44, Eduardo Habkost <ehabkost@redhat.com> 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.
> 
> The major question to ask to determine whether you need to be
> a sysbus device is how does the guest interact with this thing?
> In particular, does it have memory mapped registers (and is it
> not part of some other more specific interface like a PCI device) ?

The interaction happens through a set of dedicated MSRs, shared pages,
hypercalls, and "synthetic" interrupts (non-ioapic).  No pio, mmio,
gpio/irq.

VMBus annonces its presence by a device entry in ACPI with two IRQs;
we've seen no need to wire those IRQs so far, and the guests don't seem
to use them.  That's it; everything else is VMBus-specific.

> > I'm not sure about the consequences of simply connecting IRQs
> > inside ->realize() without using the sysbus *_irq helpers. I hope
> > others can clarify this.
> 
> Connecting what IRQs to what? If your device has an IRQ
> then it isn't the device's job to connect it up -- it is
> the job of the machine model, because only the machine
> model knows what the IRQ controller is, whether the IRQ
> needs to be advertised via ACPI or device tree, and so on.
> You can do that without sysbus though, by using the core
> DeviceState's qdev_init_gpio_* APIs. (NB that if you do
> this then you're by necessity not a device creatable
> with -device, since the board code has to wire you up.)
> 
> If you're not using sysbus then watch out for reset:
> all sysbus devices get automatically reset on QEMU
> system reset; if you're directly using the DeviceState
> baseclass then you have to arrange reset for yourself
> somehow.

Thanks!
Roman.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] vmbus bridge: machine property or device?
  2017-04-13 18:15           ` Eduardo Habkost
@ 2017-04-13 21:57             ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2017-04-13 21:57 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Roman Kagan, Markus Armbruster, Evgeny Yakovlev, Alexander Graf,
	QEMU Developers, Paolo Bonzini, Denis V. Lunev

On 13 April 2017 at 19:15, Eduardo Habkost <ehabkost@redhat.com> wrote:
> I can't say it's a good idea, but it looks like there are devices
> that do that (probably they are machine-specific, so they know
> which machine they are being connected to?).
>
> I looked for realize functions that call sysbus_mmio_map() or
> get_system_memory() directly, and I've found a few:

Yeah. Ideally these should get cleaned up. The way I think is
the right approach here is that the board code uses the
system memory address space to map its devices, and then
it passes a MemoryRegion to the SoC container device via a
QOM link property. The SoC then maps its devices inside that
MemoryRegion, and hands an MR to the CPU (and the CPU does
its accesses to that MR). You can see some of this with
stm32f205_soc.c and armv7m.c.

I think most of your examples are SoCs which are just
slightly old-style/lazy and using the system memory
address space directly (since the SoC covers the whole
of the address space anyway). The real bad code is devices
which directly map their own memory regions into the
system memory space even though they're just a leaf
device like a UART (common in not-yet-converted-to-QOM code).

> And the ones that call sysbus_connect_irq() directly:

Calling sysbus_connect_irq() is fine because it's
what you use to wire up a sysbus device's input IRQ
line to something else. If you (as an SoC container
object) have created both the IRQ chip and the other
device then it's also your job to wire the two together.

> Isn't "the board code has to wire you up" true for both sysbus
> and bus-less devices?

If you need no wiring up at all then it's ok.
hw/core/generic-loader.c is an example of that.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-04-14  2:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11 20:58 [Qemu-devel] vmbus bridge: machine property or device? Roman Kagan
2017-04-12 15:18 ` Markus Armbruster
2017-04-12 20:07   ` Eduardo Habkost
2017-04-13 15:15     ` Roman Kagan
2017-04-13 16:44       ` Eduardo Habkost
2017-04-13 16:58         ` Peter Maydell
2017-04-13 18:15           ` Eduardo Habkost
2017-04-13 21:57             ` Peter Maydell
2017-04-13 21:10           ` Roman Kagan
2017-04-13 17:04         ` Roman Kagan
2017-04-13 14:08   ` Roman Kagan

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.