Re: [Qemu-devel] [PATCH][v9] megasas: LSI Megaraid SAS HBA emulation

[Anthony Liguori <anthony@codemonkey.ws>]

On 01/16/2012 03:18 AM, Hannes Reinecke wrote:
> On 01/13/2012 05:14 PM, Anthony Liguori wrote:
>> On 01/13/2012 05:19 AM, Hannes Reinecke wrote:
>>> This patch adds an emulation for the LSI Megaraid SAS 8708EM2 HBA.
>>> I've tested it to work with Linux, Windows Vista, and Windows7.
>>>
>>> Changes since v8:
>>> - Remove 'disable' keyword from trace definitions
>>> - Convert hand-crafted debugging statements with trace
>>>     definitions
>>> - Treat 'context' tag as little endian
>>>
>>> Changes since v7:
>>> - Port to new memory API
>>> - Port to new PCI infrastructure
>>> - Use fixed buffers for sense processing
>>> - Update to updated SCSI infrastructure
>>>
>>> Changes since v6:
>>> - Preliminary patches pushed to Kevins block tree
>>> - Implement 64bit contexts, required for Windows7
>>> - Use iovecs for DCMD processing
>>> - Add MSI-X support
>>>     Latest Linux driver now happily uses MSI-X.
>>> - Static iovec allocation
>>>     We have a fixed upper number of iovecs, so we can
>>>     save us the allocation. Suggested by Alex Graf.
>>> - Update MFI header
>>>     Latest Linux driver has some more definitions,
>>>     add them
>>> - Fixup AEN handling
>>> - Update tracing details
>>> - Remove sdev pointer from megasas_cmd_t
>>>
>>> Changes since v5:
>>> - megasas: Use tracing infrastructure instead of DPRINTF
>>> - megasas: Use new PCI infrastructure
>>> - megasas: Check for iovec mapping failure
>>>     cpu_map_physical_memory() might fail, so we need to check for
>>>     it when mapping iovecs.
>>> - megasas: Trace scsi buffer overflow
>>>     The transfer length as specified in the SCSI command might
>>>     disagree with the length of the iovec. We should be tracing
>>>     these issues.
>>> - megasas: Reset frames after init firmware
>>>     When receiving an INIT FIRMWARE command we need reset all
>>>     frames, otherwise some frames might point to invalid memory.
>>>
>>> Chances since v4:
>>> - megasas: checkpatch.pl fixes and update to work with the
>>>     changed interface in scsi_req_new(). Also included the
>>>     suggested fixes from Alex.
>>>
>>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>>> ---
>>>    Makefile.objs           |    1 +
>>>    default-configs/pci.mak |    1 +
>>>    hw/megasas.c            | 2119
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>    hw/mfi.h                | 1281 ++++++++++++++++++++++++++++
>>>    hw/pci_ids.h            |    3 +-
>>>    trace-events            |   73 ++
>>>    6 files changed, 3477 insertions(+), 1 deletions(-)
>>>    create mode 100644 hw/megasas.c
>>>    create mode 100644 hw/mfi.h
>>>
>>> diff --git a/Makefile.objs b/Makefile.objs
>>> index 4f6d26c..3bb2e57 100644
>>> --- a/Makefile.objs
>>> +++ b/Makefile.objs
>>> @@ -275,6 +275,7 @@ hw-obj-$(CONFIG_AHCI) += ide/ich.o
>>>
>>>    # SCSI layer
>>>    hw-obj-$(CONFIG_LSI_SCSI_PCI) += lsi53c895a.o
>>> +hw-obj-$(CONFIG_MEGASAS_SCSI_PCI) += megasas.o
>>>    hw-obj-$(CONFIG_ESP) += esp.o
>>>
>>>    hw-obj-y += dma-helpers.o sysbus.o isa-bus.o
>>> diff --git a/default-configs/pci.mak b/default-configs/pci.mak
>>> index 22bd350..fabb56c 100644
>>> --- a/default-configs/pci.mak
>>> +++ b/default-configs/pci.mak
>>> @@ -9,6 +9,7 @@ CONFIG_EEPRO100_PCI=y
>>>    CONFIG_PCNET_PCI=y
>>>    CONFIG_PCNET_COMMON=y
>>>    CONFIG_LSI_SCSI_PCI=y
>>> +CONFIG_MEGASAS_SCSI_PCI=y
>>>    CONFIG_RTL8139_PCI=y
>>>    CONFIG_E1000_PCI=y
>>>    CONFIG_IDE_CORE=y
>>> diff --git a/hw/megasas.c b/hw/megasas.c
>>> new file mode 100644
>>> index 0000000..c49edd0
>>> --- /dev/null
>>> +++ b/hw/megasas.c
>>> @@ -0,0 +1,2119 @@
>>> +/*
>>> + * QEMU MegaRAID SAS 8708EM2 Host Bus Adapter emulation
>>> + *
>>> + * Copyright (c) 2009-2011 Hannes Reinecke, SUSE Labs
>>> + *
>>> + * This code is licensed under the LGPL.
>>> + */
>>> +
>>> +#include "hw.h"
>>> +#include "pci.h"
>>> +#include "dma.h"
>>> +#include "msix.h"
>>> +#include "iov.h"
>>> +#include "scsi.h"
>>> +#include "scsi-defs.h"
>>> +#include "block_int.h"
>>> +#include "trace.h"
>>> +
>>> +#include "mfi.h"
>>> +
>>> +/* Static definitions */
>>> +#define MEGASAS_VERSION "1.50"
>>> +#define MEGASAS_MAX_FRAMES 2048         /* Firmware limit at
>>> 65535 */
>>> +#define MEGASAS_DEFAULT_FRAMES 1000     /* Windows requires this */
>>> +#define MEGASAS_MAX_SGE 128             /* Firmware limit */
>>> +#define MEGASAS_DEFAULT_SGE 80
>>> +#define MEGASAS_MAX_SECTORS 0xFFFF      /* No real limit */
>>> +#define MEGASAS_MAX_ARRAYS 128
>>> +
>>> +#define MEGASAS_FLAG_USE_JBOD      0x00000001
>>> +#define MEGASAS_FLAG_USE_MSIX      0x00000002
>>> +#define MEGASAS_FLAG_USE_QUEUE64   0x00000004
>>> +
>>> +const char *megasas_raid_modes[] = {
>>> +    "raid", "jbod"
>>> +};
>>> +
>>> +const char *mfi_frame_desc[] = {
>>> +    "MFI init", "LD Read", "LD Write", "LD SCSI", "PD SCSI",
>>> +    "MFI Doorbell", "MFI Abort", "MFI SMP", "MFI Stop"};
>>> +
>>> +struct megasas_cmd_t {
>>> +    uint32_t index;
>>> +    uint16_t flags;
>>> +    uint16_t count;
>>> +    uint64_t context;
>>> +
>>> +    target_phys_addr_t pa;
>>> +    target_phys_addr_t pa_size;
>>> +    union mfi_frame *frame;
>>> +    SCSIRequest *req;
>>> +    struct iovec iov[MEGASAS_MAX_SGE];
>>> +    int iov_cnt;
>>> +    size_t iov_size;
>>> +    size_t iov_offset;
>>> +    struct megasas_state_t *state;
>>> +};
>>
>> This needs a check-patch.pl cleanup pass.
>>
> Hmm. checkpatch just complained here:

Sorry, I put too much faith in checkpatch sometimes.

The structure naming doesn't follow CODING_STYLE along with a few other things. 
  Please read CODING_STYLE and adjust the code accordingly.

Regards,

Anthony Liguori

>
> perl ./scripts/checkpatch.pl
> 0001-megasas-LSI-Megaraid-SAS-HBA-emulation.patch
> ERROR: space required after that ',' (ctx:VxV)
> #1356: FILE: hw/megasas.c:1259:
> +            dummy[0x00], dummy[0x01],dummy[0x02], dummy[0x03],
>                                       ^
>
> total: 1 errors, 0 warnings, 3510 lines checked
>
> 0001-megasas-LSI-Megaraid-SAS-HBA-emulation.patch has style
> problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> I'll fix this.
>
> Cheers,
>
> Hannes