[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] megasas: LSI Megaraid SAS emulation
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] megasas: LSI Megaraid SAS emulation |
Date: |
Sun, 3 Jul 2011 11:09:38 +0300 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Jul 01, 2011 at 11:16:03AM +0200, Alexander Graf wrote:
>
> On 01.07.2011, at 09:42, Hannes Reinecke wrote:
>
> > This patch adds an emulation for the LSI Megaraid SAS 8708EM2 HBA.
> >
> > Signed-off-by: Hannes Reinecke <address@hidden>
> > ---
> > Makefile.objs | 1 +
> > default-configs/pci.mak | 1 +
> > hw/megasas.c | 1923
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > hw/mfi.h | 1197 +++++++++++++++++++++++++++++
> > hw/pci_ids.h | 3 +-
> > 5 files changed, 3124 insertions(+), 1 deletions(-)
> > create mode 100644 hw/megasas.c
> > create mode 100644 hw/mfi.h
> >
> > diff --git a/Makefile.objs b/Makefile.objs
> > index cea15e4..6f5d113 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -258,6 +258,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..75f9be3
> > --- /dev/null
> > +++ b/hw/megasas.c
> > @@ -0,0 +1,1923 @@
> > +/*
> > + * QEMU MegaRAID SAS 8708EM2 Host Bus Adapter emulation
> > + *
> > + * Copyright (c) 2009-2011 Hannes Reinecke, SUSE Labs
> > + *
> > + * This code is licenced under the LGPL.
>
> Please take a look at the license header of other LGPL code and just copy it
> :).
>
> > + */
> > +
> > +#include <time.h>
> > +#include <assert.h>
>
> Are you sure you need to manually include those?
>
> > +
> > +#include "hw.h"
> > +#include "pci.h"
> > +#include "dma.h"
> > +#include "iov.h"
> > +#include "scsi.h"
> > +#include "scsi-defs.h"
> > +#include "block_int.h"
> > +#ifdef __linux__
> > +# include <scsi/sg.h>
>
> Is this really necessary? Device code shouldn't be host dependent IMHO. I
> also haven't found any user of this in the actual code, so it might be as
> easy as merely removing the include :).
>
> > +#endif
> > +
> > +#include "mfi.h"
> > +
> > +#define DEBUG_MEGASAS
> > +#undef DEBUG_MEGASAS_REG
> > +#undef DEBUG_MEGASAS_QUEUE
> > +#undef DEBUG_MEGASAS_MFI
> > +#undef DEBUG_MEGASAS_IO
> > +#undef DEBUG_MEGASAS_DCMD
> > +
> > +#ifdef DEBUG_MEGASAS
> > +#define DPRINTF(fmt, ...) \
> > +do { printf("megasas: " fmt , ## __VA_ARGS__); } while (0)
> > +#define BADF(fmt, ...) \
> > +do { fprintf(stderr, "megasas: error: " fmt , ## __VA_ARGS__); exit(1);}
> > while (0)
> > +#ifdef DEBUG_MEGASAS_REG
> > +#define DPRINTF_REG DPRINTF
> > +#else
> > +#define DPRINTF_REG(fmt, ...) do {} while(0)
> > +#endif
> > +#ifdef DEBUG_MEGASAS_QUEUE
> > +#define DPRINTF_QUEUE DPRINTF
> > +#else
> > +#define DPRINTF_QUEUE(fmt, ...) do {} while(0)
> > +#endif
> > +#ifdef DEBUG_MEGASAS_MFI
> > +#define DPRINTF_MFI DPRINTF
> > +#else
> > +#define DPRINTF_MFI(fmt, ...) do {} while(0)
> > +#endif
> > +#ifdef DEBUG_MEGASAS_IO
> > +#define DPRINTF_IO DPRINTF
> > +#else
> > +#define DPRINTF_IO(fmt, ...) do {} while(0)
> > +#endif
> > +#ifdef DEBUG_MEGASAS_DCMD
> > +#define DPRINTF_DCMD DPRINTF
> > +#else
> > +#define DPRINTF_DCMD(fmt, ...) do {} while(0)
> > +#endif
> > +#else
> > +#define DPRINTF(fmt, ...) do {} while(0)
> > +#define DPRINTF_REG DPRINTF
> > +#define DPRINTF_QUEUE DPRINTF
> > +#define DPRINTF_MFI DPRINTF
> > +#define DPRINTF_IO DPRINTF
> > +#define DPRINTF_DCMD DPRINTF
> > +#define BADF(fmt, ...) \
> > +do { fprintf(stderr, "megasas: error: " fmt , ## __VA_ARGS__);} while (0)
> > +#endif
> > +
> > +/* Static definitions */
> > +#define MEGASAS_VERSION "1.20"
> > +#define MEGASAS_MAX_FRAMES 2048 /* Firmware limit at 65535 */
> > +#define MEGASAS_DEFAULT_FRAMES 1000 /* Windows requires this */
> > +#define MEGASAS_MAX_SGE 256 /* Firmware limit */
> > +#define MEGASAS_DEFAULT_SGE 80
> > +#define MEGASAS_MAX_SECTORS 0xFFFF /* No real limit */
> > +#define MEGASAS_MAX_ARRAYS 128
> > +
> > +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 {
> > + int index;
> > + int context;
> > + int count;
> > +
> > + target_phys_addr_t pa;
> > + target_phys_addr_t pa_size;
> > + union mfi_frame *frame;
> > + SCSIRequest *req;
> > + struct iovec *iov;
> > + void *iov_buf;
> > + long iov_cnt;
> > + long iov_size;
> > + long iov_offset;
>
> Why would anything be a long? It's either target_ulong or uintXX_t for device
> code usually :).
>
> > + SCSIDevice *sdev;
> > + struct megasas_state_t *state;
> > +};
> > +
> > +typedef struct megasas_state_t {
> > + PCIDevice dev;
> > + int mmio_io_addr;
> > + int io_addr;
> > + int queue_addr;
> > + uint32_t frame_hi;
> > +
> > + int fw_state;
> > + uint32_t fw_sge;
> > + uint32_t fw_cmds;
> > + int fw_luns;
> > + int intr_mask;
> > + int doorbell;
> > + int busy;
> > + char *raid_mode_str;
> > + int is_jbod;
> > +
> > + int event_count;
> > + int shutdown_event;
> > + int boot_event;
> > +
> > + uint64_t reply_queue_pa;
> > + void *reply_queue;
> > + int reply_queue_len;
> > + int reply_queue_index;
> > + uint64_t consumer_pa;
> > + uint64_t producer_pa;
> > +
> > + struct megasas_cmd_t frames[MEGASAS_MAX_FRAMES];
> > +
> > + SCSIBus bus;
> > +} MPTState;
> > +
> > +#define MEGASAS_INTR_DISABLED_MASK 0xFFFFFFFF
> > +
> > +#define MEGASAS_INTR_ENABLED(s) (((s)->intr_mask &
> > MEGASAS_INTR_DISABLED_MASK ) != MEGASAS_INTR_DISABLED_MASK)
> > +
> > +#define megasas_frame_set_cmd_status(f,v) \
> > + stb_phys((f) + offsetof(struct mfi_frame_header, cmd_status), v);
> > +
> > +#define megasas_frame_set_scsi_status(f,v) \
> > + stb_phys((f) + offsetof(struct mfi_frame_header, scsi_status), v);
> > +
> > +#define megasas_frame_get_cmd(f) \
> > + ldub_phys((f) + offsetof(struct mfi_frame_header, frame_cmd))
> > +
> > +#define megasas_frame_get_context(f) \
> > + ldl_phys(frame_addr + offsetof(struct mfi_frame_header, context));
> > +
> > +static void megasas_soft_reset(MPTState *s);
> > +
> > +static int megasas_map_sgl(struct megasas_cmd_t *cmd, int pa_offset)
> > +{
> > + int i;
> > + uint16_t flags = le16_to_cpu(cmd->frame->header.flags);
> > + int is_sgl64 = (flags & MFI_FRAME_SGL64) ? 1 : 0;
> > + int is_write = (flags & MFI_FRAME_DIR_WRITE) ? 1 : 0;
> > + int sgl_addr_size = is_sgl64 ? sizeof(uint64_t) : sizeof(uint32_t);
> > + size_t iov_count = 0;
> > +
> > + cmd->iov = qemu_malloc(sizeof(struct iovec) *
> > (cmd->frame->header.sge_count + 1));
> > + for (i = 0; i < cmd->frame->header.sge_count; i++) {
> > + target_phys_addr_t pa, iov_pa, iov_size;
> > +
> > + pa = cmd->pa + pa_offset;
> > + if (is_sgl64)
> > + iov_pa = ldq_phys(pa);
>
> Could you please send your patch through scripts/checkpatch.pl, so that
> coding style issues don't hold up the commit process?
>
> [...]
>
> > +
> > +static void megasas_xfer_complete(SCSIRequest *req, uint32_t len)
> > +{
> > + struct megasas_cmd_t *cmd;
> > + uint8_t *buf;
> > +
> > + cmd = req->hba_private;
> > + if (!cmd) {
>
>
> Is this still necessary with the new pointer infrastructure?
>
> > + /*
> > + * Bad. A command has been completed but we couldn't find it.
> > + * Only safe way out of here is to terminate everything and
> > + * hope the HBA recovers.
> > + */
> > + DPRINTF("SCSI request %p not found", req);
> > + return;
> > + }
> > +
> > + DPRINTF_IO("%s req %p cmd %p lun %p xfer completed, len %u\n",
> > + mfi_frame_desc[cmd->frame->header.frame_cmd], req, cmd,
> > + cmd->sdev, len);
> > +
> > + if (len) {
> > + uint16_t flags = le16_to_cpu(cmd->frame->header.flags);
> > + int is_write = (flags & MFI_FRAME_DIR_WRITE) ? 1 : 0;
> > + size_t bytes;
> > +
> > + buf = scsi_req_get_buf(req);
> > + if (is_write) {
> > + DPRINTF_IO("%s req %p cmd %p lun %p write finished, left %u\n",
> > + mfi_frame_desc[cmd->frame->header.frame_cmd], req,
> > + cmd, cmd->sdev, len);
> > + bytes = iov_to_buf(cmd->iov, cmd->iov_cnt, buf,
> > + cmd->iov_offset, len);
> > + if (bytes != len) {
> > + len = bytes;
> > + }
> > + cmd->iov_offset += bytes;
> > + } else {
> > + DPRINTF_IO("%s req %p cmd %p lun %p read finished, len %u\n",
> > + mfi_frame_desc[cmd->frame->header.frame_cmd], req,
> > + cmd, cmd->sdev, len);
> > + bytes = iov_from_buf(cmd->iov, cmd->iov_cnt, buf,
> > + cmd->iov_offset, len);
> > + if (bytes != len) {
> > + len = bytes;
> > + }
> > + cmd->iov_offset += bytes;
> > + }
> > + }
> > + cmd->iov_size -= len;
> > + scsi_req_continue(req);
> > +}
> > +
> > +static void megasas_command_complete(SCSIRequest *req, uint32_t status)
> > +{
> > + struct megasas_cmd_t *cmd;
> > + uint8_t cmd_status = MFI_STAT_OK;
> > +
> > + cmd = req->hba_private;
> > + if (!cmd) {
>
> same here
>
> [...]
>
> > diff --git a/hw/pci_ids.h b/hw/pci_ids.h
> > index d94578c..796aaf1 100644
> > --- a/hw/pci_ids.h
> > +++ b/hw/pci_ids.h
> > @@ -12,9 +12,9 @@
> >
> > #define PCI_BASE_CLASS_STORAGE 0x01
> > #define PCI_BASE_CLASS_NETWORK 0x02
> > -
>
Yes this separates base class (1 byte) from class
word codes.
> I don't think this is intentional, no?
>
> > #define PCI_CLASS_STORAGE_SCSI 0x0100
> > #define PCI_CLASS_STORAGE_IDE 0x0101
> > +#define PCI_CLASS_STORAGE_RAID 0x0104
> > #define PCI_CLASS_STORAGE_SATA 0x0106
> > #define PCI_CLASS_STORAGE_OTHER 0x0180
> >
> > @@ -47,6 +47,7 @@
> >
> > #define PCI_VENDOR_ID_LSI_LOGIC 0x1000
> > #define PCI_DEVICE_ID_LSI_53C895A 0x0012
> > +#define PCI_DEVICE_ID_LSI_SAS1078 0x0060
>
> Michael, these go to your table :)
>
>
> Alex
- [Qemu-devel] [PATCH 0/3] [v4] Megasas HBA emulation, Hannes Reinecke, 2011/07/01
- [Qemu-devel] [PATCH 1/3] iov: Add 'offset' parameter to iov_to_buf(), Hannes Reinecke, 2011/07/01
- Re: [Qemu-devel] [PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer, Paolo Bonzini, 2011/07/01
- Re: [Qemu-devel] [PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer, Hannes Reinecke, 2011/07/01
- Re: [Qemu-devel] [PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer, Hannes Reinecke, 2011/07/01
- Re: [Qemu-devel] [PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer, Paolo Bonzini, 2011/07/01
Re: [Qemu-devel] [PATCH 1/3] iov: Add 'offset' parameter to iov_to_buf(), Alexander Graf, 2011/07/01
Re: [Qemu-devel] [PATCH 1/3] iov: Add 'offset' parameter to iov_to_buf(), Paolo Bonzini, 2011/07/01