From cddd4ac7a2d55a76f238b04314fdd3bfee00e268 Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Thu, 5 Jul 2012 01:41:06 +0900 Subject: [PATCH 01/41] sheepdog: always use coroutine-based network functions This reduces some code duplication. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 113 ++++++++++++++++++++--------------------------- 1 file changed, 47 insertions(+), 66 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 809df39d9e..465dc97a28 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -498,26 +498,6 @@ success: return fd; } -static int send_req(int sockfd, SheepdogReq *hdr, void *data, - unsigned int *wlen) -{ - int ret; - - ret = qemu_send_full(sockfd, hdr, sizeof(*hdr), 0); - if (ret < sizeof(*hdr)) { - error_report("failed to send a req, %s", strerror(errno)); - return -errno; - } - - ret = qemu_send_full(sockfd, data, *wlen, 0); - if (ret < *wlen) { - error_report("failed to send a req, %s", strerror(errno)); - ret = -errno; - } - - return ret; -} - static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data, unsigned int *wlen) { @@ -537,49 +517,6 @@ static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data, return ret; } -static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data, - unsigned int *wlen, unsigned int *rlen); - -static int do_req(int sockfd, SheepdogReq *hdr, void *data, - unsigned int *wlen, unsigned int *rlen) -{ - int ret; - - if (qemu_in_coroutine()) { - return do_co_req(sockfd, hdr, data, wlen, rlen); - } - - socket_set_block(sockfd); - ret = send_req(sockfd, hdr, data, wlen); - if (ret < 0) { - goto out; - } - - ret = qemu_recv_full(sockfd, hdr, sizeof(*hdr), 0); - if (ret < sizeof(*hdr)) { - error_report("failed to get a rsp, %s", strerror(errno)); - ret = -errno; - goto out; - } - - if (*rlen > hdr->data_length) { - *rlen = hdr->data_length; - } - - if (*rlen) { - ret = qemu_recv_full(sockfd, data, *rlen, 0); - if (ret < *rlen) { - error_report("failed to get the data, %s", strerror(errno)); - ret = -errno; - goto out; - } - } - ret = 0; -out: - socket_set_nonblock(sockfd); - return ret; -} - static void restart_co_req(void *opaque) { Coroutine *co = opaque; @@ -587,11 +524,26 @@ static void restart_co_req(void *opaque) qemu_coroutine_enter(co, NULL); } -static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data, - unsigned int *wlen, unsigned int *rlen) +typedef struct SheepdogReqCo { + int sockfd; + SheepdogReq *hdr; + void *data; + unsigned int *wlen; + unsigned int *rlen; + int ret; + bool finished; +} SheepdogReqCo; + +static coroutine_fn void do_co_req(void *opaque) { int ret; Coroutine *co; + SheepdogReqCo *srco = opaque; + int sockfd = srco->sockfd; + SheepdogReq *hdr = srco->hdr; + void *data = srco->data; + unsigned int *wlen = srco->wlen; + unsigned int *rlen = srco->rlen; co = qemu_coroutine_self(); qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co); @@ -627,7 +579,36 @@ static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data, out: qemu_aio_set_fd_handler(sockfd, NULL, NULL, NULL, NULL); socket_set_nonblock(sockfd); - return ret; + + srco->ret = ret; + srco->finished = true; +} + +static int do_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen, unsigned int *rlen) +{ + Coroutine *co; + SheepdogReqCo srco = { + .sockfd = sockfd, + .hdr = hdr, + .data = data, + .wlen = wlen, + .rlen = rlen, + .ret = 0, + .finished = false, + }; + + if (qemu_in_coroutine()) { + do_co_req(&srco); + } else { + co = qemu_coroutine_create(do_co_req); + qemu_coroutine_enter(co, &srco); + while (!srco.finished) { + qemu_aio_wait(); + } + } + + return srco.ret; } static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, From 19db9b9042657ef80ddd4c94b1748dc2368aab70 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 10 Jul 2012 16:12:27 +0200 Subject: [PATCH 02/41] sheepdog: do not blindly memset all read buffers Only buffers that map to unallocated blocks need to be zeroed. Signed-off-by: Christoph Hellwig Acked-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 465dc97a28..a04ad99ead 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1571,18 +1571,25 @@ static int coroutine_fn sd_co_rw_vector(void *p) len = MIN(total - done, SD_DATA_OBJ_SIZE - offset); - if (!inode->data_vdi_id[idx]) { - if (acb->aiocb_type == AIOCB_READ_UDATA) { + switch (acb->aiocb_type) { + case AIOCB_READ_UDATA: + if (!inode->data_vdi_id[idx]) { + qemu_iovec_memset(acb->qiov, done, 0, len); goto done; } - - create = 1; - } else if (acb->aiocb_type == AIOCB_WRITE_UDATA - && !is_data_obj_writable(inode, idx)) { - /* Copy-On-Write */ - create = 1; - old_oid = oid; - flags = SD_FLAG_CMD_COW; + break; + case AIOCB_WRITE_UDATA: + if (!inode->data_vdi_id[idx]) { + create = 1; + } else if (!is_data_obj_writable(inode, idx)) { + /* Copy-On-Write */ + create = 1; + old_oid = oid; + flags = SD_FLAG_CMD_COW; + } + break; + default: + break; } if (create) { @@ -1668,20 +1675,12 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) { SheepdogAIOCB *acb; - int i, ret; + int ret; acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors, NULL, NULL); acb->aiocb_type = AIOCB_READ_UDATA; acb->aio_done_func = sd_finish_aiocb; - /* - * TODO: we can do better; we don't need to initialize - * blindly. - */ - for (i = 0; i < qiov->niov; i++) { - memset(qiov->iov[i].iov_base, 0, qiov->iov[i].iov_len); - } - ret = sd_co_rw_vector(acb); if (ret <= 0) { qemu_aio_release(acb); From 61a8d649ff1e125966b51d688ff43dc6ef6ca63b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:27 +0200 Subject: [PATCH 03/41] fdc: Move floppy geometry guessing back from block.c Commit 5bbdbb46 moved it to block.c because "other geometry guessing functions already reside in block.c". Device-specific functionality should be kept in device code, not the block layer. Move it back. Disk geometry guessing is still in block.c. To be moved out in a later patch series. Bonus: the floppy type used in pc_cmos_init() now obviously matches the one in the FDrive. Before, we relied on bdrv_get_floppy_geometry_hint() picking the same type both in fd_revalidate() and in pc_cmos_init(). Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block.c | 101 --------------------------------------------- block.h | 18 -------- hw/fdc.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++----- hw/fdc.h | 10 ++++- hw/pc.c | 11 +---- 5 files changed, 123 insertions(+), 139 deletions(-) diff --git a/block.c b/block.c index 0c923f2ae9..ffda1c2e67 100644 --- a/block.c +++ b/block.c @@ -2282,107 +2282,6 @@ void bdrv_set_io_limits(BlockDriverState *bs, bs->io_limits_enabled = bdrv_io_limits_enabled(bs); } -/* Recognize floppy formats */ -typedef struct FDFormat { - FDriveType drive; - uint8_t last_sect; - uint8_t max_track; - uint8_t max_head; - FDriveRate rate; -} FDFormat; - -static const FDFormat fd_formats[] = { - /* First entry is default format */ - /* 1.44 MB 3"1/2 floppy disks */ - { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, }, - /* 2.88 MB 3"1/2 floppy disks */ - { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, }, - { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, }, - { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, }, - { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, }, - { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, }, - /* 720 kB 3"1/2 floppy disks */ - { FDRIVE_DRV_144, 9, 80, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, }, - /* 1.2 MB 5"1/4 floppy disks */ - { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, }, - { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, }, - /* 720 kB 5"1/4 floppy disks */ - { FDRIVE_DRV_120, 9, 80, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, }, - /* 360 kB 5"1/4 floppy disks */ - { FDRIVE_DRV_120, 9, 40, 1, FDRIVE_RATE_300K, }, - { FDRIVE_DRV_120, 9, 40, 0, FDRIVE_RATE_300K, }, - { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, }, - { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, }, - /* 320 kB 5"1/4 floppy disks */ - { FDRIVE_DRV_120, 8, 40, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_120, 8, 40, 0, FDRIVE_RATE_250K, }, - /* 360 kB must match 5"1/4 better than 3"1/2... */ - { FDRIVE_DRV_144, 9, 80, 0, FDRIVE_RATE_250K, }, - /* end */ - { FDRIVE_DRV_NONE, -1, -1, 0, 0, }, -}; - -void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads, - int *max_track, int *last_sect, - FDriveType drive_in, FDriveType *drive, - FDriveRate *rate) -{ - const FDFormat *parse; - uint64_t nb_sectors, size; - int i, first_match, match; - - bdrv_get_geometry(bs, &nb_sectors); - match = -1; - first_match = -1; - for (i = 0; ; i++) { - parse = &fd_formats[i]; - if (parse->drive == FDRIVE_DRV_NONE) { - break; - } - if (drive_in == parse->drive || - drive_in == FDRIVE_DRV_NONE) { - size = (parse->max_head + 1) * parse->max_track * - parse->last_sect; - if (nb_sectors == size) { - match = i; - break; - } - if (first_match == -1) { - first_match = i; - } - } - } - if (match == -1) { - if (first_match == -1) { - match = 1; - } else { - match = first_match; - } - parse = &fd_formats[match]; - } - *nb_heads = parse->max_head + 1; - *max_track = parse->max_track; - *last_sect = parse->last_sect; - *drive = parse->drive; - *rate = parse->rate; -} - int bdrv_get_translation_hint(BlockDriverState *bs) { return bs->translation; diff --git a/block.h b/block.h index e34d94210d..b24f664411 100644 --- a/block.h +++ b/block.h @@ -269,24 +269,6 @@ void bdrv_set_geometry_hint(BlockDriverState *bs, void bdrv_set_translation_hint(BlockDriverState *bs, int translation); void bdrv_get_geometry_hint(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs); -typedef enum FDriveType { - FDRIVE_DRV_144 = 0x00, /* 1.44 MB 3"5 drive */ - FDRIVE_DRV_288 = 0x01, /* 2.88 MB 3"5 drive */ - FDRIVE_DRV_120 = 0x02, /* 1.2 MB 5"25 drive */ - FDRIVE_DRV_NONE = 0x03, /* No drive connected */ -} FDriveType; - -typedef enum FDriveRate { - FDRIVE_RATE_500K = 0x00, /* 500 Kbps */ - FDRIVE_RATE_300K = 0x01, /* 300 Kbps */ - FDRIVE_RATE_250K = 0x02, /* 250 Kbps */ - FDRIVE_RATE_1M = 0x03, /* 1 Mbps */ -} FDriveRate; - -void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads, - int *max_track, int *last_sect, - FDriveType drive_in, FDriveType *drive, - FDriveRate *rate); int bdrv_get_translation_hint(BlockDriverState *bs); void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error, BlockErrorAction on_write_error); diff --git a/hw/fdc.c b/hw/fdc.c index edf07063b2..41191c76c0 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -52,6 +52,113 @@ /********************************************************/ /* Floppy drive emulation */ +typedef enum FDriveRate { + FDRIVE_RATE_500K = 0x00, /* 500 Kbps */ + FDRIVE_RATE_300K = 0x01, /* 300 Kbps */ + FDRIVE_RATE_250K = 0x02, /* 250 Kbps */ + FDRIVE_RATE_1M = 0x03, /* 1 Mbps */ +} FDriveRate; + +typedef struct FDFormat { + FDriveType drive; + uint8_t last_sect; + uint8_t max_track; + uint8_t max_head; + FDriveRate rate; +} FDFormat; + +static const FDFormat fd_formats[] = { + /* First entry is default format */ + /* 1.44 MB 3"1/2 floppy disks */ + { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, }, + /* 2.88 MB 3"1/2 floppy disks */ + { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, }, + { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, }, + { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, }, + { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, }, + { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, }, + /* 720 kB 3"1/2 floppy disks */ + { FDRIVE_DRV_144, 9, 80, 1, FDRIVE_RATE_250K, }, + { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, }, + { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, }, + { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, }, + { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, }, + { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, }, + /* 1.2 MB 5"1/4 floppy disks */ + { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, }, + { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, }, + /* 720 kB 5"1/4 floppy disks */ + { FDRIVE_DRV_120, 9, 80, 1, FDRIVE_RATE_250K, }, + { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, }, + /* 360 kB 5"1/4 floppy disks */ + { FDRIVE_DRV_120, 9, 40, 1, FDRIVE_RATE_300K, }, + { FDRIVE_DRV_120, 9, 40, 0, FDRIVE_RATE_300K, }, + { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, }, + { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, }, + /* 320 kB 5"1/4 floppy disks */ + { FDRIVE_DRV_120, 8, 40, 1, FDRIVE_RATE_250K, }, + { FDRIVE_DRV_120, 8, 40, 0, FDRIVE_RATE_250K, }, + /* 360 kB must match 5"1/4 better than 3"1/2... */ + { FDRIVE_DRV_144, 9, 80, 0, FDRIVE_RATE_250K, }, + /* end */ + { FDRIVE_DRV_NONE, -1, -1, 0, 0, }, +}; + +static void pick_geometry(BlockDriverState *bs, int *nb_heads, + int *max_track, int *last_sect, + FDriveType drive_in, FDriveType *drive, + FDriveRate *rate) +{ + const FDFormat *parse; + uint64_t nb_sectors, size; + int i, first_match, match; + + bdrv_get_geometry(bs, &nb_sectors); + match = -1; + first_match = -1; + for (i = 0; ; i++) { + parse = &fd_formats[i]; + if (parse->drive == FDRIVE_DRV_NONE) { + break; + } + if (drive_in == parse->drive || + drive_in == FDRIVE_DRV_NONE) { + size = (parse->max_head + 1) * parse->max_track * + parse->last_sect; + if (nb_sectors == size) { + match = i; + break; + } + if (first_match == -1) { + first_match = i; + } + } + } + if (match == -1) { + if (first_match == -1) { + match = 1; + } else { + match = first_match; + } + parse = &fd_formats[match]; + } + *nb_heads = parse->max_head + 1; + *max_track = parse->max_track; + *last_sect = parse->last_sect; + *drive = parse->drive; + *rate = parse->rate; +} + #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv) #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive)) @@ -187,8 +294,8 @@ static void fd_revalidate(FDrive *drv) FLOPPY_DPRINTF("revalidate\n"); if (drv->bs != NULL) { ro = bdrv_is_read_only(drv->bs); - bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track, - &last_sect, drv->drive, &drive, &rate); + pick_geometry(drv->bs, &nb_heads, &max_track, + &last_sect, drv->drive, &drive, &rate); if (!bdrv_is_inserted(drv->bs)) { FLOPPY_DPRINTF("No disk in drive\n"); } else { @@ -2054,18 +2161,13 @@ static int sun4m_fdc_init1(SysBusDevice *dev) return fdctrl_init_common(fdctrl); } -void fdc_get_bs(BlockDriverState *bs[], ISADevice *dev) +FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i) { - FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev); - FDCtrl *fdctrl = &isa->state; - int i; + FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, fdc); - for (i = 0; i < MAX_FD; i++) { - bs[i] = fdctrl->drives[i].bs; - } + return isa->state.drives[i].drive; } - static const VMStateDescription vmstate_isa_fdc ={ .name = "fdc", .version_id = 2, diff --git a/hw/fdc.h b/hw/fdc.h index 1b32b17bef..b5c9f31074 100644 --- a/hw/fdc.h +++ b/hw/fdc.h @@ -6,11 +6,19 @@ /* fdc.c */ #define MAX_FD 2 +typedef enum FDriveType { + FDRIVE_DRV_144 = 0x00, /* 1.44 MB 3"5 drive */ + FDRIVE_DRV_288 = 0x01, /* 2.88 MB 3"5 drive */ + FDRIVE_DRV_120 = 0x02, /* 1.2 MB 5"25 drive */ + FDRIVE_DRV_NONE = 0x03, /* No drive connected */ +} FDriveType; + ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds); void fdctrl_init_sysbus(qemu_irq irq, int dma_chann, target_phys_addr_t mmio_base, DriveInfo **fds); void sun4m_fdctrl_init(qemu_irq irq, target_phys_addr_t io_base, DriveInfo **fds, qemu_irq *fdc_tc); -void fdc_get_bs(BlockDriverState *bs[], ISADevice *dev); + +FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i); #endif diff --git a/hw/pc.c b/hw/pc.c index c7e9ab3ee1..91cf77dafd 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -335,10 +335,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, ISADevice *floppy, BusState *idebus0, BusState *idebus1, ISADevice *s) { - int val, nb, nb_heads, max_track, last_sect, i; + int val, nb, i; FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE }; - FDriveRate rate; - BlockDriverState *fd[MAX_FD]; static pc_cmos_init_late_arg arg; /* various important CMOS locations needed by PC/Bochs bios */ @@ -381,13 +379,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, /* floppy type */ if (floppy) { - fdc_get_bs(fd, floppy); for (i = 0; i < 2; i++) { - if (fd[i]) { - bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track, - &last_sect, FDRIVE_DRV_NONE, - &fd_type[i], &rate); - } + fd_type[i] = isa_fdc_get_drive_type(floppy, i); } } val = (cmos_get_fd_drive_type(fd_type[0]) << 4) | From f91cbefe2d0eb3f7b5071bcb1fd3a02970f1a776 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:28 +0200 Subject: [PATCH 04/41] vvfat: Fix partition table Unless parameter ":floppy:" is given, vvfat creates a virtual image with DOS MBR defining a single partition which holds the FAT file system. The size of the virtual image depends on the width of the FAT: 32 MiB (CHS 64, 16, 63) for 12 bit FAT, 504 MiB (CHS 1024, 16, 63) for 16 and 32 bit FAT, leaving (64*16-1)*63 = 64449 and (1024*16-1)*64 = 1032129 sectors for the partition. However, it screws up the end of the partition in the MBR: FAT width param. start CHS end CHS start LBA size :32: 0,1,1 1023,14,63 63 1032065 :16: 0,1,1 1023,14,55 63 1032057 :12: 0,1,1 63,14,55 63 64377 The actual FAT file system nevertheless assumes the partition has 1032129 or 64449 sectors. Oops. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block/vvfat.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index 0fd3367d82..e2b83a21a0 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -394,11 +394,12 @@ static void init_mbr(BDRVVVFATState* s) /* LBA is used when partition is outside the CHS geometry */ lba = sector2CHS(s->bs, &partition->start_CHS, s->first_sectors_number-1); - lba|= sector2CHS(s->bs, &partition->end_CHS, s->sector_count); + lba |= sector2CHS(s->bs, &partition->end_CHS, s->bs->total_sectors - 1); /*LBA partitions are identified only by start/length_sector_long not by CHS*/ - partition->start_sector_long =cpu_to_le32(s->first_sectors_number-1); - partition->length_sector_long=cpu_to_le32(s->sector_count - s->first_sectors_number+1); + partition->start_sector_long = cpu_to_le32(s->first_sectors_number - 1); + partition->length_sector_long = cpu_to_le32(s->bs->total_sectors + - s->first_sectors_number + 1); /* FAT12/FAT16/FAT32 */ /* DOS uses different types when partition is LBA, From 4480e0f924a42e1db8b8cfcac4d0634dd1bb27a0 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:29 +0200 Subject: [PATCH 05/41] vvfat: Do not clobber the user's geometry vvfat creates a virtual VFAT filesystem with a certain logical geometry that depends on its options. It sets the "geometry hint" to this geometry. It is the only block driver to do this. The geometry hint is about about *physical* geometry, and used only by certain hard disk device models. vvfat's hint is normally invisible for device models, because bdrv_open() puts a raw format on top of vvfat's fat protocol. That raw format is where drive_init() puts the user's geometry (if any), and where the device model gets it from. Nobody complained, because the default physical geometry is the same as vvfat's logical geometry: opts LCHS def. PCHS 1024,16,63 same :32: 1024,16,63 same :16: 1024,16,63 same :12: 64,16,63 same Except when you specify :floppy: opts LCHS def. PCHS :floppy: 80, 2,36 5,16,63 :32:floppy: 80, 2,36 5,16,63 :16:floppy: 80, 2,36 5,16,63 :12:floppy: 80, 2,18 2,16,63 Silly thing to do for use with a hard disk. However, the "raw" format can be suppressed by adding an redundant-looking "format=vvfat" to "file=fat:FOO". Then, vvfat's hint clobbers the user's geometry, i.e. -drive options cyls, heads, secs get silently ignored. Don't do that. No change without format=vvfat. With it, the user's hard disk geometry (-drive options cyls, heads, secs) is now obeyed, and the default hard disk geometry with :floppy: now matches the one without format=vvfat. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block/vvfat.c | 53 ++++++++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index e2b83a21a0..7b1dcee144 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -359,11 +359,12 @@ typedef struct BDRVVVFATState { * if the position is outside the specified geometry, fill maximum value for CHS * and return 1 to signal overflow. */ -static int sector2CHS(BlockDriverState* bs, mbr_chs_t * chs, int spos){ +static int sector2CHS(mbr_chs_t *chs, int spos, int cyls, int heads, int secs) +{ int head,sector; - sector = spos % (bs->secs); spos/= bs->secs; - head = spos % (bs->heads); spos/= bs->heads; - if(spos >= bs->cyls){ + sector = spos % secs; spos /= secs; + head = spos % heads; spos /= heads; + if (spos >= cyls) { /* Overflow, it happens if 32bit sector positions are used, while CHS is only 24bit. Windows/Dos is said to take 1023/255/63 as nonrepresentable CHS */ @@ -378,7 +379,7 @@ static int sector2CHS(BlockDriverState* bs, mbr_chs_t * chs, int spos){ return 0; } -static void init_mbr(BDRVVVFATState* s) +static void init_mbr(BDRVVVFATState *s, int cyls, int heads, int secs) { /* TODO: if the files mbr.img and bootsect.img exist, use them */ mbr_t* real_mbr=(mbr_t*)s->first_sectors; @@ -393,8 +394,10 @@ static void init_mbr(BDRVVVFATState* s) partition->attributes=0x80; /* bootable */ /* LBA is used when partition is outside the CHS geometry */ - lba = sector2CHS(s->bs, &partition->start_CHS, s->first_sectors_number-1); - lba |= sector2CHS(s->bs, &partition->end_CHS, s->bs->total_sectors - 1); + lba = sector2CHS(&partition->start_CHS, s->first_sectors_number - 1, + cyls, heads, secs); + lba |= sector2CHS(&partition->end_CHS, s->bs->total_sectors - 1, + cyls, heads, secs); /*LBA partitions are identified only by start/length_sector_long not by CHS*/ partition->start_sector_long = cpu_to_le32(s->first_sectors_number - 1); @@ -831,7 +834,7 @@ static inline off_t cluster2sector(BDRVVVFATState* s, uint32_t cluster_num) } static int init_directories(BDRVVVFATState* s, - const char* dirname) + const char *dirname, int heads, int secs) { bootsector_t* bootsector; mapping_t* mapping; @@ -958,8 +961,8 @@ static int init_directories(BDRVVVFATState* s, bootsector->media_type=(s->first_sectors_number>1?0xf8:0xf0); /* media descriptor (f8=hd, f0=3.5 fd)*/ s->fat.pointer[0] = bootsector->media_type; bootsector->sectors_per_fat=cpu_to_le16(s->sectors_per_fat); - bootsector->sectors_per_track=cpu_to_le16(s->bs->secs); - bootsector->number_of_heads=cpu_to_le16(s->bs->heads); + bootsector->sectors_per_track = cpu_to_le16(secs); + bootsector->number_of_heads = cpu_to_le16(heads); bootsector->hidden_sectors=cpu_to_le32(s->first_sectors_number==1?0:0x3f); bootsector->total_sectors=cpu_to_le32(s->sector_count>0xffff?s->sector_count:0); @@ -992,7 +995,7 @@ static void vvfat_rebind(BlockDriverState *bs) static int vvfat_open(BlockDriverState *bs, const char* dirname, int flags) { BDRVVVFATState *s = bs->opaque; - int i; + int i, cyls, heads, secs; #ifdef DEBUG vvv = s; @@ -1034,24 +1037,28 @@ DLOG(if (stderr == NULL) { /* 1.44MB or 2.88MB floppy. 2.88MB can be FAT12 (default) or FAT16. */ if (!s->fat_type) { s->fat_type = 12; - bs->secs = 36; + secs = 36; s->sectors_per_cluster=2; } else { - bs->secs=(s->fat_type == 12 ? 18 : 36); + secs = s->fat_type == 12 ? 18 : 36; s->sectors_per_cluster=1; } s->first_sectors_number = 1; - bs->cyls=80; bs->heads=2; + cyls = 80; + heads = 2; } else { /* 32MB or 504MB disk*/ if (!s->fat_type) { s->fat_type = 16; } - bs->cyls=(s->fat_type == 12 ? 64 : 1024); - bs->heads=16; bs->secs=63; + cyls = s->fat_type == 12 ? 64 : 1024; + heads = 16; + secs = 63; } + fprintf(stderr, "vvfat %s chs %d,%d,%d\n", + dirname, cyls, heads, secs); - s->sector_count=bs->cyls*bs->heads*bs->secs-(s->first_sectors_number-1); + s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1); if (strstr(dirname, ":rw:")) { if (enable_write_target(s)) @@ -1067,18 +1074,16 @@ DLOG(if (stderr == NULL) { else dirname += i+1; - bs->total_sectors=bs->cyls*bs->heads*bs->secs; + bs->total_sectors = cyls * heads * secs; - if(init_directories(s, dirname)) + if (init_directories(s, dirname, heads, secs)) { return -1; + } s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->cluster_count; - if(s->first_sectors_number==0x40) - init_mbr(s); - else { - /* MS-DOS does not like to know about CHS (?). */ - bs->heads = bs->cyls = bs->secs = 0; + if (s->first_sectors_number == 0x40) { + init_mbr(s, cyls, heads, secs); } // assert(is_consistent(s)); From 0e8a8c8f6d988f3907d7cdba877a711a4d47ec5c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:30 +0200 Subject: [PATCH 06/41] qtest: Add hard disk geometry test So far covers only IDE and tests only CMOS contents. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- tests/Makefile | 2 + tests/hd-geo-test.c | 403 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 405 insertions(+) create mode 100644 tests/hd-geo-test.c diff --git a/tests/Makefile b/tests/Makefile index d687ecce3f..9675ba7c13 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -21,6 +21,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh # All QTests for now are POSIX-only, but the dependencies are # really in libqtest, not in the testcases themselves. check-qtest-i386-y = tests/fdc-test$(EXESUF) +check-qtest-i386-y += tests/hd-geo-test$(EXESUF) check-qtest-i386-y += tests/rtc-test$(EXESUF) check-qtest-x86_64-y = $(check-qtest-i386-y) check-qtest-sparc-y = tests/m48t59-test$(EXESUF) @@ -72,6 +73,7 @@ tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $( tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y) tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y) tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y) +tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y) # QTest rules diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c new file mode 100644 index 0000000000..cc447a26bd --- /dev/null +++ b/tests/hd-geo-test.c @@ -0,0 +1,403 @@ +/* + * Hard disk geometry test cases. + * + * Copyright (c) 2012 Red Hat Inc. + * + * Authors: + * Markus Armbruster , + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +/* + * Covers only IDE and tests only CMOS contents. Better than nothing. + * Improvements welcome. + */ + +#include +#include +#include +#include +#include "qemu-common.h" +#include "libqtest.h" + +static const char test_image[] = "/tmp/qtest.XXXXXX"; + +static char *create_test_img(int secs) +{ + char *template = strdup("/tmp/qtest.XXXXXX"); + int fd, ret; + + fd = mkstemp(template); + g_assert(fd >= 0); + ret = ftruncate(fd, (off_t)secs * 512); + g_assert(ret == 0); + close(fd); + return template; +} + +typedef struct { + int cyls, heads, secs, trans; +} CHST; + +typedef enum { + mbr_blank, mbr_lba, mbr_chs, + mbr_last +} MBRcontents; + +typedef enum { + /* order is relevant */ + backend_small, backend_large, backend_empty, + backend_last +} Backend; + +static const int img_secs[backend_last] = { + [backend_small] = 61440, + [backend_large] = 8388608, + [backend_empty] = -1, +}; + +static const CHST hd_chst[backend_last][mbr_last] = { + [backend_small] = { + [mbr_blank] = { 60, 16, 63, 0 }, + [mbr_lba] = { 60, 16, 63, 2 }, + [mbr_chs] = { 60, 16, 63, 0 } + }, + [backend_large] = { + [mbr_blank] = { 8322, 16, 63, 1 }, + [mbr_lba] = { 8322, 16, 63, 1 }, + [mbr_chs] = { 8322, 16, 63, 0 } + }, +}; + +static const char *img_file_name[backend_last]; + +static const CHST *cur_ide[4]; + +static bool is_hd(const CHST *expected_chst) +{ + return expected_chst && expected_chst->cyls; +} + +static void test_cmos_byte(int reg, int expected) +{ + enum { cmos_base = 0x70 }; + int actual; + + outb(cmos_base + 0, reg); + actual = inb(cmos_base + 1); + g_assert(actual == expected); +} + +static void test_cmos_bytes(int reg0, int n, uint8_t expected[]) +{ + int i; + + for (i = 0; i < 9; i++) { + test_cmos_byte(reg0 + i, expected[i]); + } +} + +static void test_cmos_disk_data(void) +{ + test_cmos_byte(0x12, + (is_hd(cur_ide[0]) ? 0xf0 : 0) | + (is_hd(cur_ide[1]) ? 0x0f : 0)); +} + +static void test_cmos_drive_cyl(int reg0, const CHST *expected_chst) +{ + if (is_hd(expected_chst)) { + int c = expected_chst->cyls; + int h = expected_chst->heads; + int s = expected_chst->secs; + uint8_t expected_bytes[9] = { + c & 0xff, c >> 8, h, 0xff, 0xff, 0xc0 | ((h > 8) << 3), + c & 0xff, c >> 8, s + }; + test_cmos_bytes(reg0, 9, expected_bytes); + } else { + int i; + + for (i = 0; i < 9; i++) { + test_cmos_byte(reg0 + i, 0); + } + } +} + +static void test_cmos_drive1(void) +{ + test_cmos_byte(0x19, is_hd(cur_ide[0]) ? 47 : 0); + test_cmos_drive_cyl(0x1b, cur_ide[0]); +} + +static void test_cmos_drive2(void) +{ + test_cmos_byte(0x1a, is_hd(cur_ide[1]) ? 47 : 0); + test_cmos_drive_cyl(0x24, cur_ide[1]); +} + +static void test_cmos_disktransflag(void) +{ + int val, i; + + val = 0; + for (i = 0; i < ARRAY_SIZE(cur_ide); i++) { + if (is_hd(cur_ide[i])) { + val |= cur_ide[i]->trans << (2 * i); + } + } + test_cmos_byte(0x39, val); +} + +static void test_cmos(void) +{ + test_cmos_disk_data(); + test_cmos_drive1(); + test_cmos_drive2(); + test_cmos_disktransflag(); +} + +static int append_arg(int argc, char *argv[], int argv_sz, char *arg) +{ + g_assert(argc + 1 < argv_sz); + argv[argc++] = arg; + argv[argc] = NULL; + return argc; +} + +static int setup_common(char *argv[], int argv_sz) +{ + memset(cur_ide, 0, sizeof(cur_ide)); + return append_arg(0, argv, argv_sz, + g_strdup("-nodefaults -display none")); +} + +static void setup_mbr(int img_idx, MBRcontents mbr) +{ + static const uint8_t part_lba[16] = { + /* chs 0,1,1 (lba 63) to chs 0,127,63 (8001 sectors) */ + 0x80, 1, 1, 0, 6, 127, 63, 0, 63, 0, 0, 0, 0x41, 0x1F, 0, 0, + }; + static const uint8_t part_chs[16] = { + /* chs 0,1,1 (lba 63) to chs 7,15,63 (8001 sectors) */ + 0x80, 1, 1, 0, 6, 15, 63, 7, 63, 0, 0, 0, 0x41, 0x1F, 0, 0, + }; + uint8_t buf[512]; + int fd, ret; + + memset(buf, 0, sizeof(buf)); + + if (mbr != mbr_blank) { + buf[0x1fe] = 0x55; + buf[0x1ff] = 0xAA; + memcpy(buf + 0x1BE, mbr == mbr_lba ? part_lba : part_chs, 16); + } + + fd = open(img_file_name[img_idx], O_WRONLY); + g_assert(fd >= 0); + ret = write(fd, buf, sizeof(buf)); + g_assert(ret == sizeof(buf)); + close(fd); +} + +static int setup_ide(int argc, char *argv[], int argv_sz, + int ide_idx, const char *dev, int img_idx, + MBRcontents mbr, const char *opts) +{ + char *s1, *s2, *s3; + + s1 = g_strdup_printf("-drive id=drive%d,if=%s", + ide_idx, dev ? "none" : "ide"); + s2 = dev ? g_strdup("") : g_strdup_printf(",index=%d", ide_idx); + + if (img_secs[img_idx] >= 0) { + setup_mbr(img_idx, mbr); + s3 = g_strdup_printf(",file=%s", img_file_name[img_idx]); + } else { + s3 = g_strdup(",media=cdrom"); + } + argc = append_arg(argc, argv, argv_sz, + g_strdup_printf("%s%s%s%s", s1, s2, s3, opts)); + g_free(s1); + g_free(s2); + g_free(s3); + + if (dev) { + argc = append_arg(argc, argv, argv_sz, + g_strdup_printf("-device %s,drive=drive%d," + "bus=ide.%d,unit=%d", + dev, ide_idx, + ide_idx / 2, ide_idx % 2)); + } + return argc; +} + +/* + * Test case: no IDE devices + */ +static void test_ide_none(void) +{ + char *argv[256]; + + setup_common(argv, ARRAY_SIZE(argv)); + qtest_start(g_strjoinv(" ", argv)); + test_cmos(); + qtest_quit(global_qtest); +} + +static void test_ide_mbr(bool use_device, MBRcontents mbr) +{ + char *argv[256]; + int argc; + Backend i; + const char *dev; + + argc = setup_common(argv, ARRAY_SIZE(argv)); + for (i = 0; i < backend_last; i++) { + cur_ide[i] = &hd_chst[i][mbr]; + dev = use_device ? (is_hd(cur_ide[i]) ? "ide-hd" : "ide-cd") : NULL; + argc = setup_ide(argc, argv, ARRAY_SIZE(argv), i, dev, i, mbr, ""); + } + qtest_start(g_strjoinv(" ", argv)); + test_cmos(); + qtest_quit(global_qtest); +} + +/* + * Test case: IDE devices (if=ide) with blank MBRs + */ +static void test_ide_drive_mbr_blank(void) +{ + test_ide_mbr(false, mbr_blank); +} + +/* + * Test case: IDE devices (if=ide) with MBRs indicating LBA is in use + */ +static void test_ide_drive_mbr_lba(void) +{ + test_ide_mbr(false, mbr_lba); +} + +/* + * Test case: IDE devices (if=ide) with MBRs indicating CHS is in use + */ +static void test_ide_drive_mbr_chs(void) +{ + test_ide_mbr(false, mbr_chs); +} + +/* + * Test case: IDE devices (if=none) with blank MBRs + */ +static void test_ide_device_mbr_blank(void) +{ + test_ide_mbr(true, mbr_blank); +} + +/* + * Test case: IDE devices (if=none) with MBRs indicating LBA is in use + */ +static void test_ide_device_mbr_lba(void) +{ + test_ide_mbr(true, mbr_lba); +} + +/* + * Test case: IDE devices (if=none) with MBRs indicating CHS is in use + */ +static void test_ide_device_mbr_chs(void) +{ + test_ide_mbr(true, mbr_chs); +} + +static void test_ide_drive_user(const char *dev, bool trans) +{ + char *argv[256], *opts; + int argc; + int secs = img_secs[backend_small]; + const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans }; + + argc = setup_common(argv, ARRAY_SIZE(argv)); + opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s", + expected_chst.cyls, expected_chst.heads, + expected_chst.secs, + trans ? ",trans=lba" : ""); + cur_ide[0] = &expected_chst; + argc = setup_ide(argc, argv, ARRAY_SIZE(argv), + 0, dev, backend_small, mbr_chs, opts); + g_free(opts); + qtest_start(g_strjoinv(" ", argv)); + test_cmos(); + qtest_quit(global_qtest); +} + +/* + * Test case: IDE device (if=ide) with explicit CHS + */ +static void test_ide_drive_user_chs(void) +{ + test_ide_drive_user(NULL, false); +} + +/* + * Test case: IDE device (if=ide) with explicit CHS and translation + */ +static void test_ide_drive_user_chst(void) +{ + test_ide_drive_user(NULL, true); +} + +/* + * Test case: IDE device (if=none) with explicit CHS + */ +static void test_ide_device_user_chs(void) +{ + test_ide_drive_user("ide-hd", false); +} + +/* + * Test case: IDE device (if=none) with explicit CHS and translation + */ +static void test_ide_device_user_chst(void) +{ + test_ide_drive_user("ide-hd", true); +} + +int main(int argc, char **argv) +{ + Backend i; + int ret; + + g_test_init(&argc, &argv, NULL); + + for (i = 0; i < backend_last; i++) { + if (img_secs[i] >= 0) { + img_file_name[i] = create_test_img(img_secs[i]); + } else { + img_file_name[i] = NULL; + } + } + + qtest_add_func("hd-geo/ide/none", test_ide_none); + qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank); + qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba); + qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs); + qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs); + qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst); + qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank); + qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba); + qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs); + qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs); + qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst); + + ret = g_test_run(); + + for (i = 0; i < backend_last; i++) { + unlink(img_file_name[i]); + } + + return ret; +} From 9db1c0f7a94c6382e2b3e1365566a9a8b8ae74c1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:31 +0200 Subject: [PATCH 07/41] hd-geometry: Move disk geometry guessing back from block.c Commit f3d54fc4 factored it out of hw/ide.c for reuse. Sensible, except it was put into block.c. Device-specific functionality should be kept in device code, not the block layer. Move it to hw/hd-geometry.c, and make stylistic changes required to keep checkpatch.pl happy. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block.c | 121 ---------------------------------- block.h | 1 - hw/Makefile.objs | 2 +- hw/block-common.h | 21 ++++++ hw/hd-geometry.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++ hw/ide/core.c | 3 +- hw/scsi-disk.c | 5 +- hw/virtio-blk.c | 3 +- 8 files changed, 191 insertions(+), 127 deletions(-) create mode 100644 hw/block-common.h create mode 100644 hw/hd-geometry.c diff --git a/block.c b/block.c index ffda1c2e67..06323cfe53 100644 --- a/block.c +++ b/block.c @@ -2132,127 +2132,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr) *nb_sectors_ptr = length; } -struct partition { - uint8_t boot_ind; /* 0x80 - active */ - uint8_t head; /* starting head */ - uint8_t sector; /* starting sector */ - uint8_t cyl; /* starting cylinder */ - uint8_t sys_ind; /* What partition type */ - uint8_t end_head; /* end head */ - uint8_t end_sector; /* end sector */ - uint8_t end_cyl; /* end cylinder */ - uint32_t start_sect; /* starting sector counting from 0 */ - uint32_t nr_sects; /* nr of sectors in partition */ -} QEMU_PACKED; - -/* try to guess the disk logical geometry from the MSDOS partition table. Return 0 if OK, -1 if could not guess */ -static int guess_disk_lchs(BlockDriverState *bs, - int *pcylinders, int *pheads, int *psectors) -{ - uint8_t buf[BDRV_SECTOR_SIZE]; - int i, heads, sectors, cylinders; - struct partition *p; - uint32_t nr_sects; - uint64_t nb_sectors; - - bdrv_get_geometry(bs, &nb_sectors); - - /** - * The function will be invoked during startup not only in sync I/O mode, - * but also in async I/O mode. So the I/O throttling function has to - * be disabled temporarily here, not permanently. - */ - if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) { - return -1; - } - /* test msdos magic */ - if (buf[510] != 0x55 || buf[511] != 0xaa) - return -1; - for(i = 0; i < 4; i++) { - p = ((struct partition *)(buf + 0x1be)) + i; - nr_sects = le32_to_cpu(p->nr_sects); - if (nr_sects && p->end_head) { - /* We make the assumption that the partition terminates on - a cylinder boundary */ - heads = p->end_head + 1; - sectors = p->end_sector & 63; - if (sectors == 0) - continue; - cylinders = nb_sectors / (heads * sectors); - if (cylinders < 1 || cylinders > 16383) - continue; - *pheads = heads; - *psectors = sectors; - *pcylinders = cylinders; -#if 0 - printf("guessed geometry: LCHS=%d %d %d\n", - cylinders, heads, sectors); -#endif - return 0; - } - } - return -1; -} - -void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs) -{ - int translation, lba_detected = 0; - int cylinders, heads, secs; - uint64_t nb_sectors; - - /* if a geometry hint is available, use it */ - bdrv_get_geometry(bs, &nb_sectors); - bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs); - translation = bdrv_get_translation_hint(bs); - if (cylinders != 0) { - *pcyls = cylinders; - *pheads = heads; - *psecs = secs; - } else { - if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) { - if (heads > 16) { - /* if heads > 16, it means that a BIOS LBA - translation was active, so the default - hardware geometry is OK */ - lba_detected = 1; - goto default_geometry; - } else { - *pcyls = cylinders; - *pheads = heads; - *psecs = secs; - /* disable any translation to be in sync with - the logical geometry */ - if (translation == BIOS_ATA_TRANSLATION_AUTO) { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_NONE); - } - } - } else { - default_geometry: - /* if no geometry, use a standard physical disk geometry */ - cylinders = nb_sectors / (16 * 63); - - if (cylinders > 16383) - cylinders = 16383; - else if (cylinders < 2) - cylinders = 2; - *pcyls = cylinders; - *pheads = 16; - *psecs = 63; - if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) { - if ((*pcyls * *pheads) <= 131072) { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_LARGE); - } else { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_LBA); - } - } - } - bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); - } -} - void bdrv_set_geometry_hint(BlockDriverState *bs, int cyls, int heads, int secs) { diff --git a/block.h b/block.h index b24f664411..993894efe4 100644 --- a/block.h +++ b/block.h @@ -178,7 +178,6 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset); int64_t bdrv_getlength(BlockDriverState *bs); int64_t bdrv_get_allocated_file_size(BlockDriverState *bs); void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr); -void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs); int bdrv_commit(BlockDriverState *bs); int bdrv_commit_all(void); int bdrv_change_backing_file(BlockDriverState *bs, diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 9a350deafb..c3bdedcf28 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -138,7 +138,7 @@ common-obj-$(CONFIG_MAX111X) += max111x.o common-obj-$(CONFIG_DS1338) += ds1338.o common-obj-y += i2c.o smbus.o smbus_eeprom.o common-obj-y += eeprom93xx.o -common-obj-y += scsi-disk.o cdrom.o +common-obj-y += scsi-disk.o cdrom.o hd-geometry.o common-obj-y += scsi-generic.o scsi-bus.o common-obj-y += hid.o common-obj-$(CONFIG_SSI) += ssi.o diff --git a/hw/block-common.h b/hw/block-common.h new file mode 100644 index 0000000000..3a4d4c6292 --- /dev/null +++ b/hw/block-common.h @@ -0,0 +1,21 @@ +/* + * Common code for block device models + * + * Copyright (C) 2012 Red Hat, Inc. + * Copyright (c) 2003-2008 Fabrice Bellard + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#ifndef HW_BLOCK_COMMON_H +#define HW_BLOCK_COMMON_H + +#include "qemu-common.h" + +/* Hard disk geometry */ + +void hd_geometry_guess(BlockDriverState *bs, + int *pcyls, int *pheads, int *psecs); + +#endif diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c new file mode 100644 index 0000000000..c45eafdd4c --- /dev/null +++ b/hw/hd-geometry.c @@ -0,0 +1,162 @@ +/* + * Hard disk geometry utilities + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * This file incorporates work covered by the following copyright and + * permission notice: + * + * Copyright (c) 2003 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "block.h" +#include "hw/block-common.h" + +struct partition { + uint8_t boot_ind; /* 0x80 - active */ + uint8_t head; /* starting head */ + uint8_t sector; /* starting sector */ + uint8_t cyl; /* starting cylinder */ + uint8_t sys_ind; /* What partition type */ + uint8_t end_head; /* end head */ + uint8_t end_sector; /* end sector */ + uint8_t end_cyl; /* end cylinder */ + uint32_t start_sect; /* starting sector counting from 0 */ + uint32_t nr_sects; /* nr of sectors in partition */ +} QEMU_PACKED; + +/* try to guess the disk logical geometry from the MSDOS partition table. + Return 0 if OK, -1 if could not guess */ +static int guess_disk_lchs(BlockDriverState *bs, + int *pcylinders, int *pheads, int *psectors) +{ + uint8_t buf[BDRV_SECTOR_SIZE]; + int i, heads, sectors, cylinders; + struct partition *p; + uint32_t nr_sects; + uint64_t nb_sectors; + + bdrv_get_geometry(bs, &nb_sectors); + + /** + * The function will be invoked during startup not only in sync I/O mode, + * but also in async I/O mode. So the I/O throttling function has to + * be disabled temporarily here, not permanently. + */ + if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) { + return -1; + } + /* test msdos magic */ + if (buf[510] != 0x55 || buf[511] != 0xaa) { + return -1; + } + for (i = 0; i < 4; i++) { + p = ((struct partition *)(buf + 0x1be)) + i; + nr_sects = le32_to_cpu(p->nr_sects); + if (nr_sects && p->end_head) { + /* We make the assumption that the partition terminates on + a cylinder boundary */ + heads = p->end_head + 1; + sectors = p->end_sector & 63; + if (sectors == 0) { + continue; + } + cylinders = nb_sectors / (heads * sectors); + if (cylinders < 1 || cylinders > 16383) { + continue; + } + *pheads = heads; + *psectors = sectors; + *pcylinders = cylinders; +#if 0 + printf("guessed geometry: LCHS=%d %d %d\n", + cylinders, heads, sectors); +#endif + return 0; + } + } + return -1; +} + +void hd_geometry_guess(BlockDriverState *bs, + int *pcyls, int *pheads, int *psecs) +{ + int translation, lba_detected = 0; + int cylinders, heads, secs; + uint64_t nb_sectors; + + /* if a geometry hint is available, use it */ + bdrv_get_geometry(bs, &nb_sectors); + bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs); + translation = bdrv_get_translation_hint(bs); + if (cylinders != 0) { + *pcyls = cylinders; + *pheads = heads; + *psecs = secs; + } else { + if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) { + if (heads > 16) { + /* if heads > 16, it means that a BIOS LBA + translation was active, so the default + hardware geometry is OK */ + lba_detected = 1; + goto default_geometry; + } else { + *pcyls = cylinders; + *pheads = heads; + *psecs = secs; + /* disable any translation to be in sync with + the logical geometry */ + if (translation == BIOS_ATA_TRANSLATION_AUTO) { + bdrv_set_translation_hint(bs, + BIOS_ATA_TRANSLATION_NONE); + } + } + } else { + default_geometry: + /* if no geometry, use a standard physical disk geometry */ + cylinders = nb_sectors / (16 * 63); + + if (cylinders > 16383) { + cylinders = 16383; + } else if (cylinders < 2) { + cylinders = 2; + } + *pcyls = cylinders; + *pheads = 16; + *psecs = 63; + if ((lba_detected == 1) + && (translation == BIOS_ATA_TRANSLATION_AUTO)) { + if ((*pcyls * *pheads) <= 131072) { + bdrv_set_translation_hint(bs, + BIOS_ATA_TRANSLATION_LARGE); + } else { + bdrv_set_translation_hint(bs, + BIOS_ATA_TRANSLATION_LBA); + } + } + } + bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); + } +} diff --git a/hw/ide/core.c b/hw/ide/core.c index 71d4d7732a..0d1bf10358 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -30,6 +30,7 @@ #include "qemu-timer.h" #include "sysemu.h" #include "dma.h" +#include "hw/block-common.h" #include "blockdev.h" #include @@ -1933,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, s->drive_kind = kind; bdrv_get_geometry(bs, &nb_sectors); - bdrv_guess_geometry(bs, &cylinders, &heads, &secs); + hd_geometry_guess(bs, &cylinders, &heads, &secs); if (cylinders < 1 || cylinders > 16383) { error_report("cyls must be between 1 and 16383"); return -1; diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 34336b1b58..5339c2ef9d 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -34,6 +34,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0) #include "scsi-defs.h" #include "sysemu.h" #include "blockdev.h" +#include "hw/block-common.h" #include "dma.h" #ifdef __linux @@ -989,7 +990,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, break; } /* if a geometry hint is available, use it */ - bdrv_guess_geometry(bdrv, &cylinders, &heads, &secs); + hd_geometry_guess(bdrv, &cylinders, &heads, &secs); p[2] = (cylinders >> 16) & 0xff; p[3] = (cylinders >> 8) & 0xff; p[4] = cylinders & 0xff; @@ -1023,7 +1024,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, p[2] = 5000 >> 8; p[3] = 5000 & 0xff; /* if a geometry hint is available, use it */ - bdrv_guess_geometry(bdrv, &cylinders, &heads, &secs); + hd_geometry_guess(bdrv, &cylinders, &heads, &secs); p[4] = heads & 0xff; p[5] = secs & 0xff; p[6] = s->qdev.blocksize >> 8; diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index fe0774617b..f16c5ceb2a 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -14,6 +14,7 @@ #include "qemu-common.h" #include "qemu-error.h" #include "trace.h" +#include "hw/block-common.h" #include "blockdev.h" #include "virtio-blk.h" #include "scsi-defs.h" @@ -622,7 +623,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) s->blk = blk; s->rq = NULL; s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; - bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs); + hd_geometry_guess(s->bs, &cylinders, &heads, &secs); s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output); From 31f7eedfa6e7e8f4c4760930cbe82bf969e73fa7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:32 +0200 Subject: [PATCH 08/41] hd-geometry: Add tracepoints Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/hd-geometry.c | 7 +++---- trace-events | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index c45eafdd4c..f0dd02149d 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -32,6 +32,7 @@ #include "block.h" #include "hw/block-common.h" +#include "trace.h" struct partition { uint8_t boot_ind; /* 0x80 - active */ @@ -89,10 +90,7 @@ static int guess_disk_lchs(BlockDriverState *bs, *pheads = heads; *psectors = sectors; *pcylinders = cylinders; -#if 0 - printf("guessed geometry: LCHS=%d %d %d\n", - cylinders, heads, sectors); -#endif + trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors); return 0; } } @@ -159,4 +157,5 @@ void hd_geometry_guess(BlockDriverState *bs, } bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); } + trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation); } diff --git a/trace-events b/trace-events index fc32bc69cb..5f27f1a8d1 100644 --- a/trace-events +++ b/trace-events @@ -141,6 +141,10 @@ ecc_mem_readl_ecr1(uint32_t ret) "Read event count 2 %08x" ecc_diag_mem_writeb(uint64_t addr, uint32_t val) "Write diagnostic %"PRId64" = %02x" ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= %02x" +# hw/hd-geometry.c +hd_geometry_lchs_guess(void *bs, int cyls, int heads, int secs) "bs %p LCHS %d %d %d" +hd_geometry_guess(void *bs, int cyls, int heads, int secs, int trans) "bs %p CHS %d %d %d trans %d" + # hw/jazz-led.c jazz_led_read(uint64_t addr, uint8_t val) "read addr=0x%"PRIx64": 0x%x" jazz_led_write(uint64_t addr, uint8_t new) "write addr=0x%"PRIx64": 0x%x" From c06aaf018b1a39b62c1f6867794807dd5705b355 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:33 +0200 Subject: [PATCH 09/41] hd-geometry: Unnest conditional in hd_geometry_guess() Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/hd-geometry.c | 84 ++++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index f0dd02149d..db47846782 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -104,58 +104,58 @@ void hd_geometry_guess(BlockDriverState *bs, int cylinders, heads, secs; uint64_t nb_sectors; - /* if a geometry hint is available, use it */ bdrv_get_geometry(bs, &nb_sectors); bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs); translation = bdrv_get_translation_hint(bs); + if (cylinders != 0) { + /* already got a geometry hint: use it */ *pcyls = cylinders; *pheads = heads; *psecs = secs; - } else { - if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) { - if (heads > 16) { - /* if heads > 16, it means that a BIOS LBA - translation was active, so the default - hardware geometry is OK */ - lba_detected = 1; - goto default_geometry; - } else { - *pcyls = cylinders; - *pheads = heads; - *psecs = secs; - /* disable any translation to be in sync with - the logical geometry */ - if (translation == BIOS_ATA_TRANSLATION_AUTO) { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_NONE); - } - } - } else { - default_geometry: - /* if no geometry, use a standard physical disk geometry */ - cylinders = nb_sectors / (16 * 63); + return; + } - if (cylinders > 16383) { - cylinders = 16383; - } else if (cylinders < 2) { - cylinders = 2; - } - *pcyls = cylinders; - *pheads = 16; - *psecs = 63; - if ((lba_detected == 1) - && (translation == BIOS_ATA_TRANSLATION_AUTO)) { - if ((*pcyls * *pheads) <= 131072) { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_LARGE); - } else { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_LBA); - } + if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) { + /* no LCHS guess: use a standard physical disk geometry */ + default_geometry: + cylinders = nb_sectors / (16 * 63); + + if (cylinders > 16383) { + cylinders = 16383; + } else if (cylinders < 2) { + cylinders = 2; + } + *pcyls = cylinders; + *pheads = 16; + *psecs = 63; + if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) { + if ((*pcyls * *pheads) <= 131072) { + bdrv_set_translation_hint(bs, + BIOS_ATA_TRANSLATION_LARGE); + } else { + bdrv_set_translation_hint(bs, + BIOS_ATA_TRANSLATION_LBA); } } - bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); + } else if (heads > 16) { + /* LCHS guess with heads > 16 means that a BIOS LBA + translation was active, so a standard physical disk + geometry is OK */ + lba_detected = 1; + goto default_geometry; + } else { + /* LCHS guess with heads <= 16: use as physical geometry */ + *pcyls = cylinders; + *pheads = heads; + *psecs = secs; + /* disable any translation to be in sync with + the logical geometry */ + if (translation == BIOS_ATA_TRANSLATION_AUTO) { + bdrv_set_translation_hint(bs, + BIOS_ATA_TRANSLATION_NONE); + } } + bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation); } From 2fa5008ffd49e51540756adccf966a2fcde6e6c1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:34 +0200 Subject: [PATCH 10/41] hd-geometry: Factor out guess_chs_for_size() Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/hd-geometry.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index db47846782..1a58894cf0 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -97,14 +97,31 @@ static int guess_disk_lchs(BlockDriverState *bs, return -1; } +static void guess_chs_for_size(BlockDriverState *bs, + int *pcyls, int *pheads, int *psecs) +{ + uint64_t nb_sectors; + int cylinders; + + bdrv_get_geometry(bs, &nb_sectors); + + cylinders = nb_sectors / (16 * 63); + if (cylinders > 16383) { + cylinders = 16383; + } else if (cylinders < 2) { + cylinders = 2; + } + *pcyls = cylinders; + *pheads = 16; + *psecs = 63; +} + void hd_geometry_guess(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs) { int translation, lba_detected = 0; int cylinders, heads, secs; - uint64_t nb_sectors; - bdrv_get_geometry(bs, &nb_sectors); bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs); translation = bdrv_get_translation_hint(bs); @@ -119,16 +136,7 @@ void hd_geometry_guess(BlockDriverState *bs, if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) { /* no LCHS guess: use a standard physical disk geometry */ default_geometry: - cylinders = nb_sectors / (16 * 63); - - if (cylinders > 16383) { - cylinders = 16383; - } else if (cylinders < 2) { - cylinders = 2; - } - *pcyls = cylinders; - *pheads = 16; - *psecs = 63; + guess_chs_for_size(bs, pcyls, pheads, psecs); if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) { if ((*pcyls * *pheads) <= 131072) { bdrv_set_translation_hint(bs, From 82b11662be5b6e462ae843363b316779a9c88a61 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:35 +0200 Subject: [PATCH 11/41] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/hd-geometry.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index 1a58894cf0..fb849a3e12 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -119,8 +119,7 @@ static void guess_chs_for_size(BlockDriverState *bs, void hd_geometry_guess(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs) { - int translation, lba_detected = 0; - int cylinders, heads, secs; + int cylinders, heads, secs, translation; bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs); translation = bdrv_get_translation_hint(bs); @@ -135,23 +134,18 @@ void hd_geometry_guess(BlockDriverState *bs, if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) { /* no LCHS guess: use a standard physical disk geometry */ - default_geometry: guess_chs_for_size(bs, pcyls, pheads, psecs); - if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) { - if ((*pcyls * *pheads) <= 131072) { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_LARGE); - } else { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_LBA); - } - } } else if (heads > 16) { /* LCHS guess with heads > 16 means that a BIOS LBA translation was active, so a standard physical disk geometry is OK */ - lba_detected = 1; - goto default_geometry; + guess_chs_for_size(bs, pcyls, pheads, psecs); + if (translation == BIOS_ATA_TRANSLATION_AUTO) { + bdrv_set_translation_hint(bs, + *pcyls * *pheads <= 131072 + ? BIOS_ATA_TRANSLATION_LARGE + : BIOS_ATA_TRANSLATION_LBA); + } } else { /* LCHS guess with heads <= 16: use as physical geometry */ *pcyls = cylinders; From dc28c0cd30d7b122500b17eedc7e070624fd7c86 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:36 +0200 Subject: [PATCH 12/41] hd-geometry: Clean up confusing use of prior translation hint When hd_geometry_guess() picks a geometry, it also picks the appropriate translation, but only when the prior translation hint is BIOS_ATA_TRANSLATION_AUTO. Looks wrong, because such a prior translation would be passed to the BIOS whether it's suitable for the geometry or not. Fortunately, that can't happen. There are just two ways for the translation hint to get set to something other than BIOS_ATA_TRANSLATION_AUTO: drive_init() on behalf of -drive trans=..., and hd_geometry_guess(). Both set it only when they also set a valid geometry hint, i.e. one with a non-zero number of cylinders. Since hd_geometry_guess() returns right away when it finds a valid geometry hint, translation can only be BIOS_ATA_TRANSLATION_AUTO in the remainder of the function. Assert this, and simplify accordingly. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/hd-geometry.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index fb849a3e12..241aed9352 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -132,6 +132,8 @@ void hd_geometry_guess(BlockDriverState *bs, return; } + assert(translation == BIOS_ATA_TRANSLATION_AUTO); + if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) { /* no LCHS guess: use a standard physical disk geometry */ guess_chs_for_size(bs, pcyls, pheads, psecs); @@ -140,12 +142,10 @@ void hd_geometry_guess(BlockDriverState *bs, translation was active, so a standard physical disk geometry is OK */ guess_chs_for_size(bs, pcyls, pheads, psecs); - if (translation == BIOS_ATA_TRANSLATION_AUTO) { - bdrv_set_translation_hint(bs, - *pcyls * *pheads <= 131072 - ? BIOS_ATA_TRANSLATION_LARGE - : BIOS_ATA_TRANSLATION_LBA); - } + bdrv_set_translation_hint(bs, + *pcyls * *pheads <= 131072 + ? BIOS_ATA_TRANSLATION_LARGE + : BIOS_ATA_TRANSLATION_LBA); } else { /* LCHS guess with heads <= 16: use as physical geometry */ *pcyls = cylinders; @@ -153,10 +153,7 @@ void hd_geometry_guess(BlockDriverState *bs, *psecs = secs; /* disable any translation to be in sync with the logical geometry */ - if (translation == BIOS_ATA_TRANSLATION_AUTO) { - bdrv_set_translation_hint(bs, - BIOS_ATA_TRANSLATION_NONE); - } + bdrv_set_translation_hint(bs, BIOS_ATA_TRANSLATION_NONE); } bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation); From e2f3dc2b6a205cf969ba5d1307293db17fd9621f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:37 +0200 Subject: [PATCH 13/41] hd-geometry: Cut out block layer translation middleman hd_geometry_guess() picks geometry and translation. Callers can get the geometry directly, via parameters, but for translation they need to go through the block layer. Add a parameter for translation, so it can optionally be gotten just like geometry. In preparation of purging translation from the block layer, which will happen later in this series. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/block-common.h | 3 ++- hw/hd-geometry.c | 20 ++++++++++++++------ hw/ide/core.c | 2 +- hw/scsi-disk.c | 4 ++-- hw/virtio-blk.c | 2 +- 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/hw/block-common.h b/hw/block-common.h index 3a4d4c6292..bba817a5ba 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -16,6 +16,7 @@ /* Hard disk geometry */ void hd_geometry_guess(BlockDriverState *bs, - int *pcyls, int *pheads, int *psecs); + int *pcyls, int *pheads, int *psecs, + int *ptrans); #endif diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index 241aed9352..4d746b7155 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -117,7 +117,8 @@ static void guess_chs_for_size(BlockDriverState *bs, } void hd_geometry_guess(BlockDriverState *bs, - int *pcyls, int *pheads, int *psecs) + int *pcyls, int *pheads, int *psecs, + int *ptrans) { int cylinders, heads, secs, translation; @@ -129,6 +130,9 @@ void hd_geometry_guess(BlockDriverState *bs, *pcyls = cylinders; *pheads = heads; *psecs = secs; + if (ptrans) { + *ptrans = translation; + } return; } @@ -142,10 +146,10 @@ void hd_geometry_guess(BlockDriverState *bs, translation was active, so a standard physical disk geometry is OK */ guess_chs_for_size(bs, pcyls, pheads, psecs); - bdrv_set_translation_hint(bs, - *pcyls * *pheads <= 131072 - ? BIOS_ATA_TRANSLATION_LARGE - : BIOS_ATA_TRANSLATION_LBA); + translation = *pcyls * *pheads <= 131072 + ? BIOS_ATA_TRANSLATION_LARGE + : BIOS_ATA_TRANSLATION_LBA; + bdrv_set_translation_hint(bs, translation); } else { /* LCHS guess with heads <= 16: use as physical geometry */ *pcyls = cylinders; @@ -153,7 +157,11 @@ void hd_geometry_guess(BlockDriverState *bs, *psecs = secs; /* disable any translation to be in sync with the logical geometry */ - bdrv_set_translation_hint(bs, BIOS_ATA_TRANSLATION_NONE); + translation = BIOS_ATA_TRANSLATION_NONE; + bdrv_set_translation_hint(bs, translation); + } + if (ptrans) { + *ptrans = translation; } bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation); diff --git a/hw/ide/core.c b/hw/ide/core.c index 0d1bf10358..28f04add5d 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1934,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, s->drive_kind = kind; bdrv_get_geometry(bs, &nb_sectors); - hd_geometry_guess(bs, &cylinders, &heads, &secs); + hd_geometry_guess(bs, &cylinders, &heads, &secs, NULL); if (cylinders < 1 || cylinders > 16383) { error_report("cyls must be between 1 and 16383"); return -1; diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 5339c2ef9d..fc077f5951 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -990,7 +990,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, break; } /* if a geometry hint is available, use it */ - hd_geometry_guess(bdrv, &cylinders, &heads, &secs); + hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL); p[2] = (cylinders >> 16) & 0xff; p[3] = (cylinders >> 8) & 0xff; p[4] = cylinders & 0xff; @@ -1024,7 +1024,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, p[2] = 5000 >> 8; p[3] = 5000 & 0xff; /* if a geometry hint is available, use it */ - hd_geometry_guess(bdrv, &cylinders, &heads, &secs); + hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL); p[4] = heads & 0xff; p[5] = secs & 0xff; p[6] = s->qdev.blocksize >> 8; diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index f16c5ceb2a..d2709a741e 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -623,7 +623,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) s->blk = blk; s->rq = NULL; s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; - hd_geometry_guess(s->bs, &cylinders, &heads, &secs); + hd_geometry_guess(s->bs, &cylinders, &heads, &secs, NULL); s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output); From 9139046c16688615023f35668660f6d3947a05d6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:38 +0200 Subject: [PATCH 14/41] ide pc: Cut out the block layer geometry middleman PC BIOS setup needs IDE geometry information. Get it directly from the device model rather than through the block layer. In preparation of purging geometry from the block layer, which will happen later in this series. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/ide.h | 4 +++- hw/ide/core.c | 2 +- hw/ide/internal.h | 2 +- hw/ide/qdev.c | 21 +++++++++++++++---- hw/pc.c | 51 +++++++++++++++++++++++++---------------------- 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/hw/ide.h b/hw/ide.h index 0b18c9016b..2db4079f68 100644 --- a/hw/ide.h +++ b/hw/ide.h @@ -29,7 +29,9 @@ void mmio_ide_init (target_phys_addr_t membase, target_phys_addr_t membase2, qemu_irq irq, int shift, DriveInfo *hd0, DriveInfo *hd1); -void ide_get_bs(BlockDriverState *bs[], BusState *qbus); +int ide_get_geometry(BusState *bus, int unit, + int16_t *cyls, int8_t *heads, int8_t *secs); +int ide_get_bios_chs_trans(BusState *bus, int unit); /* ide/core.c */ void ide_drive_get(DriveInfo **hd, int max_bus); diff --git a/hw/ide/core.c b/hw/ide/core.c index 28f04add5d..7f5ad0788b 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1934,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, s->drive_kind = kind; bdrv_get_geometry(bs, &nb_sectors); - hd_geometry_guess(bs, &cylinders, &heads, &secs, NULL); + hd_geometry_guess(bs, &cylinders, &heads, &secs, &s->chs_trans); if (cylinders < 1 || cylinders > 16383) { error_report("cyls must be between 1 and 16383"); return -1; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 1a02f57bf5..56c718ef6b 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -344,7 +344,7 @@ struct IDEState { uint8_t unit; /* ide config */ IDEDriveKind drive_kind; - int cylinders, heads, sectors; + int cylinders, heads, sectors, chs_trans; int64_t nb_sectors; int mult_sectors; int identify_set; diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index c122395401..87e0b75ae4 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -111,11 +111,24 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive) return DO_UPCAST(IDEDevice, qdev, dev); } -void ide_get_bs(BlockDriverState *bs[], BusState *qbus) +int ide_get_geometry(BusState *bus, int unit, + int16_t *cyls, int8_t *heads, int8_t *secs) { - IDEBus *bus = DO_UPCAST(IDEBus, qbus, qbus); - bs[0] = bus->master ? bus->master->conf.bs : NULL; - bs[1] = bus->slave ? bus->slave->conf.bs : NULL; + IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit]; + + if (!s->bs) { + return -1; + } + + *cyls = s->cylinders; + *heads = s->heads; + *secs = s->sectors; + return 0; +} + +int ide_get_bios_chs_trans(BusState *bus, int unit) +{ + return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans; } /* --------------------------------- */ diff --git a/hw/pc.c b/hw/pc.c index 91cf77dafd..89a0c661db 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -216,11 +216,9 @@ static int cmos_get_fd_drive_type(FDriveType fd0) return val; } -static void cmos_init_hd(int type_ofs, int info_ofs, BlockDriverState *hd, - ISADevice *s) +static void cmos_init_hd(ISADevice *s, int type_ofs, int info_ofs, + int16_t cylinders, int8_t heads, int8_t sectors) { - int cylinders, heads, sectors; - bdrv_get_geometry_hint(hd, &cylinders, &heads, §ors); rtc_set_memory(s, type_ofs, 47); rtc_set_memory(s, info_ofs, cylinders); rtc_set_memory(s, info_ofs + 1, cylinders >> 8); @@ -281,37 +279,42 @@ static int pc_boot_set(void *opaque, const char *boot_device) typedef struct pc_cmos_init_late_arg { ISADevice *rtc_state; - BusState *idebus0, *idebus1; + BusState *idebus[2]; } pc_cmos_init_late_arg; static void pc_cmos_init_late(void *opaque) { pc_cmos_init_late_arg *arg = opaque; ISADevice *s = arg->rtc_state; + int16_t cylinders; + int8_t heads, sectors; int val; - BlockDriverState *hd_table[4]; int i; - ide_get_bs(hd_table, arg->idebus0); - ide_get_bs(hd_table + 2, arg->idebus1); - - rtc_set_memory(s, 0x12, (hd_table[0] ? 0xf0 : 0) | (hd_table[1] ? 0x0f : 0)); - if (hd_table[0]) - cmos_init_hd(0x19, 0x1b, hd_table[0], s); - if (hd_table[1]) - cmos_init_hd(0x1a, 0x24, hd_table[1], s); + val = 0; + if (ide_get_geometry(arg->idebus[0], 0, + &cylinders, &heads, §ors) >= 0) { + cmos_init_hd(s, 0x19, 0x1b, cylinders, heads, sectors); + val |= 0xf0; + } + if (ide_get_geometry(arg->idebus[0], 1, + &cylinders, &heads, §ors) >= 0) { + cmos_init_hd(s, 0x1a, 0x24, cylinders, heads, sectors); + val |= 0x0f; + } + rtc_set_memory(s, 0x12, val); val = 0; for (i = 0; i < 4; i++) { - if (hd_table[i]) { - int cylinders, heads, sectors, translation; - /* NOTE: bdrv_get_geometry_hint() returns the physical - geometry. It is always such that: 1 <= sects <= 63, 1 - <= heads <= 16, 1 <= cylinders <= 16383. The BIOS - geometry can be different if a translation is done. */ - translation = bdrv_get_translation_hint(hd_table[i]); + /* NOTE: ide_get_geometry() returns the physical + geometry. It is always such that: 1 <= sects <= 63, 1 + <= heads <= 16, 1 <= cylinders <= 16383. The BIOS + geometry can be different if a translation is done. */ + if (ide_get_geometry(arg->idebus[i / 2], i % 2, + &cylinders, &heads, §ors) >= 0) { + int translation = ide_get_bios_chs_trans(arg->idebus[i / 2], + i % 2); if (translation == BIOS_ATA_TRANSLATION_AUTO) { - bdrv_get_geometry_hint(hd_table[i], &cylinders, &heads, §ors); if (cylinders <= 1024 && heads <= 16 && sectors <= 63) { /* No translation. */ translation = 0; @@ -411,8 +414,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, /* hard drives */ arg.rtc_state = s; - arg.idebus0 = idebus0; - arg.idebus1 = idebus1; + arg.idebus[0] = idebus0; + arg.idebus[1] = idebus1; qemu_register_reset(pc_cmos_init_late, &arg); } From 317bb412293672ff930884d67235e970dad88566 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:39 +0200 Subject: [PATCH 15/41] blockdev: Save geometry in DriveInfo In preparation of purging it from the block layer, which will happen later in this series. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- blockdev.c | 4 ++++ blockdev.h | 1 + 2 files changed, 5 insertions(+) diff --git a/blockdev.c b/blockdev.c index a85a429aef..161985b1e5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -530,6 +530,10 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) dinfo->type = type; dinfo->bus = bus_id; dinfo->unit = unit_id; + dinfo->cyls = cyls; + dinfo->heads = heads; + dinfo->secs = secs; + dinfo->trans = translation; dinfo->opts = opts; dinfo->refcount = 1; if (serial) { diff --git a/blockdev.h b/blockdev.h index 260e16b3c6..9c29948128 100644 --- a/blockdev.h +++ b/blockdev.h @@ -35,6 +35,7 @@ struct DriveInfo { int unit; int auto_del; /* see blockdev_mark_auto_del() */ int media_cd; + int cyls, heads, secs, trans; QemuOpts *opts; char serial[BLOCK_SERIAL_STRLEN + 1]; QTAILQ_ENTRY(DriveInfo) next; From 8a4bc5aafa7286e03bbced8abdb43aa6abdf95ea Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:40 +0200 Subject: [PATCH 16/41] qdev: Introduce block geometry properties Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/block.h b/block.h index 993894efe4..1cd8a01c75 100644 --- a/block.h +++ b/block.h @@ -426,6 +426,8 @@ typedef struct BlockConf { uint32_t opt_io_size; int32_t bootindex; uint32_t discard_granularity; + /* geometry, not all devices use this */ + uint32_t cyls, heads, secs; } BlockConf; static inline unsigned int get_physical_block_exp(BlockConf *conf) @@ -453,5 +455,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) DEFINE_PROP_UINT32("discard_granularity", _state, \ _conf.discard_granularity, 0) -#endif +#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ + DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ + DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ + DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0) +#endif From 1f24d7b47e1f18b5e7f0f050f915a42e9aa645db Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:41 +0200 Subject: [PATCH 17/41] hd-geometry: Switch to uint32_t to match BlockConf Best to use the same type, to avoid unwanted truncation or sign extension. BlockConf can't use plain int for cyls, heads and secs, because integer properties require an exact width. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/block-common.h | 2 +- hw/hd-geometry.c | 4 ++-- hw/ide/core.c | 2 +- hw/scsi-disk.c | 2 +- hw/virtio-blk.c | 2 +- trace-events | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/block-common.h b/hw/block-common.h index bba817a5ba..2f651867d2 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -16,7 +16,7 @@ /* Hard disk geometry */ void hd_geometry_guess(BlockDriverState *bs, - int *pcyls, int *pheads, int *psecs, + uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs, int *ptrans); #endif diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index 4d746b7155..7626cbbe9c 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -98,7 +98,7 @@ static int guess_disk_lchs(BlockDriverState *bs, } static void guess_chs_for_size(BlockDriverState *bs, - int *pcyls, int *pheads, int *psecs) + uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs) { uint64_t nb_sectors; int cylinders; @@ -117,7 +117,7 @@ static void guess_chs_for_size(BlockDriverState *bs, } void hd_geometry_guess(BlockDriverState *bs, - int *pcyls, int *pheads, int *psecs, + uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs, int *ptrans) { int cylinders, heads, secs, translation; diff --git a/hw/ide/core.c b/hw/ide/core.c index 7f5ad0788b..f1966e37f1 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1927,7 +1927,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, const char *version, const char *serial, const char *model, uint64_t wwn) { - int cylinders, heads, secs; + uint32_t cylinders, heads, secs; uint64_t nb_sectors; s->bs = bs; diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index fc077f5951..c881acf011 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -968,7 +968,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, }; BlockDriverState *bdrv = s->qdev.conf.bs; - int cylinders, heads, secs; + uint32_t cylinders, heads, secs; uint8_t *p = *p_outbuf; if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) { diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index d2709a741e..4344e28380 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -590,7 +590,7 @@ static const BlockDevOps virtio_block_ops = { VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) { VirtIOBlock *s; - int cylinders, heads, secs; + uint32_t cylinders, heads, secs; static int virtio_blk_id; DriveInfo *dinfo; diff --git a/trace-events b/trace-events index 5f27f1a8d1..8b1fb24c57 100644 --- a/trace-events +++ b/trace-events @@ -143,7 +143,7 @@ ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= %02x # hw/hd-geometry.c hd_geometry_lchs_guess(void *bs, int cyls, int heads, int secs) "bs %p LCHS %d %d %d" -hd_geometry_guess(void *bs, int cyls, int heads, int secs, int trans) "bs %p CHS %d %d %d trans %d" +hd_geometry_guess(void *bs, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "bs %p CHS %u %u %u trans %d" # hw/jazz-led.c jazz_led_read(uint64_t addr, uint8_t val) "read addr=0x%"PRIx64": 0x%x" From d252df489879ca3b128e080409b89305491d04cf Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:42 +0200 Subject: [PATCH 18/41] scsi-hd: qdev properties for disk geometry Geometry needs to be qdev properties, because it belongs to the disk's guest part. Maintain backward compatibility exactly like for serial: fall back to DriveInfo's geometry, set with -drive cyls=... Do this only for scsi-hd. scsi-disk is legacy. scsi-cd doesn't have a geometry. scsi-block should get geometry from the host disk. Bonus: info qtree now shows the geometry. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/scsi-disk.c | 69 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index c881acf011..0a182f987e 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -966,9 +966,6 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, [MODE_PAGE_AUDIO_CTL] = (1 << TYPE_ROM), [MODE_PAGE_CAPABILITIES] = (1 << TYPE_ROM), }; - - BlockDriverState *bdrv = s->qdev.conf.bs; - uint32_t cylinders, heads, secs; uint8_t *p = *p_outbuf; if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) { @@ -990,19 +987,18 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, break; } /* if a geometry hint is available, use it */ - hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL); - p[2] = (cylinders >> 16) & 0xff; - p[3] = (cylinders >> 8) & 0xff; - p[4] = cylinders & 0xff; - p[5] = heads & 0xff; + p[2] = (s->qdev.conf.cyls >> 16) & 0xff; + p[3] = (s->qdev.conf.cyls >> 8) & 0xff; + p[4] = s->qdev.conf.cyls & 0xff; + p[5] = s->qdev.conf.heads & 0xff; /* Write precomp start cylinder, disabled */ - p[6] = (cylinders >> 16) & 0xff; - p[7] = (cylinders >> 8) & 0xff; - p[8] = cylinders & 0xff; + p[6] = (s->qdev.conf.cyls >> 16) & 0xff; + p[7] = (s->qdev.conf.cyls >> 8) & 0xff; + p[8] = s->qdev.conf.cyls & 0xff; /* Reduced current start cylinder, disabled */ - p[9] = (cylinders >> 16) & 0xff; - p[10] = (cylinders >> 8) & 0xff; - p[11] = cylinders & 0xff; + p[9] = (s->qdev.conf.cyls >> 16) & 0xff; + p[10] = (s->qdev.conf.cyls >> 8) & 0xff; + p[11] = s->qdev.conf.cyls & 0xff; /* Device step rate [ns], 200ns */ p[12] = 0; p[13] = 200; @@ -1024,18 +1020,17 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf, p[2] = 5000 >> 8; p[3] = 5000 & 0xff; /* if a geometry hint is available, use it */ - hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL); - p[4] = heads & 0xff; - p[5] = secs & 0xff; + p[4] = s->qdev.conf.heads & 0xff; + p[5] = s->qdev.conf.secs & 0xff; p[6] = s->qdev.blocksize >> 8; - p[8] = (cylinders >> 8) & 0xff; - p[9] = cylinders & 0xff; + p[8] = (s->qdev.conf.cyls >> 8) & 0xff; + p[9] = s->qdev.conf.cyls & 0xff; /* Write precomp start cylinder, disabled */ - p[10] = (cylinders >> 8) & 0xff; - p[11] = cylinders & 0xff; + p[10] = (s->qdev.conf.cyls >> 8) & 0xff; + p[11] = s->qdev.conf.cyls & 0xff; /* Reduced current start cylinder, disabled */ - p[12] = (cylinders >> 8) & 0xff; - p[13] = cylinders & 0xff; + p[12] = (s->qdev.conf.cyls >> 8) & 0xff; + p[13] = s->qdev.conf.cyls & 0xff; /* Device step rate [100us], 100us */ p[14] = 0; p[15] = 1; @@ -1755,6 +1750,33 @@ static int scsi_initfn(SCSIDevice *dev) return -1; } + if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { + /* try to fall back to value set with legacy -drive cyls=... */ + dinfo = drive_get_by_blockdev(s->qdev.conf.bs); + dev->conf.cyls = dinfo->cyls; + dev->conf.heads = dinfo->heads; + dev->conf.secs = dinfo->secs; + } + if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { + hd_geometry_guess(s->qdev.conf.bs, + &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs, + NULL); + } + if (dev->conf.cyls || dev->conf.heads || dev->conf.secs) { + if (dev->conf.cyls < 1 || dev->conf.cyls > 65535) { + error_report("cyls must be between 1 and 65535"); + return -1; + } + if (dev->conf.heads < 1 || dev->conf.heads > 255) { + error_report("heads must be between 1 and 255"); + return -1; + } + if (dev->conf.secs < 1 || dev->conf.secs > 255) { + error_report("secs must be between 1 and 255"); + return -1; + } + } + if (!s->serial) { /* try to fall back to value set with legacy -drive serial=... */ dinfo = drive_get_by_blockdev(s->qdev.conf.bs); @@ -1975,6 +1997,7 @@ static Property scsi_hd_properties[] = { DEFINE_PROP_BIT("dpofua", SCSIDiskState, features, SCSI_DISK_F_DPOFUA, false), DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0), + DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf), DEFINE_PROP_END_OF_LIST(), }; From e63e7fde24a3e88f1a4992d8f47b7a44ddcf14ff Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:43 +0200 Subject: [PATCH 19/41] virtio-blk: qdev properties for disk geometry Geometry needs to be qdev properties, because it belongs to the disk's guest part. Maintain backward compatibility exactly like for serial: fall back to DriveInfo's geometry, set with -drive cyls=... Bonus: info qtree now shows the geometry. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/s390-virtio-bus.c | 1 + hw/virtio-blk.c | 41 ++++++++++++++++++++++++++++++++--------- hw/virtio-pci.c | 1 + 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index 4d49b96f94..a245684692 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -402,6 +402,7 @@ static TypeInfo s390_virtio_net = { static Property s390_virtio_blk_properties[] = { DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf), + DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf), DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial), #ifdef __linux__ DEFINE_PROP_BIT("scsi", VirtIOS390Device, blk.scsi, 0, true), diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index 4344e28380..38859048b9 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -479,19 +479,17 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) VirtIOBlock *s = to_virtio_blk(vdev); struct virtio_blk_config blkcfg; uint64_t capacity; - int cylinders, heads, secs; int blk_size = s->conf->logical_block_size; bdrv_get_geometry(s->bs, &capacity); - bdrv_get_geometry_hint(s->bs, &cylinders, &heads, &secs); memset(&blkcfg, 0, sizeof(blkcfg)); stq_raw(&blkcfg.capacity, capacity); stl_raw(&blkcfg.seg_max, 128 - 2); - stw_raw(&blkcfg.cylinders, cylinders); + stw_raw(&blkcfg.cylinders, s->conf->cyls); stl_raw(&blkcfg.blk_size, blk_size); stw_raw(&blkcfg.min_io_size, s->conf->min_io_size / blk_size); stw_raw(&blkcfg.opt_io_size, s->conf->opt_io_size / blk_size); - blkcfg.heads = heads; + blkcfg.heads = s->conf->heads; /* * We must ensure that the block device capacity is a multiple of * the logical block size. If that is not the case, lets use @@ -503,10 +501,10 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) * divided by 512 - instead it is the amount of blk_size blocks * per track (cylinder). */ - if (bdrv_getlength(s->bs) / heads / secs % blk_size) { - blkcfg.sectors = secs & ~s->sector_mask; + if (bdrv_getlength(s->bs) / s->conf->heads / s->conf->secs % blk_size) { + blkcfg.sectors = s->conf->secs & ~s->sector_mask; } else { - blkcfg.sectors = secs; + blkcfg.sectors = s->conf->secs; } blkcfg.size_max = 0; blkcfg.physical_block_exp = get_physical_block_exp(s->conf); @@ -590,7 +588,6 @@ static const BlockDevOps virtio_block_ops = { VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) { VirtIOBlock *s; - uint32_t cylinders, heads, secs; static int virtio_blk_id; DriveInfo *dinfo; @@ -623,7 +620,33 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) s->blk = blk; s->rq = NULL; s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; - hd_geometry_guess(s->bs, &cylinders, &heads, &secs, NULL); + + if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) { + /* try to fall back to value set with legacy -drive cyls=... */ + dinfo = drive_get_by_blockdev(blk->conf.bs); + blk->conf.cyls = dinfo->cyls; + blk->conf.heads = dinfo->heads; + blk->conf.secs = dinfo->secs; + } + if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) { + hd_geometry_guess(s->bs, + &blk->conf.cyls, &blk->conf.heads, &blk->conf.secs, + NULL); + } + if (blk->conf.cyls || blk->conf.heads || blk->conf.secs) { + if (blk->conf.cyls < 1 || blk->conf.cyls > 65535) { + error_report("cyls must be between 1 and 65535"); + return NULL; + } + if (blk->conf.heads < 1 || blk->conf.heads > 255) { + error_report("heads must be between 1 and 255"); + return NULL; + } + if (blk->conf.secs < 1 || blk->conf.secs > 255) { + error_report("secs must be between 1 and 255"); + return NULL; + } + } s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output); diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 9342eed070..557d1d3b13 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -936,6 +936,7 @@ static int virtio_balloon_exit_pci(PCIDevice *pci_dev) static Property virtio_blk_properties[] = { DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf), + DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf), DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial), #ifdef __linux__ DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true), From ba801960db1c08035f7e1772bd482aa80d909a35 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:44 +0200 Subject: [PATCH 20/41] ide: qdev properties for disk geometry Geometry needs to be qdev properties, because it belongs to the disk's guest part. Maintain backward compatibility exactly like for serial: fall back to DriveInfo's geometry, set with -drive cyls=... Do this only for ide-hd. ide-drive is legacy. ide-cd doesn't have a geometry. Bonus: info qtree now shows the geometry. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/ide/core.c | 19 ++++++++++++++----- hw/ide/internal.h | 4 +++- hw/ide/qdev.c | 22 +++++++++++++++++++++- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index f1966e37f1..bf1ce8968a 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1925,16 +1925,16 @@ static const BlockDevOps ide_cd_block_ops = { int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, const char *version, const char *serial, const char *model, - uint64_t wwn) + uint64_t wwn, + uint32_t cylinders, uint32_t heads, uint32_t secs, + int chs_trans) { - uint32_t cylinders, heads, secs; uint64_t nb_sectors; s->bs = bs; s->drive_kind = kind; bdrv_get_geometry(bs, &nb_sectors); - hd_geometry_guess(bs, &cylinders, &heads, &secs, &s->chs_trans); if (cylinders < 1 || cylinders > 16383) { error_report("cyls must be between 1 and 16383"); return -1; @@ -1950,6 +1950,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, s->cylinders = cylinders; s->heads = heads; s->sectors = secs; + s->chs_trans = chs_trans; s->nb_sectors = nb_sectors; s->wwn = wwn; /* The SMART values should be preserved across power cycles @@ -2076,17 +2077,25 @@ void ide_init2(IDEBus *bus, qemu_irq irq) void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1, qemu_irq irq) { - int i; + int i, trans; DriveInfo *dinfo; + uint32_t cyls, heads, secs; for(i = 0; i < 2; i++) { dinfo = i == 0 ? hd0 : hd1; ide_init1(bus, i); if (dinfo) { + cyls = dinfo->cyls; + heads = dinfo->heads; + secs = dinfo->secs; + trans = dinfo->trans; + if (!cyls && !heads && !secs) { + hd_geometry_guess(dinfo->bdrv, &cyls, &heads, &secs, &trans); + } if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, dinfo->media_cd ? IDE_CD : IDE_HD, NULL, *dinfo->serial ? dinfo->serial : NULL, - NULL, 0) < 0) { + NULL, 0, cyls, heads, secs, trans) < 0) { error_report("Can't set up IDE drive %s", dinfo->id); exit(1); } diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 56c718ef6b..685e976c51 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -545,7 +545,9 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr); int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, const char *version, const char *serial, const char *model, - uint64_t wwn); + uint64_t wwn, + uint32_t cylinders, uint32_t heads, uint32_t secs, + int chs_trans); void ide_init2(IDEBus *bus, qemu_irq irq); void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1, qemu_irq irq); diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 87e0b75ae4..3e297dc88a 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -21,6 +21,7 @@ #include "qemu-error.h" #include #include "blockdev.h" +#include "hw/block-common.h" #include "sysemu.h" /* --------------------------------- */ @@ -143,6 +144,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) IDEState *s = bus->ifs + dev->unit; const char *serial; DriveInfo *dinfo; + int trans; if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) { error_report("discard_granularity must be 512 for ide"); @@ -158,8 +160,25 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) } } + trans = BIOS_ATA_TRANSLATION_AUTO; + if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { + /* try to fall back to value set with legacy -drive cyls=... */ + dinfo = drive_get_by_blockdev(dev->conf.bs); + dev->conf.cyls = dinfo->cyls; + dev->conf.heads = dinfo->heads; + dev->conf.secs = dinfo->secs; + trans = dinfo->trans; + } + if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { + hd_geometry_guess(dev->conf.bs, + &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs, + &trans); + } + if (ide_init_drive(s, dev->conf.bs, kind, - dev->version, serial, dev->model, dev->wwn) < 0) { + dev->version, serial, dev->model, dev->wwn, + dev->conf.cyls, dev->conf.heads, dev->conf.secs, + trans) < 0) { return -1; } @@ -202,6 +221,7 @@ static int ide_drive_initfn(IDEDevice *dev) static Property ide_hd_properties[] = { DEFINE_IDE_DEV_PROPERTIES(), + DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf), DEFINE_PROP_END_OF_LIST(), }; From eb0e4b9804ce634386c4de4b2708af0ad01edaa1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:45 +0200 Subject: [PATCH 21/41] qtest: Cover qdev properties for disk geometry Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- tests/hd-geo-test.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c index cc447a26bd..a47b94507f 100644 --- a/tests/hd-geo-test.c +++ b/tests/hd-geo-test.c @@ -321,13 +321,15 @@ static void test_ide_drive_user(const char *dev, bool trans) const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans }; argc = setup_common(argv, ARRAY_SIZE(argv)); - opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s", + opts = g_strdup_printf("%s,cyls=%d,heads=%d,secs=%d%s", + dev && !trans ? dev : "", expected_chst.cyls, expected_chst.heads, expected_chst.secs, trans ? ",trans=lba" : ""); cur_ide[0] = &expected_chst; argc = setup_ide(argc, argv, ARRAY_SIZE(argv), - 0, dev, backend_small, mbr_chs, opts); + 0, dev && !trans ? opts : NULL, backend_small, mbr_chs, + dev && !trans ? "" : opts); g_free(opts); qtest_start(g_strjoinv(" ", argv)); test_cmos(); From d4d34b0d3f5af5c8e09980da0de2eebe9a27dc71 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:46 +0200 Subject: [PATCH 22/41] qdev: Collect private helpers in one place Just code motion, with one long line wrapped to keep checkpatch.pl happy. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/qdev-properties.c | 144 +++++++++++++++++++++---------------------- 1 file changed, 72 insertions(+), 72 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 0b894620c9..002c7f9726 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -10,6 +10,78 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) return ptr; } +static void get_pointer(Object *obj, Visitor *v, Property *prop, + const char *(*print)(void *ptr), + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + void **ptr = qdev_get_prop_ptr(dev, prop); + char *p; + + p = (char *) (*ptr ? print(*ptr) : ""); + visit_type_str(v, &p, name, errp); +} + +static void set_pointer(Object *obj, Visitor *v, Property *prop, + int (*parse)(DeviceState *dev, const char *str, + void **ptr), + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Error *local_err = NULL; + void **ptr = qdev_get_prop_ptr(dev, prop); + char *str; + int ret; + + if (dev->state != DEV_STATE_CREATED) { + error_set(errp, QERR_PERMISSION_DENIED); + return; + } + + visit_type_str(v, &str, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + if (!*str) { + g_free(str); + *ptr = NULL; + return; + } + ret = parse(dev, str, ptr); + error_set_from_qdev_prop_error(errp, ret, dev, prop, str); + g_free(str); +} + +static void get_enum(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + int *ptr = qdev_get_prop_ptr(dev, prop); + + visit_type_enum(v, ptr, prop->info->enum_table, + prop->info->name, prop->name, errp); +} + +static void set_enum(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + int *ptr = qdev_get_prop_ptr(dev, prop); + + if (dev->state != DEV_STATE_CREATED) { + error_set(errp, QERR_PERMISSION_DENIED); + return; + } + + visit_type_enum(v, ptr, prop->info->enum_table, + prop->info->name, prop->name, errp); +} + +/* Bit */ + static uint32_t qdev_get_prop_mask(Property *prop) { assert(prop->info == &qdev_prop_bit); @@ -26,8 +98,6 @@ static void bit_prop_set(DeviceState *dev, Property *props, bool val) *p &= ~mask; } -/* Bit */ - static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len) { uint32_t *p = qdev_get_prop_ptr(dev, prop); @@ -435,48 +505,6 @@ static const char *print_drive(void *ptr) return bdrv_get_device_name(ptr); } -static void get_pointer(Object *obj, Visitor *v, Property *prop, - const char *(*print)(void *ptr), - const char *name, Error **errp) -{ - DeviceState *dev = DEVICE(obj); - void **ptr = qdev_get_prop_ptr(dev, prop); - char *p; - - p = (char *) (*ptr ? print(*ptr) : ""); - visit_type_str(v, &p, name, errp); -} - -static void set_pointer(Object *obj, Visitor *v, Property *prop, - int (*parse)(DeviceState *dev, const char *str, void **ptr), - const char *name, Error **errp) -{ - DeviceState *dev = DEVICE(obj); - Error *local_err = NULL; - void **ptr = qdev_get_prop_ptr(dev, prop); - char *str; - int ret; - - if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); - return; - } - - visit_type_str(v, &str, name, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - if (!*str) { - g_free(str); - *ptr = NULL; - return; - } - ret = parse(dev, str, ptr); - error_set_from_qdev_prop_error(errp, ret, dev, prop, str); - g_free(str); -} - static void get_drive(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { @@ -735,7 +763,6 @@ PropertyInfo qdev_prop_macaddr = { .set = set_mac, }; - /* --- lost tick policy --- */ static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = { @@ -748,33 +775,6 @@ static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = { QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int)); -static void get_enum(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) -{ - DeviceState *dev = DEVICE(obj); - Property *prop = opaque; - int *ptr = qdev_get_prop_ptr(dev, prop); - - visit_type_enum(v, ptr, prop->info->enum_table, - prop->info->name, prop->name, errp); -} - -static void set_enum(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) -{ - DeviceState *dev = DEVICE(obj); - Property *prop = opaque; - int *ptr = qdev_get_prop_ptr(dev, prop); - - if (dev->state != DEV_STATE_CREATED) { - error_set(errp, QERR_PERMISSION_DENIED); - return; - } - - visit_type_enum(v, ptr, prop->info->enum_table, - prop->info->name, prop->name, errp); -} - PropertyInfo qdev_prop_losttickpolicy = { .name = "LostTickPolicy", .enum_table = lost_tick_policy_table, From 8cd41745fbe4ebbc7adff247cf2955765f7eb5e2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:47 +0200 Subject: [PATCH 23/41] qdev: New property type chs-translation Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/qdev-properties.c | 15 +++++++++++++++ hw/qdev.h | 3 +++ 2 files changed, 18 insertions(+) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 002c7f9726..0b18f8c330 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -782,6 +782,21 @@ PropertyInfo qdev_prop_losttickpolicy = { .set = set_enum, }; +/* --- BIOS CHS translation */ + +static const char *bios_chs_trans_table[] = { + [BIOS_ATA_TRANSLATION_AUTO] = "auto", + [BIOS_ATA_TRANSLATION_NONE] = "none", + [BIOS_ATA_TRANSLATION_LBA] = "lba", +}; + +PropertyInfo qdev_prop_bios_chs_trans = { + .name = "bios-chs-trans", + .enum_table = bios_chs_trans_table, + .get = get_enum, + .set = set_enum, +}; + /* --- pci address --- */ /* diff --git a/hw/qdev.h b/hw/qdev.h index f4683dc771..9be35d47e5 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -232,6 +232,7 @@ extern PropertyInfo qdev_prop_chr; extern PropertyInfo qdev_prop_ptr; extern PropertyInfo qdev_prop_macaddr; extern PropertyInfo qdev_prop_losttickpolicy; +extern PropertyInfo qdev_prop_bios_chs_trans; extern PropertyInfo qdev_prop_drive; extern PropertyInfo qdev_prop_netdev; extern PropertyInfo qdev_prop_vlan; @@ -299,6 +300,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr; #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ LostTickPolicy) +#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \ + DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int) #define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t) #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \ From 6e6f61a66aa45a15c9f411ad000a8d3d57272f8a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:48 +0200 Subject: [PATCH 24/41] ide: qdev property for BIOS CHS translation This isn't quite orthodox. CHS translation is firmware configuration, communicated via the RTC's CMOS RAM, not a property of the disk. But it's best to treat it just like geometry anyway. Maintain backward compatibility exactly like for geometry: fall back to DriveInfo's translation, set with -drive trans=... Bonus: info qtree now shows the translation. Except when it shows "auto": that's resolved by pc_cmos_init_late(). To be addressed shortly. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/ide/internal.h | 1 + hw/ide/qdev.c | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 685e976c51..c3ecafc1f4 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -474,6 +474,7 @@ struct IDEDevice { DeviceState qdev; uint32_t unit; BlockConf conf; + int chs_trans; char *version; char *serial; char *model; diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 3e297dc88a..f191dd3d90 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -144,7 +144,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) IDEState *s = bus->ifs + dev->unit; const char *serial; DriveInfo *dinfo; - int trans; if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) { error_report("discard_granularity must be 512 for ide"); @@ -160,25 +159,24 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) } } - trans = BIOS_ATA_TRANSLATION_AUTO; if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { /* try to fall back to value set with legacy -drive cyls=... */ dinfo = drive_get_by_blockdev(dev->conf.bs); dev->conf.cyls = dinfo->cyls; dev->conf.heads = dinfo->heads; dev->conf.secs = dinfo->secs; - trans = dinfo->trans; + dev->chs_trans = dinfo->trans; } if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { hd_geometry_guess(dev->conf.bs, &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs, - &trans); + &dev->chs_trans); } if (ide_init_drive(s, dev->conf.bs, kind, dev->version, serial, dev->model, dev->wwn, dev->conf.cyls, dev->conf.heads, dev->conf.secs, - trans) < 0) { + dev->chs_trans) < 0) { return -1; } @@ -222,6 +220,8 @@ static int ide_drive_initfn(IDEDevice *dev) static Property ide_hd_properties[] = { DEFINE_IDE_DEV_PROPERTIES(), DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf), + DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans", + IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO), DEFINE_PROP_END_OF_LIST(), }; From 856dcba23ad2aeea4d98d5d3c97cd46aac0cd073 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:49 +0200 Subject: [PATCH 25/41] qtest: Cover qdev property for BIOS CHS translation Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- tests/hd-geo-test.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c index a47b94507f..5d9d2e4c6d 100644 --- a/tests/hd-geo-test.c +++ b/tests/hd-geo-test.c @@ -321,15 +321,16 @@ static void test_ide_drive_user(const char *dev, bool trans) const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans }; argc = setup_common(argv, ARRAY_SIZE(argv)); - opts = g_strdup_printf("%s,cyls=%d,heads=%d,secs=%d%s", - dev && !trans ? dev : "", + opts = g_strdup_printf("%s,%s%scyls=%d,heads=%d,secs=%d", + dev ?: "", + trans && dev ? "bios-chs-" : "", + trans ? "trans=lba," : "", expected_chst.cyls, expected_chst.heads, - expected_chst.secs, - trans ? ",trans=lba" : ""); + expected_chst.secs); cur_ide[0] = &expected_chst; argc = setup_ide(argc, argv, ARRAY_SIZE(argv), - 0, dev && !trans ? opts : NULL, backend_small, mbr_chs, - dev && !trans ? "" : opts); + 0, dev ? opts : NULL, backend_small, mbr_chs, + dev ? "" : opts); g_free(opts); qtest_start(g_strjoinv(" ", argv)); test_cmos(); From 2b584959ed300ddff4acba0d7554becad5f274fd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:50 +0200 Subject: [PATCH 26/41] block: Geometry and translation hints are now useless, purge them There are two producers of these hints: drive_init() on behalf of -drive, and hd_geometry_guess(). The only consumer of the hint is hd_geometry_guess(). The callers of hd_geometry_guess() call it only when drive_init() didn't set the hints. Therefore, drive_init()'s hints are never used. Thus, hd_geometry_guess() only ever sees hints it produced itself in a prior call. Only the first call computes something, subsequent calls just repeat the first call's results. However, hd_geometry_guess() is never called more than once: the device models don't, and the block device is destroyed on unplug. Thus, dropping the repeat feature doesn't break anything now. If a block device wasn't destroyed on unplug and could be reused with a new device, then repeating old results would be wrong. Thus, dropping the repeat feature prevents future breakage. This renders the hints unused. Purge them from the block layer. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block.c | 32 -------------------------------- block.h | 12 ------------ block_int.h | 1 - blockdev.c | 14 ++------------ hw/block-common.h | 6 ++++++ hw/hd-geometry.c | 20 +------------------- hw/pc.c | 1 + hw/qdev-properties.c | 1 + vl.c | 2 +- 9 files changed, 12 insertions(+), 77 deletions(-) diff --git a/block.c b/block.c index 06323cfe53..ce7eb8f79b 100644 --- a/block.c +++ b/block.c @@ -996,12 +996,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, bs_dest->block_timer = bs_src->block_timer; bs_dest->io_limits_enabled = bs_src->io_limits_enabled; - /* geometry */ - bs_dest->cyls = bs_src->cyls; - bs_dest->heads = bs_src->heads; - bs_dest->secs = bs_src->secs; - bs_dest->translation = bs_src->translation; - /* r/w error */ bs_dest->on_read_error = bs_src->on_read_error; bs_dest->on_write_error = bs_src->on_write_error; @@ -2132,27 +2126,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr) *nb_sectors_ptr = length; } -void bdrv_set_geometry_hint(BlockDriverState *bs, - int cyls, int heads, int secs) -{ - bs->cyls = cyls; - bs->heads = heads; - bs->secs = secs; -} - -void bdrv_set_translation_hint(BlockDriverState *bs, int translation) -{ - bs->translation = translation; -} - -void bdrv_get_geometry_hint(BlockDriverState *bs, - int *pcyls, int *pheads, int *psecs) -{ - *pcyls = bs->cyls; - *pheads = bs->heads; - *psecs = bs->secs; -} - /* throttling disk io limits */ void bdrv_set_io_limits(BlockDriverState *bs, BlockIOLimit *io_limits) @@ -2161,11 +2134,6 @@ void bdrv_set_io_limits(BlockDriverState *bs, bs->io_limits_enabled = bdrv_io_limits_enabled(bs); } -int bdrv_get_translation_hint(BlockDriverState *bs) -{ - return bs->translation; -} - void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error, BlockErrorAction on_write_error) { diff --git a/block.h b/block.h index 1cd8a01c75..29c5eabf6e 100644 --- a/block.h +++ b/block.h @@ -257,18 +257,6 @@ int bdrv_has_zero_init(BlockDriverState *bs); int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum); -#define BIOS_ATA_TRANSLATION_AUTO 0 -#define BIOS_ATA_TRANSLATION_NONE 1 -#define BIOS_ATA_TRANSLATION_LBA 2 -#define BIOS_ATA_TRANSLATION_LARGE 3 -#define BIOS_ATA_TRANSLATION_RECHS 4 - -void bdrv_set_geometry_hint(BlockDriverState *bs, - int cyls, int heads, int secs); -void bdrv_set_translation_hint(BlockDriverState *bs, int translation); -void bdrv_get_geometry_hint(BlockDriverState *bs, - int *pcyls, int *pheads, int *psecs); -int bdrv_get_translation_hint(BlockDriverState *bs); void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error, BlockErrorAction on_write_error); BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read); diff --git a/block_int.h b/block_int.h index 1fb5352d0e..d72317fbe3 100644 --- a/block_int.h +++ b/block_int.h @@ -320,7 +320,6 @@ struct BlockDriverState { /* NOTE: the following infos are only hints for real hardware drivers. They are not used by the block driver */ - int cyls, heads, secs, translation; BlockErrorAction on_read_error, on_write_error; bool iostatus_enabled; BlockDeviceIoStatus iostatus; diff --git a/blockdev.c b/blockdev.c index 161985b1e5..06c997e864 100644 --- a/blockdev.c +++ b/blockdev.c @@ -7,8 +7,8 @@ * later. See the COPYING file in the top-level directory. */ -#include "block.h" #include "blockdev.h" +#include "hw/block-common.h" #include "monitor.h" #include "qerror.h" #include "qemu-option.h" @@ -551,17 +551,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) case IF_SCSI: case IF_XEN: case IF_NONE: - switch(media) { - case MEDIA_DISK: - if (cyls != 0) { - bdrv_set_geometry_hint(dinfo->bdrv, cyls, heads, secs); - bdrv_set_translation_hint(dinfo->bdrv, translation); - } - break; - case MEDIA_CDROM: - dinfo->media_cd = 1; - break; - } + dinfo->media_cd = media == MEDIA_CDROM; break; case IF_SD: case IF_FLOPPY: diff --git a/hw/block-common.h b/hw/block-common.h index 2f651867d2..ec7810d7d4 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -15,6 +15,12 @@ /* Hard disk geometry */ +#define BIOS_ATA_TRANSLATION_AUTO 0 +#define BIOS_ATA_TRANSLATION_NONE 1 +#define BIOS_ATA_TRANSLATION_LBA 2 +#define BIOS_ATA_TRANSLATION_LARGE 3 +#define BIOS_ATA_TRANSLATION_RECHS 4 + void hd_geometry_guess(BlockDriverState *bs, uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs, int *ptrans); diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index 7626cbbe9c..74678a656b 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -122,25 +122,10 @@ void hd_geometry_guess(BlockDriverState *bs, { int cylinders, heads, secs, translation; - bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs); - translation = bdrv_get_translation_hint(bs); - - if (cylinders != 0) { - /* already got a geometry hint: use it */ - *pcyls = cylinders; - *pheads = heads; - *psecs = secs; - if (ptrans) { - *ptrans = translation; - } - return; - } - - assert(translation == BIOS_ATA_TRANSLATION_AUTO); - if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) { /* no LCHS guess: use a standard physical disk geometry */ guess_chs_for_size(bs, pcyls, pheads, psecs); + translation = BIOS_ATA_TRANSLATION_AUTO; } else if (heads > 16) { /* LCHS guess with heads > 16 means that a BIOS LBA translation was active, so a standard physical disk @@ -149,7 +134,6 @@ void hd_geometry_guess(BlockDriverState *bs, translation = *pcyls * *pheads <= 131072 ? BIOS_ATA_TRANSLATION_LARGE : BIOS_ATA_TRANSLATION_LBA; - bdrv_set_translation_hint(bs, translation); } else { /* LCHS guess with heads <= 16: use as physical geometry */ *pcyls = cylinders; @@ -158,11 +142,9 @@ void hd_geometry_guess(BlockDriverState *bs, /* disable any translation to be in sync with the logical geometry */ translation = BIOS_ATA_TRANSLATION_NONE; - bdrv_set_translation_hint(bs, translation); } if (ptrans) { *ptrans = translation; } - bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs); trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation); } diff --git a/hw/pc.c b/hw/pc.c index 89a0c661db..77b12b4c11 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -44,6 +44,7 @@ #include "kvm.h" #include "xen.h" #include "blockdev.h" +#include "hw/block-common.h" #include "ui/qemu-spice.h" #include "memory.h" #include "exec-memory.h" diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 0b18f8c330..01c378f534 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -2,6 +2,7 @@ #include "qdev.h" #include "qerror.h" #include "blockdev.h" +#include "hw/block-common.h" void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) { diff --git a/vl.c b/vl.c index 46248b9c1c..8904db1a33 100644 --- a/vl.c +++ b/vl.c @@ -130,8 +130,8 @@ int main(int argc, char **argv) #include "qemu-timer.h" #include "qemu-char.h" #include "cache-utils.h" -#include "block.h" #include "blockdev.h" +#include "hw/block-common.h" #include "block-migration.h" #include "dma.h" #include "audio/audio.h" From 9dc13e381385787b65ad2095ed343bbc8b4e5220 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:51 +0200 Subject: [PATCH 27/41] ide pc: Put hard disk info into CMOS only for hard disks In particular, don't set disk type and geometry when a CD-ROM on bus ide.0 has media during CMOS initialization. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/ide/qdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index f191dd3d90..84097fd4ec 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -117,7 +117,7 @@ int ide_get_geometry(BusState *bus, int unit, { IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit]; - if (!s->bs) { + if (s->drive_kind != IDE_HD || !s->bs) { return -1; } From 4e4e6e319b5508289da0f2966f63c841c832b847 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:52 +0200 Subject: [PATCH 28/41] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- tests/hd-geo-test.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c index 5d9d2e4c6d..9a31e8587f 100644 --- a/tests/hd-geo-test.c +++ b/tests/hd-geo-test.c @@ -369,6 +369,27 @@ static void test_ide_device_user_chst(void) test_ide_drive_user("ide-hd", true); } +/* + * Test case: IDE devices (if=ide), but use index=0 for CD-ROM + */ +static void test_ide_drive_cd_0(void) +{ + char *argv[256]; + int argc, ide_idx; + Backend i; + + argc = setup_common(argv, ARRAY_SIZE(argv)); + for (i = 0; i <= backend_empty; i++) { + ide_idx = backend_empty - i; + cur_ide[ide_idx] = &hd_chst[i][mbr_blank]; + argc = setup_ide(argc, argv, ARRAY_SIZE(argv), + ide_idx, NULL, i, mbr_blank, ""); + } + qtest_start(g_strjoinv(" ", argv)); + test_cmos(); + qtest_quit(global_qtest); +} + int main(int argc, char **argv) { Backend i; @@ -390,6 +411,7 @@ int main(int argc, char **argv) qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs); qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs); qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst); + qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0); qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank); qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba); qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs); From 2adc99b277ab05877ef847bddde45346378f561a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:53 +0200 Subject: [PATCH 29/41] hd-geometry: Compute BIOS CHS translation in one place Currently, it is split between hd_geometry_guess() and pc_cmos_init_late(). Confusing. info qtree shows the result of the former. Also confusing. Fold the part done in pc_cmos_init_late() into hd_geometry_guess(). Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/block-common.h | 1 + hw/hd-geometry.c | 9 ++++++++- hw/ide/core.c | 2 ++ hw/ide/qdev.c | 3 +++ hw/pc.c | 19 ++++--------------- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/hw/block-common.h b/hw/block-common.h index ec7810d7d4..31e12baced 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -24,5 +24,6 @@ void hd_geometry_guess(BlockDriverState *bs, uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs, int *ptrans); +int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs); #endif diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index 74678a656b..1cdb9fb753 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -125,7 +125,7 @@ void hd_geometry_guess(BlockDriverState *bs, if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) { /* no LCHS guess: use a standard physical disk geometry */ guess_chs_for_size(bs, pcyls, pheads, psecs); - translation = BIOS_ATA_TRANSLATION_AUTO; + translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs); } else if (heads > 16) { /* LCHS guess with heads > 16 means that a BIOS LBA translation was active, so a standard physical disk @@ -148,3 +148,10 @@ void hd_geometry_guess(BlockDriverState *bs, } trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation); } + +int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs) +{ + return cyls <= 1024 && heads <= 16 && secs <= 63 + ? BIOS_ATA_TRANSLATION_NONE + : BIOS_ATA_TRANSLATION_LBA; +} diff --git a/hw/ide/core.c b/hw/ide/core.c index bf1ce8968a..1ca7cdf7d0 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2091,6 +2091,8 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, trans = dinfo->trans; if (!cyls && !heads && !secs) { hd_geometry_guess(dinfo->bdrv, &cyls, &heads, &secs, &trans); + } else if (trans == BIOS_ATA_TRANSLATION_AUTO) { + trans = hd_bios_chs_auto_trans(cyls, heads, secs); } if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, dinfo->media_cd ? IDE_CD : IDE_HD, NULL, diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 84097fd4ec..de9db3bf9f 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -171,6 +171,9 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) hd_geometry_guess(dev->conf.bs, &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs, &dev->chs_trans); + } else if (dev->chs_trans == BIOS_ATA_TRANSLATION_AUTO) { + dev->chs_trans = hd_bios_chs_auto_trans(dev->conf.cyls, + dev->conf.heads, dev->conf.secs); } if (ide_init_drive(s, dev->conf.bs, kind, diff --git a/hw/pc.c b/hw/pc.c index 77b12b4c11..598267af89 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -290,7 +290,7 @@ static void pc_cmos_init_late(void *opaque) int16_t cylinders; int8_t heads, sectors; int val; - int i; + int i, trans; val = 0; if (ide_get_geometry(arg->idebus[0], 0, @@ -313,20 +313,9 @@ static void pc_cmos_init_late(void *opaque) geometry can be different if a translation is done. */ if (ide_get_geometry(arg->idebus[i / 2], i % 2, &cylinders, &heads, §ors) >= 0) { - int translation = ide_get_bios_chs_trans(arg->idebus[i / 2], - i % 2); - if (translation == BIOS_ATA_TRANSLATION_AUTO) { - if (cylinders <= 1024 && heads <= 16 && sectors <= 63) { - /* No translation. */ - translation = 0; - } else { - /* LBA translation. */ - translation = 1; - } - } else { - translation--; - } - val |= translation << (i * 2); + trans = ide_get_bios_chs_trans(arg->idebus[i / 2], i % 2) - 1; + assert((trans & ~3) == 0); + val |= trans << (i * 2); } } rtc_set_memory(s, 0x39, val); From aaea3f366eeb8c5c23d821cdd1ce078086fe3764 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:54 +0200 Subject: [PATCH 30/41] blockdev: Drop redundant CHS validation for if=ide Leave it to ide_init_drive(). Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- blockdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/blockdev.c b/blockdev.c index 06c997e864..5f8677ef05 100644 --- a/blockdev.c +++ b/blockdev.c @@ -330,15 +330,15 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) max_devs = if_max_devs[type]; if (cyls || heads || secs) { - if (cyls < 1 || (type == IF_IDE && cyls > 16383)) { + if (cyls < 1) { error_report("invalid physical cyls number"); return NULL; } - if (heads < 1 || (type == IF_IDE && heads > 16)) { + if (heads < 1) { error_report("invalid physical heads number"); return NULL; } - if (secs < 1 || (type == IF_IDE && secs > 63)) { + if (secs < 1) { error_report("invalid physical secs number"); return NULL; } From b51daf003aa42c5c23876739ebd0b64dd2075931 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 10 Jul 2012 11:12:55 +0200 Subject: [PATCH 31/41] Relax IDE CHS limits from 16383,16,63 to 65535,16,255 New limits straight from ATA4 6.2 Register delivered data transfer command sector addressing. I figure the old sector limit 63 was blindly copied from the BIOS int 13 limit. Doesn't apply to the hardware. No idea where the old cylinder limit comes from. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/ide/core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 1ca7cdf7d0..58a454fde5 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1935,16 +1935,16 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, s->drive_kind = kind; bdrv_get_geometry(bs, &nb_sectors); - if (cylinders < 1 || cylinders > 16383) { - error_report("cyls must be between 1 and 16383"); + if (cylinders < 1 || cylinders > 65535) { + error_report("cyls must be between 1 and 65535"); return -1; } if (heads < 1 || heads > 16) { error_report("heads must be between 1 and 16"); return -1; } - if (secs < 1 || secs > 63) { - error_report("secs must be between 1 and 63"); + if (secs < 1 || secs > 255) { + error_report("secs must be between 1 and 255"); return -1; } s->cylinders = cylinders; From 31e404f4ffb6adadea0b35de08e0a6b640e81a02 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 11 Jul 2012 15:08:36 +0200 Subject: [PATCH 32/41] hw/block-common: Move BlockConf & friends from block.h This stuff doesn't belong to block layer, and was put there only because a better home didn't exist then. Now it does. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block.h | 45 --------------------------------------------- hw/block-common.h | 45 +++++++++++++++++++++++++++++++++++++++++++++ hw/ide/internal.h | 1 + hw/scsi.h | 1 + hw/usb.h | 1 - hw/virtio-blk.h | 2 +- hw/virtio.h | 1 - 7 files changed, 48 insertions(+), 48 deletions(-) diff --git a/block.h b/block.h index 29c5eabf6e..c89590df4d 100644 --- a/block.h +++ b/block.h @@ -403,49 +403,4 @@ typedef enum { #define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt) void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event); - -/* Convenience for block device models */ - -typedef struct BlockConf { - BlockDriverState *bs; - uint16_t physical_block_size; - uint16_t logical_block_size; - uint16_t min_io_size; - uint32_t opt_io_size; - int32_t bootindex; - uint32_t discard_granularity; - /* geometry, not all devices use this */ - uint32_t cyls, heads, secs; -} BlockConf; - -static inline unsigned int get_physical_block_exp(BlockConf *conf) -{ - unsigned int exp = 0, size; - - for (size = conf->physical_block_size; - size > conf->logical_block_size; - size >>= 1) { - exp++; - } - - return exp; -} - -#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ - DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \ - DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \ - _conf.logical_block_size, 512), \ - DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ - _conf.physical_block_size, 512), \ - DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ - DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ - DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ - DEFINE_PROP_UINT32("discard_granularity", _state, \ - _conf.discard_granularity, 0) - -#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ - DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ - DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ - DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0) - #endif diff --git a/hw/block-common.h b/hw/block-common.h index 31e12baced..f0d509b93d 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -13,6 +13,51 @@ #include "qemu-common.h" +/* Configuration */ + +typedef struct BlockConf { + BlockDriverState *bs; + uint16_t physical_block_size; + uint16_t logical_block_size; + uint16_t min_io_size; + uint32_t opt_io_size; + int32_t bootindex; + uint32_t discard_granularity; + /* geometry, not all devices use this */ + uint32_t cyls, heads, secs; +} BlockConf; + +static inline unsigned int get_physical_block_exp(BlockConf *conf) +{ + unsigned int exp = 0, size; + + for (size = conf->physical_block_size; + size > conf->logical_block_size; + size >>= 1) { + exp++; + } + + return exp; +} + +#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ + DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \ + DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \ + _conf.logical_block_size, 512), \ + DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ + _conf.physical_block_size, 512), \ + DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ + DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ + DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ + DEFINE_PROP_UINT32("discard_granularity", _state, \ + _conf.discard_granularity, 0) + +#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ + DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ + DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ + DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0) + + /* Hard disk geometry */ #define BIOS_ATA_TRANSLATION_AUTO 0 diff --git a/hw/ide/internal.h b/hw/ide/internal.h index c3ecafc1f4..7170bd9cd0 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -11,6 +11,7 @@ #include "iorange.h" #include "dma.h" #include "sysemu.h" +#include "hw/block-common.h" #include "hw/scsi-defs.h" /* debug IDE devices */ diff --git a/hw/scsi.h b/hw/scsi.h index 76f06d41de..d90e9702fb 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -3,6 +3,7 @@ #include "qdev.h" #include "block.h" +#include "hw/block-common.h" #include "sysemu.h" #define MAX_SCSI_DEVS 255 diff --git a/hw/usb.h b/hw/usb.h index 7ed8fb8fcf..432ccae57f 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -25,7 +25,6 @@ * THE SOFTWARE. */ -#include "block.h" #include "qdev.h" #include "qemu-queue.h" diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h index d7850012bd..79ebccc95b 100644 --- a/hw/virtio-blk.h +++ b/hw/virtio-blk.h @@ -15,7 +15,7 @@ #define _QEMU_VIRTIO_BLK_H #include "virtio.h" -#include "block.h" +#include "hw/block-common.h" /* from Linux's linux/virtio_blk.h */ diff --git a/hw/virtio.h b/hw/virtio.h index 85aabe53d8..42a7762999 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -18,7 +18,6 @@ #include "net.h" #include "qdev.h" #include "sysemu.h" -#include "block.h" #include "event_notifier.h" #ifdef CONFIG_LINUX #include "9p.h" From 911525dba9ecc21f97b05c0f09bf9319a9de3a7d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 11 Jul 2012 15:08:37 +0200 Subject: [PATCH 33/41] hw/block-common: Factor out fall back to legacy -drive serial=... Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/Makefile.objs | 2 +- hw/block-common.c | 24 ++++++++++++++++++++++++ hw/block-common.h | 3 +++ hw/ide/qdev.c | 12 ++---------- hw/scsi-disk.c | 8 +------- hw/usb/dev-storage.c | 10 ++-------- hw/virtio-blk.c | 8 +------- 7 files changed, 34 insertions(+), 33 deletions(-) create mode 100644 hw/block-common.c diff --git a/hw/Makefile.objs b/hw/Makefile.objs index c3bdedcf28..8327e55c03 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -138,7 +138,7 @@ common-obj-$(CONFIG_MAX111X) += max111x.o common-obj-$(CONFIG_DS1338) += ds1338.o common-obj-y += i2c.o smbus.o smbus_eeprom.o common-obj-y += eeprom93xx.o -common-obj-y += scsi-disk.o cdrom.o hd-geometry.o +common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.o common-obj-y += scsi-generic.o scsi-bus.o common-obj-y += hid.o common-obj-$(CONFIG_SSI) += ssi.o diff --git a/hw/block-common.c b/hw/block-common.c new file mode 100644 index 0000000000..036334b1ec --- /dev/null +++ b/hw/block-common.c @@ -0,0 +1,24 @@ +/* + * Common code for block device models + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#include "blockdev.h" +#include "hw/block-common.h" + +void blkconf_serial(BlockConf *conf, char **serial) +{ + DriveInfo *dinfo; + + if (!*serial) { + /* try to fall back to value set with legacy -drive serial=... */ + dinfo = drive_get_by_blockdev(conf->bs); + if (*dinfo->serial) { + *serial = g_strdup(dinfo->serial); + } + } +} diff --git a/hw/block-common.h b/hw/block-common.h index f0d509b93d..52bdddaac6 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -57,6 +57,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0) +/* Configuration helpers */ + +void blkconf_serial(BlockConf *conf, char **serial); /* Hard disk geometry */ diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index de9db3bf9f..7fe803c85f 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -142,7 +142,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) { IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus); IDEState *s = bus->ifs + dev->unit; - const char *serial; DriveInfo *dinfo; if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) { @@ -150,14 +149,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) return -1; } - serial = dev->serial; - if (!serial) { - /* try to fall back to value set with legacy -drive serial=... */ - dinfo = drive_get_by_blockdev(dev->conf.bs); - if (*dinfo->serial) { - serial = dinfo->serial; - } - } + blkconf_serial(&dev->conf, &dev->serial); if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { /* try to fall back to value set with legacy -drive cyls=... */ @@ -177,7 +169,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) } if (ide_init_drive(s, dev->conf.bs, kind, - dev->version, serial, dev->model, dev->wwn, + dev->version, dev->serial, dev->model, dev->wwn, dev->conf.cyls, dev->conf.heads, dev->conf.secs, dev->chs_trans) < 0) { return -1; diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 0a182f987e..39a07d7579 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1777,13 +1777,7 @@ static int scsi_initfn(SCSIDevice *dev) } } - if (!s->serial) { - /* try to fall back to value set with legacy -drive serial=... */ - dinfo = drive_get_by_blockdev(s->qdev.conf.bs); - if (*dinfo->serial) { - s->serial = g_strdup(dinfo->serial); - } - } + blkconf_serial(&s->qdev.conf, &s->serial); if (!s->version) { s->version = g_strdup(qemu_get_version()); diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 251e7de1cd..7fa8b83d2e 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -532,13 +532,14 @@ static int usb_msd_initfn(USBDevice *dev) { MSDState *s = DO_UPCAST(MSDState, dev, dev); BlockDriverState *bs = s->conf.bs; - DriveInfo *dinfo; if (!bs) { error_report("drive property not set"); return -1; } + blkconf_serial(&s->conf, &s->serial); + /* * Hack alert: this pretends to be a block device, but it's really * a SCSI bus that can serve only a single device, which it @@ -551,13 +552,6 @@ static int usb_msd_initfn(USBDevice *dev) bdrv_detach_dev(bs, &s->dev.qdev); s->conf.bs = NULL; - if (!s->serial) { - /* try to fall back to value set with legacy -drive serial=... */ - dinfo = drive_get_by_blockdev(bs); - if (*dinfo->serial) { - s->serial = strdup(dinfo->serial); - } - } if (s->serial) { usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial); } else { diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index 38859048b9..ba087bc17f 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -600,13 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) return NULL; } - if (!blk->serial) { - /* try to fall back to value set with legacy -drive serial=... */ - dinfo = drive_get_by_blockdev(blk->conf.bs); - if (*dinfo->serial) { - blk->serial = strdup(dinfo->serial); - } - } + blkconf_serial(&blk->conf, &blk->serial); s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK, sizeof(struct virtio_blk_config), From 577d0a38070d1d6c4c7fab5c2054380770b1ec6b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 11 Jul 2012 15:08:38 +0200 Subject: [PATCH 34/41] blockdev: Don't limit DriveInfo serial to 20 characters All current users (IDE, SCSI and virtio-blk) happen to share this 20 characters limit. Still, it should be left to device models. They already enforce their limits. They have to, as the DriveInfo limit only affects legacy -drive serial=..., not the qdev properties. usb-storage, which doesn't limit serial number length, also uses DriveInfo for -usbdevice. But that doesn't provide access to DriveInfo serial. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- blockdev.c | 4 +--- blockdev.h | 4 +--- hw/block-common.c | 2 +- hw/ide/core.c | 6 +++--- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/blockdev.c b/blockdev.c index 5f8677ef05..3d7501565d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -536,9 +536,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) dinfo->trans = translation; dinfo->opts = opts; dinfo->refcount = 1; - if (serial) { - pstrcpy(dinfo->serial, sizeof(dinfo->serial), serial); - } + dinfo->serial = serial; QTAILQ_INSERT_TAIL(&drives, dinfo, next); bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error); diff --git a/blockdev.h b/blockdev.h index 9c29948128..5f27b643be 100644 --- a/blockdev.h +++ b/blockdev.h @@ -17,8 +17,6 @@ void blockdev_mark_auto_del(BlockDriverState *bs); void blockdev_auto_del(BlockDriverState *bs); -#define BLOCK_SERIAL_STRLEN 20 - typedef enum { IF_DEFAULT = -1, /* for use with drive_add() only */ IF_NONE, @@ -37,7 +35,7 @@ struct DriveInfo { int media_cd; int cyls, heads, secs, trans; QemuOpts *opts; - char serial[BLOCK_SERIAL_STRLEN + 1]; + const char *serial; QTAILQ_ENTRY(DriveInfo) next; int refcount; }; diff --git a/hw/block-common.c b/hw/block-common.c index 036334b1ec..0a0542a650 100644 --- a/hw/block-common.c +++ b/hw/block-common.c @@ -17,7 +17,7 @@ void blkconf_serial(BlockConf *conf, char **serial) if (!*serial) { /* try to fall back to value set with legacy -drive serial=... */ dinfo = drive_get_by_blockdev(conf->bs); - if (*dinfo->serial) { + if (dinfo->serial) { *serial = g_strdup(dinfo->serial); } } diff --git a/hw/ide/core.c b/hw/ide/core.c index 58a454fde5..5378fc39fe 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2095,9 +2095,9 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, trans = hd_bios_chs_auto_trans(cyls, heads, secs); } if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, - dinfo->media_cd ? IDE_CD : IDE_HD, NULL, - *dinfo->serial ? dinfo->serial : NULL, - NULL, 0, cyls, heads, secs, trans) < 0) { + dinfo->media_cd ? IDE_CD : IDE_HD, + NULL, dinfo->serial, NULL, 0, + cyls, heads, secs, trans) < 0) { error_report("Can't set up IDE drive %s", dinfo->id); exit(1); } From b7eb0c9f95e50239ce5b5266373dc52c85e75299 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 11 Jul 2012 15:08:39 +0200 Subject: [PATCH 35/41] hw/block-common: Factor out fall back to legacy -drive cyls=... Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- hw/block-common.c | 40 ++++++++++++++++++++++++++++++++++++++++ hw/block-common.h | 2 ++ hw/ide/core.c | 24 ++++++++++++------------ hw/ide/qdev.c | 19 ++----------------- hw/scsi-disk.c | 31 +++---------------------------- hw/virtio-blk.c | 31 +++---------------------------- 6 files changed, 62 insertions(+), 85 deletions(-) diff --git a/hw/block-common.c b/hw/block-common.c index 0a0542a650..f0196d78dc 100644 --- a/hw/block-common.c +++ b/hw/block-common.c @@ -9,6 +9,7 @@ #include "blockdev.h" #include "hw/block-common.h" +#include "qemu-error.h" void blkconf_serial(BlockConf *conf, char **serial) { @@ -22,3 +23,42 @@ void blkconf_serial(BlockConf *conf, char **serial) } } } + +int blkconf_geometry(BlockConf *conf, int *ptrans, + unsigned cyls_max, unsigned heads_max, unsigned secs_max) +{ + DriveInfo *dinfo; + + if (!conf->cyls && !conf->heads && !conf->secs) { + /* try to fall back to value set with legacy -drive cyls=... */ + dinfo = drive_get_by_blockdev(conf->bs); + conf->cyls = dinfo->cyls; + conf->heads = dinfo->heads; + conf->secs = dinfo->secs; + if (ptrans) { + *ptrans = dinfo->trans; + } + } + if (!conf->cyls && !conf->heads && !conf->secs) { + hd_geometry_guess(conf->bs, + &conf->cyls, &conf->heads, &conf->secs, + ptrans); + } else if (ptrans && *ptrans == BIOS_ATA_TRANSLATION_AUTO) { + *ptrans = hd_bios_chs_auto_trans(conf->cyls, conf->heads, conf->secs); + } + if (conf->cyls || conf->heads || conf->secs) { + if (conf->cyls < 1 || conf->cyls > cyls_max) { + error_report("cyls must be between 1 and %u", cyls_max); + return -1; + } + if (conf->heads < 1 || conf->heads > heads_max) { + error_report("heads must be between 1 and %u", heads_max); + return -1; + } + if (conf->secs < 1 || conf->secs > secs_max) { + error_report("secs must be between 1 and %u", secs_max); + return -1; + } + } + return 0; +} diff --git a/hw/block-common.h b/hw/block-common.h index 52bdddaac6..bb808f7f56 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -60,6 +60,8 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) /* Configuration helpers */ void blkconf_serial(BlockConf *conf, char **serial); +int blkconf_geometry(BlockConf *conf, int *trans, + unsigned cyls_max, unsigned heads_max, unsigned secs_max); /* Hard disk geometry */ diff --git a/hw/ide/core.c b/hw/ide/core.c index 5378fc39fe..d65ef3d58d 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1935,18 +1935,6 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, s->drive_kind = kind; bdrv_get_geometry(bs, &nb_sectors); - if (cylinders < 1 || cylinders > 65535) { - error_report("cyls must be between 1 and 65535"); - return -1; - } - if (heads < 1 || heads > 16) { - error_report("heads must be between 1 and 16"); - return -1; - } - if (secs < 1 || secs > 255) { - error_report("secs must be between 1 and 255"); - return -1; - } s->cylinders = cylinders; s->heads = heads; s->sectors = secs; @@ -2094,6 +2082,18 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, } else if (trans == BIOS_ATA_TRANSLATION_AUTO) { trans = hd_bios_chs_auto_trans(cyls, heads, secs); } + if (cyls < 1 || cyls > 65535) { + error_report("cyls must be between 1 and 65535"); + exit(1); + } + if (heads < 1 || heads > 16) { + error_report("heads must be between 1 and 16"); + exit(1); + } + if (secs < 1 || secs > 255) { + error_report("secs must be between 1 and 255"); + exit(1); + } if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, dinfo->media_cd ? IDE_CD : IDE_HD, NULL, dinfo->serial, NULL, 0, diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 7fe803c85f..22e58dfc8a 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -142,7 +142,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) { IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus); IDEState *s = bus->ifs + dev->unit; - DriveInfo *dinfo; if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) { error_report("discard_granularity must be 512 for ide"); @@ -150,22 +149,8 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) } blkconf_serial(&dev->conf, &dev->serial); - - if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { - /* try to fall back to value set with legacy -drive cyls=... */ - dinfo = drive_get_by_blockdev(dev->conf.bs); - dev->conf.cyls = dinfo->cyls; - dev->conf.heads = dinfo->heads; - dev->conf.secs = dinfo->secs; - dev->chs_trans = dinfo->trans; - } - if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { - hd_geometry_guess(dev->conf.bs, - &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs, - &dev->chs_trans); - } else if (dev->chs_trans == BIOS_ATA_TRANSLATION_AUTO) { - dev->chs_trans = hd_bios_chs_auto_trans(dev->conf.cyls, - dev->conf.heads, dev->conf.secs); + if (blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255) < 0) { + return -1; } if (ide_init_drive(s, dev->conf.bs, kind, diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 39a07d7579..525816cb76 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1737,7 +1737,6 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev) static int scsi_initfn(SCSIDevice *dev) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); - DriveInfo *dinfo; if (!s->qdev.conf.bs) { error_report("drive property not set"); @@ -1750,34 +1749,10 @@ static int scsi_initfn(SCSIDevice *dev) return -1; } - if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { - /* try to fall back to value set with legacy -drive cyls=... */ - dinfo = drive_get_by_blockdev(s->qdev.conf.bs); - dev->conf.cyls = dinfo->cyls; - dev->conf.heads = dinfo->heads; - dev->conf.secs = dinfo->secs; - } - if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) { - hd_geometry_guess(s->qdev.conf.bs, - &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs, - NULL); - } - if (dev->conf.cyls || dev->conf.heads || dev->conf.secs) { - if (dev->conf.cyls < 1 || dev->conf.cyls > 65535) { - error_report("cyls must be between 1 and 65535"); - return -1; - } - if (dev->conf.heads < 1 || dev->conf.heads > 255) { - error_report("heads must be between 1 and 255"); - return -1; - } - if (dev->conf.secs < 1 || dev->conf.secs > 255) { - error_report("secs must be between 1 and 255"); - return -1; - } - } - blkconf_serial(&s->qdev.conf, &s->serial); + if (blkconf_geometry(&dev->conf, NULL, 65535, 255, 255) < 0) { + return -1; + } if (!s->version) { s->version = g_strdup(qemu_get_version()); diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index ba087bc17f..f21757ed55 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -589,7 +589,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) { VirtIOBlock *s; static int virtio_blk_id; - DriveInfo *dinfo; if (!blk->conf.bs) { error_report("drive property not set"); @@ -601,6 +600,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) } blkconf_serial(&blk->conf, &blk->serial); + if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) { + return NULL; + } s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK, sizeof(struct virtio_blk_config), @@ -615,33 +617,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) s->rq = NULL; s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; - if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) { - /* try to fall back to value set with legacy -drive cyls=... */ - dinfo = drive_get_by_blockdev(blk->conf.bs); - blk->conf.cyls = dinfo->cyls; - blk->conf.heads = dinfo->heads; - blk->conf.secs = dinfo->secs; - } - if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) { - hd_geometry_guess(s->bs, - &blk->conf.cyls, &blk->conf.heads, &blk->conf.secs, - NULL); - } - if (blk->conf.cyls || blk->conf.heads || blk->conf.secs) { - if (blk->conf.cyls < 1 || blk->conf.cyls > 65535) { - error_report("cyls must be between 1 and 65535"); - return NULL; - } - if (blk->conf.heads < 1 || blk->conf.heads > 255) { - error_report("heads must be between 1 and 255"); - return NULL; - } - if (blk->conf.secs < 1 || blk->conf.secs > 255) { - error_report("secs must be between 1 and 255"); - return NULL; - } - } - s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output); qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s); From 9e559533bd825a3e371497875576137a8586c831 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 2 Jul 2012 15:13:53 +0200 Subject: [PATCH 36/41] qemu-io: Fix memory leaks Almost all callers of create_iovec() forgot to destroy the qiov when the request has completed. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- qemu-io.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qemu-io.c b/qemu-io.c index 5882067443..8f3b94b838 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -670,6 +670,7 @@ static int readv_f(int argc, char **argv) print_report("read", &t2, offset, qiov.size, total, cnt, Cflag); out: + qemu_iovec_destroy(&qiov); qemu_io_free(buf); return 0; } @@ -928,6 +929,7 @@ static int writev_f(int argc, char **argv) t2 = tsub(t2, t1); print_report("wrote", &t2, offset, qiov.size, total, cnt, Cflag); out: + qemu_iovec_destroy(&qiov); qemu_io_free(buf); return 0; } @@ -1126,6 +1128,7 @@ static void aio_write_done(void *opaque, int ret) ctx->qiov.size, 1, ctx->Cflag); out: qemu_io_free(ctx->buf); + qemu_iovec_destroy(&ctx->qiov); g_free(ctx); } @@ -1166,6 +1169,7 @@ static void aio_read_done(void *opaque, int ret) ctx->qiov.size, 1, ctx->Cflag); out: qemu_io_free(ctx->buf); + qemu_iovec_destroy(&ctx->qiov); g_free(ctx); } From 3f4349dc8b9494315f8331b2ea4e8d1f83fb801d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 29 Jun 2012 13:40:27 +0200 Subject: [PATCH 37/41] coroutine-ucontext: Help valgrind understand coroutines valgrind tends to get confused and report false positives when you switch stacks and don't tell it about it. Signed-off-by: Kevin Wolf Reviewed-by: Paolo Bonzini --- configure | 20 ++++++++++++++++++++ coroutine-ucontext.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/configure b/configure index 0a3896e757..cef0a71a27 100755 --- a/configure +++ b/configure @@ -2870,6 +2870,22 @@ if compile_prog "" "" ; then linux_magic_h=yes fi +######################################## +# check if we have valgrind/valgrind.h + +valgrind_h=no +cat > $TMPC << EOF +#include +#pragma GCC diagnostic ignored "-Wunused-but-set-variable" +int main(void) { + VALGRIND_STACK_DEREGISTER(0); + return 0; +} +EOF +if compile_prog "" "" ; then + valgrind_h=yes +fi + ######################################## # check if environ is declared @@ -3379,6 +3395,10 @@ if test "$linux_magic_h" = "yes" ; then echo "CONFIG_LINUX_MAGIC_H=y" >> $config_host_mak fi +if test "$valgrind_h" = "yes" ; then + echo "CONFIG_VALGRIND_H=y" >> $config_host_mak +fi + if test "$has_environ" = "yes" ; then echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak fi diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c index 5f43083af5..e3c450b322 100644 --- a/coroutine-ucontext.c +++ b/coroutine-ucontext.c @@ -30,6 +30,10 @@ #include "qemu-common.h" #include "qemu-coroutine-int.h" +#ifdef CONFIG_VALGRIND_H +#include +#endif + enum { /* Maximum free pool size prevents holding too many freed coroutines */ POOL_MAX_SIZE = 64, @@ -43,6 +47,11 @@ typedef struct { Coroutine base; void *stack; jmp_buf env; + +#ifdef CONFIG_VALGRIND_H + unsigned int valgrind_stack_id; +#endif + } CoroutineUContext; /** @@ -159,6 +168,11 @@ static Coroutine *coroutine_new(void) uc.uc_stack.ss_size = stack_size; uc.uc_stack.ss_flags = 0; +#ifdef CONFIG_VALGRIND_H + co->valgrind_stack_id = + VALGRIND_STACK_REGISTER(co->stack, co->stack + stack_size); +#endif + arg.p = co; makecontext(&uc, (void (*)(void))coroutine_trampoline, @@ -185,6 +199,16 @@ Coroutine *qemu_coroutine_new(void) return co; } +#ifdef CONFIG_VALGRIND_H +/* Work around an unused variable in the valgrind.h macro... */ +#pragma GCC diagnostic ignored "-Wunused-but-set-variable" +static inline void valgrind_stack_deregister(CoroutineUContext *co) +{ + VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id); +} +#pragma GCC diagnostic error "-Wunused-but-set-variable" +#endif + void qemu_coroutine_delete(Coroutine *co_) { CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_); @@ -196,6 +220,10 @@ void qemu_coroutine_delete(Coroutine *co_) return; } +#ifdef CONFIG_VALGRIND_H + valgrind_stack_deregister(co); +#endif + g_free(co->stack); g_free(co); } From 2f24e8fb8d684f576f3dcced820860d70652a7f9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 28 Jun 2012 16:55:54 +0200 Subject: [PATCH 38/41] qemu-iotests: Valgrind support check -valgrind wraps all qemu-io calls with valgrind. This makes it a bit easier to debug problems that occur somewhere deep in a test case. Signed-off-by: Kevin Wolf Reviewed-by: Paolo Bonzini --- tests/qemu-iotests/common | 11 +++++++++++ tests/qemu-iotests/common.rc | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index eeb70cbcdc..1f6fdf5c56 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -41,6 +41,7 @@ sortme=false expunge=true have_test_arg=false randomize=false +valgrind=false rm -f $tmp.list $tmp.tmp $tmp.sed export IMGFMT=raw @@ -212,6 +213,11 @@ testlist options xpand=false ;; + -valgrind) + valgrind=true + xpand=false + ;; + -g) # -g group ... pick from group file group=true xpand=false @@ -345,3 +351,8 @@ fi [ "$QEMU" = "" ] && _fatal "qemu not found" [ "$QEMU_IMG" = "" ] && _fatal "qemu-img not found" [ "$QEMU_IO" = "" ] && _fatal "qemu-img not found" + +if $valgrind; then + export REAL_QEMU_IO="$QEMU_IO_PROG" + export QEMU_IO_PROG=valgrind_qemu_io +fi diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index e535874e4c..5e3a524bc8 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -53,6 +53,16 @@ else TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT fi +function valgrind_qemu_io() +{ + valgrind --log-file=/tmp/$$.valgrind --error-exitcode=99 $REAL_QEMU_IO "$@" + if [ $? != 0 ]; then + cat /tmp/$$.valgrind + fi + rm -f /tmp/$$.valgrind +} + + _optstr_add() { if [ -n "$1" ]; then From 6d013772c03e916adc3d57868542854311489b51 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Mon, 16 Jul 2012 15:48:26 +0200 Subject: [PATCH 39/41] fdc: fix relative seek Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- hw/fdc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index 41191c76c0..08830c1ba2 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -1802,7 +1802,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct } } -static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) +static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) { FDrive *cur_drv; @@ -1812,14 +1812,15 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) fd_seek(cur_drv, cur_drv->head, cur_drv->max_track - 1, cur_drv->sect, 1); } else { - fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1); + fd_seek(cur_drv, cur_drv->head, + cur_drv->track + fdctrl->fifo[2], cur_drv->sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ fdctrl_raise_irq(fdctrl, FD_SR0_SEEK); } -static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) +static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) { FDrive *cur_drv; @@ -1828,7 +1829,8 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) if (fdctrl->fifo[2] > cur_drv->track) { fd_seek(cur_drv, cur_drv->head, 0, cur_drv->sect, 1); } else { - fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1); + fd_seek(cur_drv, cur_drv->head, + cur_drv->track - fdctrl->fifo[2], cur_drv->sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ From 98272dbb5c0679018718814b5cc3fc26f3f76c5f Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Mon, 16 Jul 2012 15:48:27 +0200 Subject: [PATCH 40/41] fdc-test: introduce test_relative_seek Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- tests/fdc-test.c | 46 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 585fb0e343..10d11a4173 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -47,9 +47,11 @@ enum { }; enum { - CMD_SENSE_INT = 0x08, - CMD_SEEK = 0x0f, - CMD_READ = 0xe6, + CMD_SENSE_INT = 0x08, + CMD_SEEK = 0x0f, + CMD_READ = 0xe6, + CMD_RELATIVE_SEEK_OUT = 0x8f, + CMD_RELATIVE_SEEK_IN = 0xcf, }; enum { @@ -91,13 +93,17 @@ static uint8_t floppy_recv(void) return inb(FLOPPY_BASE + reg_fifo); } -static void ack_irq(void) +static uint8_t ack_irq(void) { + uint8_t ret; + g_assert(get_irq(FLOPPY_IRQ)); floppy_send(CMD_SENSE_INT); floppy_recv(); - floppy_recv(); + ret = floppy_recv(); g_assert(!get_irq(FLOPPY_IRQ)); + + return ret; } static uint8_t send_read_command(void) @@ -281,6 +287,35 @@ static void test_sense_interrupt(void) floppy_recv(); } +static void test_relative_seek(void) +{ + uint8_t drive = 0; + uint8_t head = 0; + uint8_t cyl = 1; + uint8_t ret; + + /* Send seek to track 0 */ + send_step_pulse(0); + + /* Send relative seek to increase track by 1 */ + floppy_send(CMD_RELATIVE_SEEK_IN); + floppy_send(head << 2 | drive); + g_assert(!get_irq(FLOPPY_IRQ)); + floppy_send(cyl); + + ret = ack_irq(); + g_assert(ret == 1); + + /* Send relative seek to decrease track by 1 */ + floppy_send(CMD_RELATIVE_SEEK_OUT); + floppy_send(head << 2 | drive); + g_assert(!get_irq(FLOPPY_IRQ)); + floppy_send(cyl); + + ret = ack_irq(); + g_assert(ret == 0); +} + /* success if no crash or abort */ static void fuzz_registers(void) { @@ -329,6 +364,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/read_without_media", test_read_without_media); qtest_add_func("/fdc/media_change", test_media_change); qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt); + qtest_add_func("/fdc/relative_seek", test_relative_seek); qtest_add_func("/fdc/fuzz-registers", fuzz_registers); ret = g_test_run(); From c3cdc1b0ff84d1cfed0c8052d2c83f8ecbf24166 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 16 Jul 2012 16:06:56 +0200 Subject: [PATCH 41/41] fdc-test: Clean up a bit Readability of the test code has suffered as the test case evolved. This should improve it a bit again. Signed-off-by: Kevin Wolf --- tests/fdc-test.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 10d11a4173..fa7441110d 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -93,17 +93,21 @@ static uint8_t floppy_recv(void) return inb(FLOPPY_BASE + reg_fifo); } -static uint8_t ack_irq(void) +/* pcn: Present Cylinder Number */ +static void ack_irq(uint8_t *pcn) { uint8_t ret; g_assert(get_irq(FLOPPY_IRQ)); floppy_send(CMD_SENSE_INT); floppy_recv(); - ret = floppy_recv(); - g_assert(!get_irq(FLOPPY_IRQ)); - return ret; + ret = floppy_recv(); + if (pcn != NULL) { + *pcn = ret; + } + + g_assert(!get_irq(FLOPPY_IRQ)); } static uint8_t send_read_command(void) @@ -162,7 +166,7 @@ static uint8_t send_read_command(void) return ret; } -static void send_step_pulse(int cyl) +static void send_seek(int cyl) { int drive = 0; int head = 0; @@ -171,7 +175,7 @@ static void send_step_pulse(int cyl) floppy_send(head << 2 | drive); g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); - ack_irq(); + ack_irq(NULL); } static uint8_t cmos_read(uint8_t reg) @@ -198,7 +202,7 @@ static void test_no_media_on_start(void) assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - send_step_pulse(1); + send_seek(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -229,14 +233,14 @@ static void test_media_change(void) dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - send_step_pulse(0); + send_seek(0); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); /* Step to next track should clear DSKCHG bit. */ - send_step_pulse(1); + send_seek(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_clear(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -252,13 +256,13 @@ static void test_media_change(void) dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - send_step_pulse(0); + send_seek(0); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - send_step_pulse(1); + send_seek(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -292,10 +296,10 @@ static void test_relative_seek(void) uint8_t drive = 0; uint8_t head = 0; uint8_t cyl = 1; - uint8_t ret; + uint8_t pcn; /* Send seek to track 0 */ - send_step_pulse(0); + send_seek(0); /* Send relative seek to increase track by 1 */ floppy_send(CMD_RELATIVE_SEEK_IN); @@ -303,8 +307,8 @@ static void test_relative_seek(void) g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); - ret = ack_irq(); - g_assert(ret == 1); + ack_irq(&pcn); + g_assert(pcn == 1); /* Send relative seek to decrease track by 1 */ floppy_send(CMD_RELATIVE_SEEK_OUT); @@ -312,8 +316,8 @@ static void test_relative_seek(void) g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); - ret = ack_irq(); - g_assert(ret == 0); + ack_irq(&pcn); + g_assert(pcn == 0); } /* success if no crash or abort */