Am 13.01.2012 12:19, schrieb Hannes Reinecke:
> 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 <h...@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/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.
Please clarify the license versions(s). Or-later kindly requested.
> + if (cmd->iov_size > iov_size) {
> + trace_megasas_iovec_len(cmd->index, "overflow", iov_size,
> + cmd->iov_size);
> + } else if (cmd->iov_size < iov_size) {
> + trace_megasas_iovec_len(cmd->iov_size, "underflow", iov_size,
> + cmd->iov_size);
> + }
For anything except stderr these are going to be problematic. For
simpletrace "overflow" and "underflow" will just show up as a pointer
value and cannot be looked up post-mortem.
> +static void megasas_unmap_sgl(struct megasas_cmd_t *cmd)
> +{
> + int i, is_write = megasas_frame_is_write(cmd);
> +
> + for (i = 0; i < cmd->iov_cnt; i++) {
> + cpu_physical_memory_unmap(cmd->iov[i].iov_base, cmd->iov[i].iov_len,
> + is_write, cmd->iov[i].iov_len);
Not sure, but cpu_physical_memory_* sounds old-fashioned. Might need an
update to MemoryRegion?
> + if (cmd->iov_size < sizeof(struct mfi_defaults)) {
> + trace_megasas_dcmd_invalid_xfer_len(cmd->index, "MFC Get defaults",
> + cmd->iov_size);
String in tracepoint.
> + trace_megasas_dcmd_invalid_xfer_len(cmd->index, "Get BIOS info",
> + cmd->iov_size);
dito
> + trace_megasas_dcmd_enter(cmd->index, "Get Event Info");
dito
> + trace_megasas_dcmd_invalid_xfer_len(cmd->index, "Get Event Wait",
> + cmd->iov_size);
Dito, rest as exercise for the reader.
> +static const struct dcmd_cmd_tbl_t {
> + int opcode;
> + int (*func)(MPTState *s, struct megasas_cmd_t *cmd);
> +} dcmd_cmd_tbl[] = {
> + {MFI_DCMD_CTRL_MFI_HOST_MEM_ALLOC, megasas_dcmd_dummy},
Spaces after { and before } missing.
> + if (!sdev || (megasas_is_jbod(s) && is_logical)) {
> + trace_megasas_scsi_target_not_present(
> + mfi_frame_desc[cmd->frame->header.frame_cmd],
> + is_logical ? "logical" : "physical",
> + cmd->frame->header.target_id, cmd->frame->header.lun_id);
Two-state string could be 0/1-encoded here.
> + if (len > cmd->iov_size) {
> + trace_megasas_scsi_overflow(cmd->index, "scsi read",
> + len, cmd->iov_size);
> + }
> + if (len < cmd->iov_size) {
> + trace_megasas_scsi_underflow(cmd->index, "scsi read",
> + len, cmd->iov_size);
> + cmd->iov_size = len;
> + }
> + trace_megasas_scsi_start(cmd->index, "read", len);
> + scsi_req_continue(cmd->req);
> + } else if (len < 0) {
> + if (-len > cmd->iov_size) {
> + trace_megasas_scsi_overflow(cmd->index, "scsi write",
> + -len, cmd->iov_size);
> + }
> + if (-len < cmd->iov_size) {
> + trace_megasas_scsi_underflow(cmd->index, "scsi write",
> + -len, cmd->iov_size);
> + cmd->iov_size = -len;
> + }
> + trace_megasas_scsi_start(cmd->index, "write", -len);
> + scsi_req_continue(cmd->req);
> + } else {
Here you have tracepoints for overflow and underflow and pass "scsi
read" vs. "scsi write" as arguments; above (iovec) you were passing
"overflow" and "underflow" as arguments. Unify the scheme?
You already introduce quite a few tracepoints (a shining example, one
might say) so maybe just do ..._scsi_read_overflow() and
_scsi_read_underflow()?
On the other hand, when someone is actually looking for a particular
nesting of events, DTrace for example allows to set some per-CPU flag so
not passing the specific argument might work as well.
> + if (desc) {
> + trace_megasas_readl_reg("mmio", desc, retval);
> + } else {
> + trace_megasas_readl("mmio", addr, retval);
> + }
Drop _reg version?
Didn't review the SCSI bits. Patch is pretty large, too. ;)
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
