Re: [Qemu-block] [PATCH 01/16] ide: add limit to .prepare_buf()

[Stefan Hajnoczi]

On Mon, Jun 22, 2015 at 08:21:00PM -0400, John Snow wrote:
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 95d228f..f873ab1 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -49,7 +49,7 @@ static int handle_cmd(AHCIState *s,int port,int slot);
>  static void ahci_reset_port(AHCIState *s, int port);
>  static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis);
>  static void ahci_init_d2h(AHCIDevice *ad);
> -static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write);
> +static int ahci_dma_prepare_buf(IDEDMA *dma, int64_t limit, int is_write);

Why int64_t?

Types involved here are uint64_t, dma_addr_t, size_t, and int.  Out of
these, uint64_t seems like a good candidate but I'm not sure why it
needs to be signed.

> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
> index 4afd0cf..a295baa 100644
> --- a/hw/ide/pci.c
> +++ b/hw/ide/pci.c
> @@ -55,8 +55,11 @@ static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
>  /**
>   * Return the number of bytes successfully prepared.
>   * -1 on error.
> + * BUG?: Does not currently heed the 'limit' parameter because
> + *       it is not clear what the correct behavior here is,
> + *       see tests/ide-test.c

QEMU implements both short and long PRDT cases for IDE in ide_dma_cb()
and the tests check them.  I saw nothing indicating that existing
behavior might not correspond to real hardware behavior.  Why do you say
the correct behavior is unclear?

pgpMpK_jpBDFy.pgp
Description: PGP signature