On Fri, Nov 26, 2010 at 7:17 PM, Alexander Graf <ag...@suse.de> wrote: > +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) { > + *ptr = NULL;
We need to unmap the subset of the requested range. > +static void ncq_cb(void *opaque, int ret) > +{ > + NCQTransferState *ncq_tfs = (NCQTransferState *)opaque; > + IDEState *ide_state; > + > + if (ret < 0) { > + /* error */ Error handling missing. > +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; > + > + pr = &s->dev[port].port_regs; > + cmd = (AHCICmdHdr *)&s->dev[port].lst[slot * 32]; This isn't much easier to read but gets rid of the sizeof(AHCICmdHdr) scaling: cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot]; > +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_RAID); > + pci_set_word(d->card.config + PCI_COMMAND, PCI_COMMAND_IO | > + PCI_COMMAND_MEMORY | > + PCI_COMMAND_MASTER); We are enabling IO and memory space decode, as well as bus master here. According to the AHCI spec 1.3, PCI_COMMAND_MEMORY and PCI_COMMAND_MASTER should be 0 on reset ("2.1.2 Offset 04h: CMD - Command"). I thought the guest's firmware or operating system would set these bits when initializing PCI devices? > +static int pci_ahci_uninit(PCIDevice *dev) > +{ > + struct AHCIPCIState *d; > + d = DO_UPCAST(struct AHCIPCIState, card, dev); > + > + if (msi_enabled(&d->card)) { > + msi_uninit(dev); > + } It looks like d and &d->card are not needed, we already have dev. Stefan