Re: [Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough

[Michael S. Tsirkin]

On Thu, Jul 24, 2014 at 07:30:27PM +0800, Tiejun Chen wrote:
> Implement that pci

s/that/a/

> host bridge to specific

s/to specific/specific/

> to passthrough. Actually
> this just inherit

s/inherit/inherits/

> the standard one.
> 
> This is based on http://patchwork.ozlabs.org/patch/363810/.
> 
> Signed-off-by: Tiejun Chen <address@hidden>
> ---
>  hw/pci-host/piix.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index e0e0946..9feddf5 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/i386/ioapic.h"
>  #include "qapi/visitor.h"
> +#include "hw/xen/xen_pt.h"

What call needs this, exactly?

>  
>  /*
>   * I440FX chipset data sheet.
> @@ -44,6 +45,10 @@
>  #define I440FX_PCI_HOST_BRIDGE(obj) \
>      OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
>  

OK cool, but don't you want to put "igd" and "passthrough"
somewhere in the name? Maybe "legacy" as well since
future drivers will work with existing machine type, right?

Applies to functions and macro names as well.

> +#define TYPE_I440FX_XEN_PCI_DEVICE "i440FX-xen"
> +#define I440FX_XEN_PCI_DEVICE(obj) \
> +    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_XEN_PCI_DEVICE)
> +
>  typedef struct I440FXState {
>      PCIHostState parent_obj;
>      PcPciInfo pci_info;
> @@ -305,6 +310,16 @@ static int i440fx_initfn(PCIDevice *dev)
>      return 0;
>  }
>  
> +static int i440fx_xen_initfn(PCIDevice *dev)
> +{
> +    PCII440FXState *d = I440FX_XEN_PCI_DEVICE(dev);
> +
> +    dev->config[I440FX_SMRAM] = 0x02;
> +
> +    cpu_smm_register(&i440fx_set_smm, d);
> +    return 0;
> +}
> +
>  PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>                      int *piix3_devfn,
>                      ISABus **isa_bus, qemu_irq *pic,
> @@ -704,6 +719,33 @@ static const TypeInfo i440fx_info = {
>      .class_init    = i440fx_class_init,
>  };
>  
> +static void i440fx_xen_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->init = i440fx_xen_initfn;
> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
> +    k->revision = 0x02;
> +    k->class_id = PCI_CLASS_BRIDGE_ISA;
> +    dc->desc = "XEN Host bridge";
> +    dc->vmsd = &vmstate_i440fx;
> +    /*
> +     * PCI-facing part of the host bridge, not usable without the
> +     * host-facing part, which can't be device_add'ed, yet.
> +     */
> +    dc->cannot_instantiate_with_device_add_yet = true;
> +    dc->hotpluggable   = false;
> +}
> +
> +static const TypeInfo i440fx_xen_info = {
> +    .name          = TYPE_I440FX_XEN_PCI_DEVICE,
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCII440FXState),
> +    .class_init    = i440fx_xen_class_init,
> +};
> +
>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>                                                  PCIBus *rootbus)
>  {
> @@ -745,6 +787,7 @@ static const TypeInfo i440fx_pcihost_info = {
>  static void i440fx_register_types(void)
>  {
>      type_register_static(&i440fx_info);
> +    type_register_static(&i440fx_xen_info);
>      type_register_static(&piix3_info);
>      type_register_static(&piix3_xen_info);
>      type_register_static(&i440fx_pcihost_info);
> -- 
> 1.9.1