[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] Replace bdrv_* to bdrv_aio_* functions in DM
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v4] Replace bdrv_* to bdrv_aio_* functions in DMA mode in fdc.c. |
Date: |
Mon, 16 Apr 2012 13:36:10 +0100 |
On Mon, Apr 16, 2012 at 6:29 AM, Li Zhi Hui <address@hidden> wrote:
> Replace bdrv_* to bdrv_aio_* functions in DMA mode in fdc.c.
>
> Signed-off-by: Li Zhi Hui <address@hidden>
> ---
Please include a changelog below the '---' showing what was changed in
each patch revision (v2, v3, v4, etc). This allows reviewers to
quickly check the things they suggested in the previous version.
> hw/fdc.c | 266
> +++++++++++++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 195 insertions(+), 71 deletions(-)
This patch seems to depend on "add function DMA_set_return and delete
bh_schedule in dma.c". I suggest sending them together in a 2 patch
series so they can be applied easily. If you send patches that do not
apply to qemu.git/master (like this one), then it's important to
mention what tree to build against (or just send them in a single
patch series).
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..f7c36f3 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -429,6 +429,8 @@ struct FDCtrl {
> /* Timers state */
> uint8_t timer0;
> uint8_t timer1;
> + QEMUIOVector qiov;
> + struct iovec iov;
> };
>
> typedef struct FDCtrlSysBus {
> @@ -443,6 +445,12 @@ typedef struct FDCtrlISABus {
> int32_t bootindexB;
> } FDCtrlISABus;
>
> +typedef struct FDC_opaque {
> + FDCtrl *fdctrl;
> + int nchan;
> + int dma_len;
> +} FDC_opaque;
> +
> static uint32_t fdctrl_read (void *opaque, uint32_t reg)
> {
> FDCtrl *fdctrl = opaque;
> @@ -1154,7 +1162,6 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int
> direction)
> * recall us...
> */
> DMA_hold_DREQ(fdctrl->dma_chann);
> - DMA_schedule(fdctrl->dma_chann);
> return;
> } else {
> FLOPPY_ERROR("dma_mode=%d direction=%d\n", dma_mode, direction);
> @@ -1181,6 +1188,123 @@ static void fdctrl_start_transfer_del(FDCtrl *fdctrl,
> int direction)
> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
> }
>
> +static void fdctrl_read_DMA_cb(void *opaque, int ret)
> +{
> + FDC_opaque *opaque_cb = opaque;
> + FDCtrl *fdctrl = opaque_cb->fdctrl;
> + int nchan = opaque_cb->nchan;
> + int dma_len = opaque_cb->dma_len;
> + FDrive *cur_drv;
> + int len, start_pos, rel_pos;
> + uint8_t status0 = 0x00, status1 = 0x00, status2 = 0x00;
> +
> + if (ret < 0) {
> + FLOPPY_DPRINTF("Floppy: error getting sector %d\n",
> + fd_sector(cur_drv));
> + /* Sure, image size is too small... */
> + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00,
> 0x00);
> + goto error;
> + }
> +
> + cur_drv = get_cur_drv(fdctrl);
> + rel_pos = fdctrl->data_pos % FD_SECTOR_LEN;
> + for (start_pos = fdctrl->data_pos; fdctrl->data_pos < dma_len;) {
> + len = dma_len - fdctrl->data_pos;
> + if (len + rel_pos > FD_SECTOR_LEN) {
> + len = FD_SECTOR_LEN - rel_pos;
> + }
> +
> + if (fdctrl->data_dir == FD_DIR_READ) {
> + /* READ commands */
> + DMA_write_memory(nchan,
> + (char *)(fdctrl->qiov.iov->iov_base) +
> + fdctrl->data_pos + rel_pos, fdctrl->data_pos, len);
> + } else {
> + /* SCAN commands */
> + uint8_t tmpbuf[FD_SECTOR_LEN];
> + int ret;
> + DMA_read_memory(nchan, tmpbuf, fdctrl->data_pos, len);
> + ret = memcmp(tmpbuf, fdctrl->fifo + rel_pos, len);
> + if (ret == 0) {
> + status2 = FD_SR2_SEH;
> + }
> + if ((ret < 0 && fdctrl->data_dir == FD_DIR_SCANL) ||
> + (ret > 0 && fdctrl->data_dir == FD_DIR_SCANH)) {
> + status2 = 0x00;
> + }
> + goto end_transfer;
> + }
> + fdctrl->data_pos += len;
> + rel_pos = fdctrl->data_pos % FD_SECTOR_LEN;
> + if (rel_pos == 0) {
> + /* Seek to next sector */
> + if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
> + break;
> + }
> + }
> + }
> +end_transfer:
> + len = fdctrl->data_pos - start_pos;
> + FLOPPY_DPRINTF("end transfer %d %d %d\n",
> + fdctrl->data_pos, len, fdctrl->data_len);
> + if ((fdctrl->data_dir == FD_DIR_SCANE) ||
> + (fdctrl->data_dir == FD_DIR_SCANL) ||
> + (fdctrl->data_dir == FD_DIR_SCANH)) {
> + status2 = FD_SR2_SEH;
> + }
> +
> + if (FD_DID_SEEK(fdctrl->data_state)) {
> + status0 |= FD_SR0_SEEK;
> + }
> + fdctrl->data_len -= len;
> + DMA_set_return(len, nchan);
> + fdctrl_stop_transfer(fdctrl, status0, status1, status2);
> +
> +error:
> + qemu_vfree(fdctrl->iov.iov_base);
> + g_free(opaque_cb);
> + DMA_schedule(fdctrl->dma_chann);
> + return;
> +}
> +
> +static void fdctrl_write_DMA_cb(void *opaque, int ret)
> +{
> + FDC_opaque *opaque_cb = opaque;
> + FDCtrl *fdctrl = opaque_cb->fdctrl;
> + int nchan = opaque_cb->nchan;
> + int dma_len = opaque_cb->dma_len;
> + uint8_t status0 = 0x00, status1 = 0x00, status2 = 0x00;
> +
> + if (ret < 0) {
> + FLOPPY_DPRINTF("Floppy: error getting sector %d\n",
> + fd_sector(cur_drv));
> + /* Sure, image size is too small... */
> + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00,
> 0x00);
> + goto error;
> + }
> +
> + FLOPPY_DPRINTF("end transfer %d %d %d\n",
Indentation is wrong here (should be 4 spaces).
> + fdctrl->data_pos, dma_len, fdctrl->data_len);
> + if ((fdctrl->data_dir == FD_DIR_SCANE) ||
> + (fdctrl->data_dir == FD_DIR_SCANL) ||
> + (fdctrl->data_dir == FD_DIR_SCANH)) {
> + status2 = FD_SR2_SEH;
> + }
> +
> + if (FD_DID_SEEK(fdctrl->data_state)) {
> + status0 |= FD_SR0_SEEK;
> + }
> + fdctrl->data_len -= dma_len;
> + DMA_set_return(dma_len, nchan);
> + fdctrl_stop_transfer(fdctrl, status0, status1, status2);
> +
> +error:
> + qemu_vfree(fdctrl->iov.iov_base);
> + g_free(opaque_cb);
> + DMA_schedule(fdctrl->dma_chann);
> + return;
> +}
> +
> /* handlers for DMA transfers */
> static int fdctrl_transfer_handler (void *opaque, int nchan,
> int dma_pos, int dma_len)
> @@ -1189,6 +1313,11 @@ static int fdctrl_transfer_handler (void *opaque, int
> nchan,
> FDrive *cur_drv;
> int len, start_pos, rel_pos;
> uint8_t status0 = 0x00, status1 = 0x00, status2 = 0x00;
> + int fdc_sector_num = 0;
> + uint8_t *pfdc_string = NULL;
> + FDC_opaque *opaque_cb;
> + bool write_flag = false;
> + int write_sector = 0;
>
> fdctrl = opaque;
> if (fdctrl->msr & FD_MSR_RQM) {
> @@ -1196,109 +1325,104 @@ static int fdctrl_transfer_handler (void *opaque,
> int nchan,
> return 0;
> }
> cur_drv = get_cur_drv(fdctrl);
> - if (fdctrl->data_dir == FD_DIR_SCANE || fdctrl->data_dir == FD_DIR_SCANL
> ||
> - fdctrl->data_dir == FD_DIR_SCANH)
> + if ((fdctrl->data_dir == FD_DIR_SCANE) ||
> + (fdctrl->data_dir == FD_DIR_SCANL) ||
> + (fdctrl->data_dir == FD_DIR_SCANH)) {
> status2 = FD_SR2_SNS;
> - if (dma_len > fdctrl->data_len)
> + }
> + if (dma_len > fdctrl->data_len) {
> dma_len = fdctrl->data_len;
> + }
> if (cur_drv->bs == NULL) {
> - if (fdctrl->data_dir == FD_DIR_WRITE)
> - fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00,
> 0x00);
> - else
> + if (fdctrl->data_dir == FD_DIR_WRITE) {
> + fdctrl_stop_transfer(fdctrl,
> + FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
> + } else {
> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00);
> + }
> len = 0;
> goto transfer_error;
> }
> +
> + if ((fdctrl->data_dir != FD_DIR_WRITE) && (fdctrl->data_pos < dma_len)) {
> + fdc_sector_num = (dma_len + FD_SECTOR_LEN - 1) / FD_SECTOR_LEN;
> + opaque_cb = g_malloc0(sizeof(FDC_opaque));
> + pfdc_string = qemu_blockalign(cur_drv->bs,
> + fdc_sector_num * FD_SECTOR_LEN);
> + opaque_cb->fdctrl = fdctrl;
> + opaque_cb->nchan = nchan;
> + opaque_cb->dma_len = dma_len;
> +
> + fdctrl->iov.iov_base = pfdc_string;
> + fdctrl->iov.iov_len = fdc_sector_num * FD_SECTOR_LEN;
> + qemu_iovec_init_external(&fdctrl->qiov, &fdctrl->iov, 1);
> + bdrv_aio_readv(cur_drv->bs, fd_sector(cur_drv),
> + &fdctrl->qiov, fdc_sector_num, fdctrl_read_DMA_cb, opaque_cb);
> + return 0;
> + }
> +
> + if ((fdctrl->data_dir == FD_DIR_WRITE) && (fdctrl->data_pos < dma_len)) {
> + fdc_sector_num = (dma_len + FD_SECTOR_LEN - 1) / FD_SECTOR_LEN;
> + pfdc_string = g_malloc0(fdc_sector_num * FD_SECTOR_LEN);
qemu_blockalign()
> + write_flag = TRUE;
> + write_sector = fd_sector(cur_drv);
> + }
> +
> rel_pos = fdctrl->data_pos % FD_SECTOR_LEN;
> for (start_pos = fdctrl->data_pos; fdctrl->data_pos < dma_len;) {
> len = dma_len - fdctrl->data_pos;
> - if (len + rel_pos > FD_SECTOR_LEN)
> + if (len + rel_pos > FD_SECTOR_LEN) {
> len = FD_SECTOR_LEN - rel_pos;
> + }
> FLOPPY_DPRINTF("copy %d bytes (%d %d %d) %d pos %d %02x "
> "(%d-0x%08x 0x%08x)\n", len, dma_len, fdctrl->data_pos,
> fdctrl->data_len, GET_CUR_DRV(fdctrl), cur_drv->head,
> cur_drv->track, cur_drv->sect, fd_sector(cur_drv),
> fd_sector(cur_drv) * FD_SECTOR_LEN);
> - if (fdctrl->data_dir != FD_DIR_WRITE ||
> - len < FD_SECTOR_LEN || rel_pos != 0) {
> - /* READ & SCAN commands and realign to a sector for WRITE */
> - if (bdrv_read(cur_drv->bs, fd_sector(cur_drv),
> - fdctrl->fifo, 1) < 0) {
> - FLOPPY_DPRINTF("Floppy: error getting sector %d\n",
> - fd_sector(cur_drv));
> - /* Sure, image size is too small... */
> - memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
> - }
> - }
> - switch (fdctrl->data_dir) {
> - case FD_DIR_READ:
> - /* READ commands */
> - DMA_write_memory (nchan, fdctrl->fifo + rel_pos,
> - fdctrl->data_pos, len);
> - break;
> - case FD_DIR_WRITE:
> - /* WRITE commands */
> - if (cur_drv->ro) {
> - /* Handle readonly medium early, no need to do DMA, touch the
> - * LED or attempt any writes. A real floppy doesn't attempt
> - * to write to readonly media either. */
> - fdctrl_stop_transfer(fdctrl,
> - FD_SR0_ABNTERM | FD_SR0_SEEK, FD_SR1_NW,
> - 0x00);
> - goto transfer_error;
> - }
>
> - DMA_read_memory (nchan, fdctrl->fifo + rel_pos,
> + /* WRITE commands */
> + DMA_read_memory(nchan, pfdc_string + fdctrl->data_pos + rel_pos,
> fdctrl->data_pos, len);
> - if (bdrv_write(cur_drv->bs, fd_sector(cur_drv),
> - fdctrl->fifo, 1) < 0) {
> - FLOPPY_ERROR("writing sector %d\n", fd_sector(cur_drv));
> - fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK,
> 0x00, 0x00);
> - goto transfer_error;
> - }
> - break;
> - default:
> - /* SCAN commands */
> - {
> - uint8_t tmpbuf[FD_SECTOR_LEN];
> - int ret;
> - DMA_read_memory (nchan, tmpbuf, fdctrl->data_pos, len);
> - ret = memcmp(tmpbuf, fdctrl->fifo + rel_pos, len);
> - if (ret == 0) {
> - status2 = FD_SR2_SEH;
> - goto end_transfer;
> - }
> - if ((ret < 0 && fdctrl->data_dir == FD_DIR_SCANL) ||
> - (ret > 0 && fdctrl->data_dir == FD_DIR_SCANH)) {
> - status2 = 0x00;
> - goto end_transfer;
> - }
> - }
> - break;
> - }
> fdctrl->data_pos += len;
> rel_pos = fdctrl->data_pos % FD_SECTOR_LEN;
> if (rel_pos == 0) {
> /* Seek to next sector */
> - if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv))
> + if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
> break;
> + }
> }
> }
> - end_transfer:
> +
> + if (true == write_flag) {
> + opaque_cb = g_malloc0(sizeof(FDC_opaque));
> + opaque_cb->fdctrl = fdctrl;
> + opaque_cb->nchan = nchan;
> + opaque_cb->dma_len = dma_len;
> +
> + fdctrl->iov.iov_base = pfdc_string;
> + fdctrl->iov.iov_len = fdc_sector_num * FD_SECTOR_LEN;
> + qemu_iovec_init_external(&fdctrl->qiov, &fdctrl->iov, 1);
> + bdrv_aio_writev(cur_drv->bs, write_sector,
> + &fdctrl->qiov, fdc_sector_num, fdctrl_write_DMA_cb, opaque_cb);
> + return dma_len;
If we return dma_len here then we don't need to call DMA_set_return()
in fdctrl_write_DMA_cb().
Stefan