From df1e857ac4d1410ae2bd354e361210b842ab7bc8 Mon Sep 17 00:00:00 2001 From: Minep Date: Thu, 10 Nov 2022 22:15:09 +0000 Subject: [PATCH 1/1] fix: (blkio) enforce disk io buffer size alignment (to block size) fix: (blkio) handle buffer smaller than block size fix: (ahci) handle the hba interrupt as per spec refactor: (vecbuf) a more compact interface --- lunaix-os/hal/ahci/ahci.c | 2 +- lunaix-os/hal/ahci/atapi.c | 2 +- lunaix-os/hal/ahci/io_event.c | 48 ++++++++++++++++++++++++------ lunaix-os/includes/hal/ahci/hba.h | 14 +++++++-- lunaix-os/includes/lunaix/buffer.h | 2 +- lunaix-os/kernel/block/blkio.c | 2 -- lunaix-os/kernel/block/block.c | 45 ++++++++++++++++------------ lunaix-os/kernel/ds/buffer.c | 10 ++++--- 8 files changed, 85 insertions(+), 40 deletions(-) diff --git a/lunaix-os/hal/ahci/ahci.c b/lunaix-os/hal/ahci/ahci.c index b9bca95..e07200e 100644 --- a/lunaix-os/hal/ahci/ahci.c +++ b/lunaix-os/hal/ahci/ahci.c @@ -27,7 +27,7 @@ #define HBA_FIS_SIZE 256 #define HBA_CLB_SIZE 1024 -#define HBA_MY_IE (HBA_PxINTR_DHR | HBA_PxINTR_TFEE | HBA_PxINTR_OFE) +#define HBA_MY_IE (HBA_PxINTR_DHR | HBA_PxINTR_TFE | HBA_PxINTR_OF) // #define DO_HBA_FULL_RESET diff --git a/lunaix-os/hal/ahci/atapi.c b/lunaix-os/hal/ahci/atapi.c index 26ad533..bf3e8c9 100644 --- a/lunaix-os/hal/ahci/atapi.c +++ b/lunaix-os/hal/ahci/atapi.c @@ -77,7 +77,7 @@ scsi_submit(struct hba_device* dev, struct blkio_req* io_req) } else { scsi_create_packet12((struct scsi_cdb12*)cdb, write ? SCSI_WRITE_BLOCKS_12 : SCSI_READ_BLOCKS_12, - io_req->blk_addr & -1, + io_req->blk_addr & (u32_t)-1, count); } diff --git a/lunaix-os/hal/ahci/io_event.c b/lunaix-os/hal/ahci/io_event.c index 632197c..e11016a 100644 --- a/lunaix-os/hal/ahci/io_event.c +++ b/lunaix-os/hal/ahci/io_event.c @@ -1,27 +1,54 @@ -#include +#include #include #include #include +#include -extern struct ahci_hba hba; +LOG_MODULE("io_evt") + +extern struct llist_header ahcis; void __ahci_hba_isr(const isr_param* param) { + struct ahci_hba* hba; + struct ahci_driver *pos, *n; + llist_for_each(pos, n, &ahcis, ahci_drvs) + { + if (pos->id == param->vector) { + hba = &pos->hba; + goto proceed; + } + } + + return; + +proceed: // ignore spurious interrupt - if (!hba.base[HBA_RIS]) + if (!hba->base[HBA_RIS]) return; - u32_t port_num = 31 - __builtin_clz(hba.base[HBA_RIS]); - struct hba_port* port = hba.ports[port_num]; + u32_t port_num = 31 - __builtin_clz(hba->base[HBA_RIS]); + struct hba_port* port = hba->ports[port_num]; struct hba_cmd_context* cmdctx = &port->cmdctx; - u32_t ci_filtered = port->regs[HBA_RPxCI] ^ cmdctx->tracked_ci; + u32_t processed = port->regs[HBA_RPxCI] ^ cmdctx->tracked_ci; - if (!ci_filtered) { + sata_read_error(port); + + // FIXME When error occurs, CI will not change. Need error recovery! + if (!processed) { + if (port->regs[HBA_RPxIS] & HBA_FATAL) { + // TODO perform error recovery + // This should include: + // 1. Discard all issued (but pending) requests (signaled as + // error) + // 2. Restart port + // Complete steps refer to AHCI spec 6.2.2.1 + } goto done; } - u32_t slot = 31 - __builtin_clz(ci_filtered); + u32_t slot = 31 - __builtin_clz(processed); struct hba_cmd_state* cmdstate = cmdctx->issued[slot]; if (!cmdstate) { @@ -29,17 +56,20 @@ __ahci_hba_isr(const isr_param* param) } struct blkio_req* ioreq = (struct blkio_req*)cmdstate->state_ctx; - sata_read_error(port); + if ((port->device->last_result.status & HBA_PxTFD_ERR)) { ioreq->errcode = port->regs[HBA_RPxTFD] & 0xffff; ioreq->flags |= BLKIO_ERROR; + hba_clear_reg(port->regs[HBA_RPxSERR]); } + blkio_schedule(ioreq->io_ctx); blkio_complete(ioreq); vfree(cmdstate->cmd_table); done: hba_clear_reg(port->regs[HBA_RPxIS]); + hba->base[HBA_RIS] &= ~(1 << (31 - port_num)); } void diff --git a/lunaix-os/includes/hal/ahci/hba.h b/lunaix-os/includes/hal/ahci/hba.h index 287f868..9bebe8e 100644 --- a/lunaix-os/includes/hal/ahci/hba.h +++ b/lunaix-os/includes/hal/ahci/hba.h @@ -35,13 +35,21 @@ #define HBA_PxINTR_DMA (1 << 2) #define HBA_PxINTR_DHR (1) #define HBA_PxINTR_DPS (1 << 5) -#define HBA_PxINTR_TFEE (1 << 30) -#define HBA_PxINTR_OFE (1 << 24) -#define HBA_PxINTR_IFE (1 << 27) +#define HBA_PxINTR_TFE (1 << 30) +#define HBA_PxINTR_HBF (1 << 29) +#define HBA_PxINTR_HBD (1 << 28) +#define HBA_PxINTR_IF (1 << 27) +#define HBA_PxINTR_NIF (1 << 26) +#define HBA_PxINTR_OF (1 << 24) #define HBA_PxTFD_ERR (1) #define HBA_PxTFD_BSY (1 << 7) #define HBA_PxTFD_DRQ (1 << 3) +#define HBA_FATAL \ + (HBA_PxINTR_TFE | HBA_PxINTR_HBF | HBA_PxINTR_HBD | HBA_PxINTR_IF) + +#define HBA_NONFATAL (HBA_PxINTR_NIF | HBA_PxINTR_OF) + #define HBA_RGHC_ACHI_ENABLE (1 << 31) #define HBA_RGHC_INTR_ENABLE (1 << 1) #define HBA_RGHC_RESET 1 diff --git a/lunaix-os/includes/lunaix/buffer.h b/lunaix-os/includes/lunaix/buffer.h index 851d568..07176cd 100644 --- a/lunaix-os/includes/lunaix/buffer.h +++ b/lunaix-os/includes/lunaix/buffer.h @@ -35,7 +35,7 @@ vbuf_free(struct vecbuf* vbuf); * @return struct vecbuf* */ struct vecbuf* -vbuf_alloc(struct vecbuf* vec, void* buf, size_t len); +vbuf_alloc(struct vecbuf** vec, void* buf, size_t len); static inline size_t vbuf_size(struct vecbuf* vbuf) diff --git a/lunaix-os/kernel/block/blkio.c b/lunaix-os/kernel/block/blkio.c index 7cfc354..c5d688f 100644 --- a/lunaix-os/kernel/block/blkio.c +++ b/lunaix-os/kernel/block/blkio.c @@ -118,6 +118,4 @@ blkio_complete(struct blkio_req* req) } req->io_ctx->busy--; - - blkio_schedule(req->io_ctx); } \ No newline at end of file diff --git a/lunaix-os/kernel/block/block.c b/lunaix-os/kernel/block/block.c index dbdb95a..bcbc60f 100644 --- a/lunaix-os/kernel/block/block.c +++ b/lunaix-os/kernel/block/block.c @@ -55,19 +55,22 @@ __block_read(struct device* dev, void* buf, size_t offset, size_t len) return 0; } - struct vecbuf* vbuf = vbuf_alloc(NULL, buf, len); + struct vecbuf* vbuf = NULL; struct blkio_req* req; - void* tmp_buf = NULL; + void *head_buf = NULL, *tail_buf = NULL; - if (r) { - tmp_buf = vzalloc(bsize); + // align the boundary + if (r || len < bsize) { + head_buf = valloc(bsize); rd_size = MIN(len, bsize - r); - vbuf->buf.size = bsize; - vbuf->buf.buffer = tmp_buf; + vbuf_alloc(&vbuf, head_buf, bsize); + } - if ((len - rd_size)) { - vbuf_alloc(vbuf, buf + rd_size, len - rd_size); - } + // align the length + if ((len - rd_size)) { + size_t llen = len - rd_size; + assert_msg(!(llen % bsize), "misalign block read"); + vbuf_alloc(&vbuf, buf + rd_size, llen); } req = blkio_vrd(vbuf, rd_block, NULL, NULL, 0); @@ -76,14 +79,14 @@ __block_read(struct device* dev, void* buf, size_t offset, size_t len) pwait(&req->wait); if (!(errno = req->errcode)) { - memcpy(buf, tmp_buf + r, rd_size); + memcpy(buf, head_buf + r, rd_size); errno = len; } else { errno = -errno; } - if (tmp_buf) { - vfree(tmp_buf); + if (head_buf) { + vfree(head_buf); } blkio_free_req(req); @@ -96,24 +99,28 @@ __block_write(struct device* dev, void* buf, size_t offset, size_t len) { struct block_dev* bdev = (struct block_dev*)dev->underlay; size_t bsize = bdev->blk_size, rd_block = offset / bsize + bdev->start_lba, - r = offset % bsize; + r = offset % bsize, wr_size = 0; if (!(len = MIN(len, ((size_t)bdev->end_lba - rd_block + 1) * bsize))) { return 0; } - struct vecbuf* vbuf = vbuf_alloc(NULL, buf, len); + struct vecbuf* vbuf = NULL; struct blkio_req* req; void* tmp_buf = NULL; if (r) { - size_t rd_size = MIN(len, bsize - r); + size_t wr_size = MIN(len, bsize - r); tmp_buf = vzalloc(bsize); - vbuf->buf.size = bsize; - vbuf->buf.buffer = tmp_buf; + vbuf_alloc(&vbuf, tmp_buf, bsize); + + memcpy(tmp_buf + r, buf, wr_size); + } - memcpy(tmp_buf + r, buf, rd_size); - vbuf_alloc(vbuf, buf + rd_size, len - rd_size); + if ((len - wr_size)) { + size_t llen = len - wr_size; + assert_msg(!(llen % bsize), "misalign block write"); + vbuf_alloc(&vbuf, buf + wr_size, llen); } req = blkio_vwr(vbuf, rd_block, NULL, NULL, 0); diff --git a/lunaix-os/kernel/ds/buffer.c b/lunaix-os/kernel/ds/buffer.c index 7b277c8..c40ed51 100644 --- a/lunaix-os/kernel/ds/buffer.c +++ b/lunaix-os/kernel/ds/buffer.c @@ -2,17 +2,19 @@ #include struct vecbuf* -vbuf_alloc(struct vecbuf* vec, void* buf, size_t size) +vbuf_alloc(struct vecbuf** vec, void* buf, size_t size) { struct vecbuf* vbuf = valloc(sizeof(struct vecbuf)); + struct vecbuf* _vec = *vec; *vbuf = (struct vecbuf){ .buf = { .buffer = buf, .size = size }, - .acc_sz = vbuf_size(vec) + size }; + .acc_sz = vbuf_size(_vec) + size }; - if (vec) { - llist_append(&vec->components, &vbuf->components); + if (_vec) { + llist_append(&_vec->components, &vbuf->components); } else { llist_init_head(&vbuf->components); + *vec = vbuf; } return vbuf; -- 2.27.0