Il 13/09/2014 06:34, John Snow ha scritto:
> AHCI devices support a feature where individual entries in the
> scatter-gather list may have interrupt request bits set, in order
> to receive notification partway through a command that a portion
> of a transfer has completed. AHCI specs refer to this as the
> DPS bit or Descriptor Processed Status. It is named the
> "Interrupt on Completion" bit inside the PRDT.
>
> This is not currently feasible in QEMU without reworking the inner
> DMA loop extensively, but we can at least record if we saw such
> a bit in advance and sent the interrupt at the end of the transfer,
> in case a guest is expecting to see the PRD/DPS interrupt bit set.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> hw/ide/ahci.c | 11 +++++++++++
> hw/ide/ahci.h | 5 ++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index d3ece78..8e6a352 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -501,6 +501,7 @@ static void ahci_reset_port(AHCIState *s, int port)
> pr->scr_err = 0;
> pr->scr_act = 0;
> d->busy_slot = -1;
> + d->dp_intr_req = false;
> d->init_d2h_sent = false;
>
> ide_state = &s->dev[port].port.ifs[0];
> @@ -775,11 +776,15 @@ static int ahci_populate_sglist(AHCIDevice *ad,
> QEMUSGList *sglist, int offset)
> ad->hba->as);
> qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr + off_pos),
> prdt_tbl_entry_size(&tbl[off_idx]) - off_pos);
> + ad->dp_intr_req = le32_to_cpu(tbl[off_idx].flags_size) &
> AHCI_PRDT_INTR;
Why is this also not an "OR"? It feels safer that way.
> for (i = off_idx + 1; i < sglist_alloc_hint; i++) {
> /* flags_size is zero-based */
> qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr),
> prdt_tbl_entry_size(&tbl[i]));
> + if (le32_to_cpu(tbl[i].flags_size) & AHCI_PRDT_INTR) {
> + ad->dp_intr_req = 1;
> + }
> }
> }
>
> @@ -1151,6 +1156,11 @@ static int ahci_commit_buf(IDEDMA *dma, int xfer_ok)
>
> qemu_sglist_destroy(&s->sg);
>
> + if (ad->dp_intr_req) {
> + ahci_trigger_irq(ad->hba, ad, PORT_IRQ_SG_DONE);
> + ad->dp_intr_req = 0;
> + }
Is it also needed in the error case? Especially the short-PRDT case
that you are adding in the next patch.
> return s->sg.size != 0;
> }
>
> @@ -1307,6 +1317,7 @@ static const VMStateDescription vmstate_ahci_device = {
> VMSTATE_UINT32(port_regs.cmd_issue, AHCIDevice),
> VMSTATE_BOOL(done_atapi_packet, AHCIDevice),
> VMSTATE_INT32(busy_slot, AHCIDevice),
> + VMSTATE_BOOL(dp_intr_req, AHCIDevice),
> VMSTATE_BOOL(init_d2h_sent, AHCIDevice),
> VMSTATE_END_OF_LIST()
> },
> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> index 1543df7..31f32d3 100644
> --- a/hw/ide/ahci.h
> +++ b/hw/ide/ahci.h
> @@ -180,7 +180,9 @@
>
> #define AHCI_COMMAND_TABLE_ACMD 0x40
>
> -#define AHCI_PRDT_SIZE_MASK 0x3fffff
> +#define AHCI_PRDT_SIZE_MASK 0x003fffff
> +#define AHCI_PRDT_RESERVED 0x7fc00000
> +#define AHCI_PRDT_INTR 0x80000000
>
> #define IDE_FEATURE_DMA 1
>
> @@ -265,6 +267,7 @@ struct AHCIDevice {
> bool done_atapi_packet;
> int32_t busy_slot;
> bool init_d2h_sent;
> + bool dp_intr_req;
> AHCICmdHdr *cur_cmd;
> NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
> };
>
