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

[Eduardo Habkost <ehabkost@redhat.com>]

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