Re: [Qemu-devel] [PATCH 1/3] megasas: add MegaRAID SAS 2108 emulation

[Andreas Färber]

Am 28.04.2014 15:01, schrieb Hannes Reinecke:
> The 2108 chip supports MSI and MSI-X, so update the emulation
> to support both chips.
> 
> Signed-off-by: Hannes Reinecke <address@hidden>
> ---
>  hw/scsi/megasas.c        | 138 
> ++++++++++++++++++++++++++++++++++++++++++-----
>  hw/scsi/mfi.h            |   7 +++
>  include/hw/pci/pci_ids.h |   1 +
>  3 files changed, 134 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index baee46f..519e3bc 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
[...]
> @@ -2116,6 +2168,25 @@ static const VMStateDescription vmstate_megasas = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_megasas_gen2 = {
> +    .name = "megasas-gen2",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .minimum_version_id_old = 0,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> +        VMSTATE_MSIX(parent_obj, MegasasState),
> +
> +        VMSTATE_INT32(fw_state, MegasasState),
> +        VMSTATE_INT32(intr_mask, MegasasState),
> +        VMSTATE_INT32(doorbell, MegasasState),
> +        VMSTATE_UINT64(reply_queue_pa, MegasasState),
> +        VMSTATE_UINT64(consumer_pa, MegasasState),
> +        VMSTATE_UINT64(producer_pa, MegasasState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static void megasas_scsi_uninit(PCIDevice *d)
>  {
>      MegasasState *s = MEGASAS(d);
[...]
> @@ -2278,9 +2357,44 @@ static const TypeInfo megasas_info = {
>      .class_init = megasas_class_init,
>  };
>  
> +static Property megasas_gen2_properties[] = {
> +    DEFINE_PROP_UINT32("max_sge", MegasasState, fw_sge,
> +                       MEGASAS_DEFAULT_SGE),
> +    DEFINE_PROP_UINT32("max_cmds", MegasasState, fw_cmds,
> +                       MEGASAS_DEFAULT_FRAMES),
> +    DEFINE_PROP_STRING("hba_serial", MegasasState, hba_serial),
> +    DEFINE_PROP_UINT64("sas_address", MegasasState, sas_addr, 0),
> +    DEFINE_PROP_BIT("use_msi", MegasasState, flags,
> +                    MEGASAS_FLAG_USE_MSI, true),
> +    DEFINE_PROP_BIT("use_msix", MegasasState, flags,
> +                    MEGASAS_FLAG_USE_MSIX, true),
> +    DEFINE_PROP_BIT("use_jbod", MegasasState, flags,
> +                    MEGASAS_FLAG_USE_JBOD, false),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void megasas_gen2_class_init(ObjectClass *oc, void *data)
> +{
> +    PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    pc->device_id = PCI_DEVICE_ID_LSI_SAS0079;
> +    pc->subsystem_id = 0x9261;
> +    dc->props = megasas_gen2_properties;
> +    dc->desc = "LSI MegaRAID SAS 2108";
> +    dc->vmsd = &vmstate_megasas_gen2;
> +}
> +
> +static TypeInfo megasas_gen2_info = {

static const

> +    .name = TYPE_MEGASAS_GEN2,
> +    .parent = TYPE_MEGASAS,
> +    .class_init = megasas_gen2_class_init,
> +};
> +
>  static void megasas_register_types(void)
>  {
>      type_register_static(&megasas_info);
> +    type_register_static(&megasas_gen2_info);
>  }
>  
>  type_init(megasas_register_types)

I note that there is still no qtest in this series. We need to draw a
line somewhere.

Have you tested this device at all? I would expect it to fail horribly
due to the "use_jbod" property being defined on derived and parent type!

Further, deriving the new model from the original one conflicts with my
series for QOM'ifying ->vmsd similar to ->props. Please use an abstract
base type with dc->vmsd == NULL if we need to keep the VMSDs separate.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg