On Wed, Dec 8, 2010 at 12:13 PM, Alexander Graf <ag...@suse.de> wrote: > +struct AHCIDevice { > + IDEBus port; > + int port_no; > + uint32_t port_state; > + uint32_t finished; > + AHCIPortRegs port_regs; > + struct AHCIState *hba; > + uint8_t *lst; > + uint8_t *res_fis; > + uint8_t *cmd_fis;
Are these unmapped on reset? > + int cmd_fis_len; > + int dma_status; > + BlockDriverCompletionFunc *dma_cb; > + AHCICmdHdr *cur_cmd; > + NCQTransferState ncq_tfs[AHCI_MAX_CMDS]; Are the ncq_tfs[] elements cleaned up on reset (i.e. cancellation and free sglist)? > +static void map_page(uint8_t **ptr, uint64_t addr, uint32_t wanted) > +{ > + target_phys_addr_t len = wanted; > + > + if (*ptr) { > + cpu_physical_memory_unmap(*ptr, 1, len, len); > + } > + > + *ptr = cpu_physical_memory_map(addr, &len, 1); > + if (len < wanted) { > + cpu_physical_memory_unmap(*ptr, 1, len, len); *ptr = NULL; > +static void ncq_cb(void *opaque, int ret) > +{ > + NCQTransferState *ncq_tfs = (NCQTransferState *)opaque; > + IDEState *ide_state; > + > + if (ret < 0) { > + /* XXX error */ > + } Missing error handling. > +static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, > + int slot, QEMUSGList *sg) > +{ > + NCQFrame *ncq_fis = (NCQFrame*)cmd_fis; > + uint8_t tag = ncq_fis->tag >> 3; > + NCQTransferState *ncq_tfs = &s->dev[port].ncq_tfs[tag]; > + > + if (ncq_tfs->used) { > + /* error - already in use */ > + fprintf(stderr, "%s: tag %d already used\n", __FUNCTION__, tag); > + return; > + } > + > + ncq_tfs->used = 1; > + ncq_tfs->drive = &s->dev[port]; > + ncq_tfs->drive->cmd_fis = cmd_fis; > + ncq_tfs->drive->cmd_fis_len = 0x20; > + ncq_tfs->slot = slot; > + ncq_tfs->lba = ((uint64_t)ncq_fis->lba5 << 40) | > + ((uint64_t)ncq_fis->lba4 << 32) | > + ((uint64_t)ncq_fis->lba3 << 24) | > + ((uint64_t)ncq_fis->lba2 << 16) | > + ((uint64_t)ncq_fis->lba1 << 8) | > + (uint64_t)ncq_fis->lba0; > + > + /* Note: We calculate the sector count, but don't currently rely on it. > + * The total size of the DMA buffer tells us the transfer size instead. > */ > + ncq_tfs->sector_count = ((uint16_t)ncq_fis->sector_count_high << 8) | > + ncq_fis->sector_count_low; > + > + DPRINTF(port, "NCQ transfer LBA from %ld to %ld, drive max %ld\n", > + ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 2, > + s->dev[port].port.ifs[0].nb_sectors - 1); > + > + ncq_tfs->sglist = *sg; > + ncq_tfs->tag = tag; > + > + switch(ncq_fis->command) { > + case READ_FPDMA_QUEUED: > + DPRINTF(port, "NCQ reading %d sectors from LBA %ld, tag %d\n", > + ncq_tfs->sector_count-1, ncq_tfs->lba, ncq_tfs->tag); > + ncq_tfs->is_read = 1; > + > + /* XXX: The specification is unclear about whether the DMA Setup > + * FIS here should have the I bit set, but it suggest that it > should > + * not. Linux works without this interrupt, so I disabled it. > + * If someone knows if it is needed, please tell me, or fix > this. */ > + > + /* ahci_trigger_irq(s,s->dev[port],PORT_IRQ_STAT_DSS); */ > + DPRINTF(port, "tag %d aio read %ld\n", ncq_tfs->tag, > ncq_tfs->lba); > + dma_bdrv_read(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->sglist, > + ncq_tfs->lba, ncq_cb, ncq_tfs); > + break; > + case WRITE_FPDMA_QUEUED: > + DPRINTF(port, "NCQ writing %d sectors to LBA %ld, tag %d\n", > + ncq_tfs->sector_count-1, ncq_tfs->lba, ncq_tfs->tag); > + ncq_tfs->is_read = 0; > + /* ahci_trigger_irq(s,s->dev[port],PORT_IRQ_STAT_DSS); */ > + DPRINTF(port, "tag %d aio write %ld\n", ncq_tfs->tag, > ncq_tfs->lba); > + dma_bdrv_write(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->sglist, > + ncq_tfs->lba, ncq_cb, ncq_tfs); > + break; > + default: > + hw_error("ahci: tried to process non-NCQ command as NCQ\n"); Guest triggerable abort. > + break; > + } > +} > + > +static int handle_cmd(AHCIState *s, int port, int slot) > +{ > + IDEState *ide_state; > + > + int sglist_alloc_hint; > + QEMUSGList sglist; > + int atapi_packet_len = 0; > + AHCIPortRegs *pr; > + uint32_t opts; > + uint64_t tbl_addr; > + AHCICmdHdr *cmd; > + uint8_t *cmd_fis; > + > + target_phys_addr_t cmd_len; > + int i; > + > + if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) { > + /* Engine currently busy, try again later */ > + DPRINTF(port, "engine busy\n"); > + return -1; > + } > + > + pr = &s->dev[port].port_regs; > + cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot]; > + > + if (!s->dev[port].lst) { > + hw_error("%s: lst not given but cmd handled", __FUNCTION__); Guest triggerable abort. > + } > + > + opts = le32_to_cpu(cmd->opts); > + tbl_addr = le64_to_cpu(cmd->tbl_addr); > + > + sglist_alloc_hint = opts >> AHCI_CMD_HDR_PRDT_LEN; > + cmd_len = 0x80 + (sglist_alloc_hint * sizeof(AHCI_SG)); > + cmd_fis = cpu_physical_memory_map(tbl_addr, &cmd_len, 1); NULL dereference later if cpu_physical_memory_map() fails due to invalid address (tbl_addr). > + > + /* The device we are working for */ > + ide_state = &s->dev[port].port.ifs[0]; > + > + if (!ide_state->bs) { > + hw_error("%s: guest accessed unused port", __FUNCTION__); Guest triggerable abort. > + } > + > + /* Get number of entries in the PRDT, init a qemu sglist accordingly */ > + memset(&sglist, 0, sizeof(sglist)); > + > + if (sglist_alloc_hint > 0) { > + AHCI_SG *tbl = (AHCI_SG *)(&cmd_fis[0x80]); > + > + qemu_sglist_init(&sglist, sglist_alloc_hint); > + /* Parse the PRDs and create qemu sglist entries accordingly */ > + for (i = 0; i < sglist_alloc_hint; i++) { > + /* flags_size is zero-based */ > + qemu_sglist_add(&sglist, le64_to_cpu(tbl[i].addr), > + le32_to_cpu(tbl[i].flags_size) + 1); > + } > + } Only the SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER codepath seems to clean up sglist. The guest can leak host memory by setting sglist_alloc_hint > 0 and not using SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER. > + > + debug_print_fis(cmd_fis, (opts & AHCI_CMD_HDR_CMD_FIS_LEN) * 4); > + > + switch (cmd_fis[0]) { > + case SATA_FIS_TYPE_REGISTER_H2D: > + break; > + default: > + hw_error("unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x > cmd_fis[2]=%02x\n", > + cmd_fis[0], cmd_fis[1], cmd_fis[2]); Guest triggerable abort. > + break; > + } > + > + switch (cmd_fis[1]) { > + case SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER: > + break; > + case 0: > + break; > + default: > + hw_error("unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x > cmd_fis[2]=%02x\n", > + cmd_fis[0], cmd_fis[1], cmd_fis[2]); Guest triggerable abort. > + break; > + } > + > + switch (s->dev[port].port_state) { > + case STATE_RUN: > + if (cmd_fis[15] & ATA_SRST) { > + s->dev[port].port_state = STATE_RESET; > + } > + break; > + case STATE_RESET: > + if (!(cmd_fis[15] & ATA_SRST)) { > + ahci_reset_port(s, port); > + } > + break; > + } > + > + if (cmd_fis[1] == SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER) { > + > + /* Check for NCQ command */ > + if ((cmd_fis[2] == READ_FPDMA_QUEUED) || > + (cmd_fis[2] == WRITE_FPDMA_QUEUED)) { > + process_ncq_command(s, port, cmd_fis, slot, &sglist); > + goto out; > + } > + > + /* If the command is not NCQ, the sglist is needed in the core */ > + ide_state->sg = sglist; > + > + /* Decompose the FIS */ > + ide_state->nsector = (int64_t)((cmd_fis[13] << 8) | cmd_fis[12]); > + ide_state->feature = cmd_fis[3]; > + if (!ide_state->nsector) { > + ide_state->nsector = 256; > + } > + > + if (ide_state->drive_kind != IDE_CD) { > + ide_set_sector(ide_state, (cmd_fis[6] << 16) | (cmd_fis[5] << 8) > | > + cmd_fis[4]); > + } > + > + /* Copy the ACMD field (ATAPI packet, if any) from the AHCI command > + * table to ide_state->io_buffer > + */ > + if (opts & AHCI_CMD_ATAPI) { > + atapi_packet_len = ((ide_state->hcyl) << 8) + ide_state->lcyl; Unused variable. > +static int pci_ahci_init(PCIDevice *dev) > +{ > + struct AHCIPCIState *d; > + d = DO_UPCAST(struct AHCIPCIState, card, dev); > + > + pci_config_set_vendor_id(d->card.config, PCI_VENDOR_ID_INTEL); > + pci_config_set_device_id(d->card.config, > + PCI_DEVICE_ID_INTEL_ICH7_AHCI); > + > + pci_config_set_class(d->card.config, PCI_CLASS_STORAGE_SATA); > + pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1); > + > + d->card.config[PCI_CACHE_LINE_SIZE] = 0x08; /* Cache line size */ > + d->card.config[PCI_LATENCY_TIMER] = 0x00; /* Latency timer */ > + pci_config_set_interrupt_pin(d->card.config, 1); > + > + qemu_register_reset(ahci_reset, d); Missing qemu_unregister_reset() in pci_ahci_uninit(). Stefan