Changeset View
Standalone View
usr.sbin/bhyve/pci_ahci.c
Show First 20 Lines • Show All 130 Lines • ▼ Show 20 Lines | struct ahci_ioreq { | ||||
uint32_t done; | uint32_t done; | ||||
int slot; | int slot; | ||||
int more; | int more; | ||||
}; | }; | ||||
struct ahci_port { | struct ahci_port { | ||||
struct blockif_ctxt *bctx; | struct blockif_ctxt *bctx; | ||||
struct pci_ahci_softc *pr_sc; | struct pci_ahci_softc *pr_sc; | ||||
struct ata_params ata_ident; | |||||
uint8_t *cmd_lst; | uint8_t *cmd_lst; | ||||
uint8_t *rfis; | uint8_t *rfis; | ||||
char ident[AHCI_PORT_IDENT]; | |||||
int port; | int port; | ||||
int atapi; | int atapi; | ||||
int reset; | int reset; | ||||
int waitforclear; | int waitforclear; | ||||
int mult_sectors; | int mult_sectors; | ||||
uint8_t xfermode; | uint8_t xfermode; | ||||
uint8_t err_cfis[20]; | uint8_t err_cfis[20]; | ||||
uint8_t sense_key; | uint8_t sense_key; | ||||
▲ Show 20 Lines • Show All 828 Lines • ▼ Show 20 Lines | |||||
{ | { | ||||
struct ahci_cmd_hdr *hdr; | struct ahci_cmd_hdr *hdr; | ||||
hdr = (struct ahci_cmd_hdr *)(p->cmd_lst + slot * AHCI_CL_SIZE); | hdr = (struct ahci_cmd_hdr *)(p->cmd_lst + slot * AHCI_CL_SIZE); | ||||
if (p->atapi || hdr->prdtl == 0) { | if (p->atapi || hdr->prdtl == 0) { | ||||
ahci_write_fis_d2h(p, slot, cfis, | ahci_write_fis_d2h(p, slot, cfis, | ||||
(ATA_E_ABORT << 8) | ATA_S_READY | ATA_S_ERROR); | (ATA_E_ABORT << 8) | ATA_S_READY | ATA_S_ERROR); | ||||
} else { | } else { | ||||
uint16_t buf[256]; | struct ata_params *ata_ident = &p->ata_ident; | ||||
ahci_checksum((uint8_t*)(ata_ident), sizeof(struct ata_params)); | |||||
jhb: I think the ahci_checksum()'s can move into ata_identify_init()? | |||||
Done Inline ActionsThe previous code logic is putting ahci_checksum() here. so I think is better leave as it is. wanpengqian_gmail.com: The previous code logic is putting ahci_checksum() here. so I think is better leave as it is.
I… | |||||
Done Inline ActionsI guess I view this as part of generating p->ata_idents value. The checksum won't change since the value of p->ata_ident is constant once ata_identify_init completes. This seems consistent my earlier request of generating p->ata_ident once instead of on each identify command. jhb: I guess I view this as part of generating `p->ata_ident`s value. The checksum won't change… | |||||
ahci_write_fis_piosetup(p); | |||||
write_prdt(p, slot, cfis, (void*)ata_ident, sizeof(struct ata_params)); | |||||
Done Inline ActionsYou could then probably just use &p->ata_ident here directly without need for the ata_ident local variable. jhb: You could then probably just use `&p->ata_ident` here directly without need for the `ata_ident`… | |||||
Done Inline ActionsI just thinking create a local variable here will make code more readable. wanpengqian_gmail.com: I just thinking create a local variable here will make code more readable.
May I know why? | |||||
Not Done Inline ActionsIf it's only for a single use, then I'm not sure it helps. You also don't really need the cast anymore, plus the existing version has a style bug in not having a blank line between the variable declaration and the code and removing the variable seems the simpler fix for that. That is all conditional on moving ahci_checksum however as it is only that change that changes the variable to being single-use. jhb: If it's only for a single use, then I'm not sure it helps. You also don't really need the cast… | |||||
Done Inline ActionsYes, now I understand your meaning. thanks. wanpengqian_gmail.com: Yes, now I understand your meaning. thanks. | |||||
ahci_write_fis_d2h(p, slot, cfis, ATA_S_DSC | ATA_S_READY); | |||||
} | |||||
} | |||||
static void | |||||
ata_identify_init(struct ahci_port* p, int atapi) | |||||
{ | |||||
struct ata_params* ata_ident = &p->ata_ident; | |||||
if (atapi) { | |||||
ata_ident->config = (2 << 14 | 5 << 8 | 1 << 7 | 2 << 5); | |||||
ata_ident->capabilities1 = ATA_SUPPORT_LBA | | |||||
ATA_SUPPORT_DMA; | |||||
ata_ident->capabilities2 = (1 << 14 | 1); | |||||
ata_ident->atavalid = ATA_FLAG_54_58 | ATA_FLAG_64_70; | |||||
ata_ident->obsolete62 = 0x3f; | |||||
ata_ident->mwdmamodes = 7; | |||||
if (p->xfermode & ATA_WDMA0) | |||||
ata_ident->mwdmamodes |= (1 << ((p->xfermode & 7) + 8)); | |||||
ata_ident->apiomodes = 3; | |||||
ata_ident->mwdmamin = 0x0078; | |||||
ata_ident->mwdmarec = 0x0078; | |||||
ata_ident->pioblind = 0x0078; | |||||
ata_ident->pioiordy = 0x0078; | |||||
ata_ident->satacapabilities = (ATA_SATA_GEN1 | ATA_SATA_GEN2 | ATA_SATA_GEN3); | |||||
ata_ident->satacapabilities2 = ((p->ssts & ATA_SS_SPD_MASK) >> 3); | |||||
ata_ident->satasupport = ATA_SUPPORT_NCQ_STREAM; | |||||
ata_ident->version_major = 0x3f0; | |||||
ata_ident->support.command1 = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_PACKET | | |||||
ATA_SUPPORT_RESET | ATA_SUPPORT_NOP); | |||||
ata_ident->support.command2 = (1 << 14); | |||||
ata_ident->support.extension = (1 << 14); | |||||
ata_ident->enabled.command1 = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_PACKET | | |||||
ATA_SUPPORT_RESET | ATA_SUPPORT_NOP); | |||||
ata_ident->enabled.extension = (1 << 14); | |||||
ata_ident->udmamodes = 0x7f; | |||||
if (p->xfermode & ATA_UDMA0) | |||||
ata_ident->udmamodes |= (1 << ((p->xfermode & 7) + 8)); | |||||
ata_ident->transport_major = 0x1020; | |||||
ata_ident->integrity = 0x00a5; | |||||
} | |||||
else { | |||||
Done Inline ActionsSo I think if we are going to store this structure, we should just initialize it once when the port is created rather than redoing it each time? Either that or we should keep 'ata_ident' on the stack I think. jhb: So I think if we are going to store this structure, we should just initialize it once when the… | |||||
Done Inline ActionsYes, at first I want to do that, But it require a little more modification. wanpengqian_gmail.com: Yes, at first I want to do that, But it require a little more modification.
Now I am creating a… | |||||
Done Inline ActionsTiny style nit: please put the else { on the same line as the previous { here and in a few other places late run the patch. jhb: Tiny style nit: please put the `else {` on the same line as the previous `{` here and in a few… | |||||
uint64_t sectors; | uint64_t sectors; | ||||
int sectsz, psectsz, psectoff, candelete, ro; | int sectsz, psectsz, psectoff, candelete, ro; | ||||
uint16_t cyl; | uint16_t cyl; | ||||
uint8_t sech, heads; | uint8_t sech, heads; | ||||
ro = blockif_is_ro(p->bctx); | ro = blockif_is_ro(p->bctx); | ||||
candelete = blockif_candelete(p->bctx); | candelete = blockif_candelete(p->bctx); | ||||
sectsz = blockif_sectsz(p->bctx); | sectsz = blockif_sectsz(p->bctx); | ||||
sectors = blockif_size(p->bctx) / sectsz; | sectors = blockif_size(p->bctx) / sectsz; | ||||
blockif_chs(p->bctx, &cyl, &heads, &sech); | blockif_chs(p->bctx, &cyl, &heads, &sech); | ||||
blockif_psectsz(p->bctx, &psectsz, &psectoff); | blockif_psectsz(p->bctx, &psectsz, &psectoff); | ||||
memset(buf, 0, sizeof(buf)); | ata_ident->config = ATA_DRQ_FAST; | ||||
buf[0] = 0x0040; | ata_ident->cylinders = cyl; | ||||
buf[1] = cyl; | ata_ident->heads = heads; | ||||
buf[3] = heads; | ata_ident->sectors = sech; | ||||
buf[6] = sech; | |||||
ata_string((uint8_t *)(buf+10), p->ident, 20); | ata_ident->sectors_intr = (0x8000 | 128); | ||||
Done Inline ActionsNit: space before = jhb: Nit: space before `=` | |||||
Done Inline Actionsdone. wanpengqian_gmail.com: done. | |||||
ata_string((uint8_t *)(buf+23), "001", 8); | ata_ident->tcg = 0; | ||||
ata_string((uint8_t *)(buf+27), "BHYVE SATA DISK", 40); | |||||
buf[47] = (0x8000 | 128); | ata_ident->capabilities1 = ATA_SUPPORT_DMA | | ||||
buf[48] = 0; | ATA_SUPPORT_LBA | ATA_SUPPORT_IORDY; | ||||
buf[49] = (1 << 8 | 1 << 9 | 1 << 11); | ata_ident->capabilities2 = (1 << 14); | ||||
buf[50] = (1 << 14); | ata_ident->atavalid = ATA_FLAG_54_58 | | ||||
buf[53] = (1 << 1 | 1 << 2); | ATA_FLAG_64_70; | ||||
if (p->mult_sectors) | if (p->mult_sectors) | ||||
buf[59] = (0x100 | p->mult_sectors); | ata_ident->multi = (ATA_MULTI_VALID | p->mult_sectors); | ||||
if (sectors <= 0x0fffffff) { | if (sectors <= 0x0fffffff) { | ||||
buf[60] = sectors; | ata_ident->lba_size_1 = sectors; | ||||
buf[61] = (sectors >> 16); | ata_ident->lba_size_2 = (sectors >> 16); | ||||
} else { | } else { | ||||
buf[60] = 0xffff; | ata_ident->lba_size_1 = 0xffff; | ||||
buf[61] = 0x0fff; | ata_ident->lba_size_2 = 0x0fff; | ||||
} | } | ||||
buf[63] = 0x7; | ata_ident->mwdmamodes = 0x7; | ||||
if (p->xfermode & ATA_WDMA0) | if (p->xfermode & ATA_WDMA0) | ||||
buf[63] |= (1 << ((p->xfermode & 7) + 8)); | ata_ident->mwdmamodes |= (1 << ((p->xfermode & 7) + 8)); | ||||
buf[64] = 0x3; | ata_ident->apiomodes = 0x3; | ||||
buf[65] = 120; | ata_ident->mwdmamin = 0x0078; | ||||
buf[66] = 120; | ata_ident->mwdmarec = 0x0078; | ||||
buf[67] = 120; | ata_ident->pioblind = 0x0078; | ||||
buf[68] = 120; | ata_ident->pioiordy = 0x0078; | ||||
buf[69] = 0; | ata_ident->support3 = 0; | ||||
buf[75] = 31; | ata_ident->queue = 31; | ||||
buf[76] = (ATA_SATA_GEN1 | ATA_SATA_GEN2 | ATA_SATA_GEN3 | | ata_ident->satacapabilities = (ATA_SATA_GEN1 | ATA_SATA_GEN2 | ATA_SATA_GEN3 | | ||||
ATA_SUPPORT_NCQ); | ATA_SUPPORT_NCQ); | ||||
buf[77] = (ATA_SUPPORT_RCVSND_FPDMA_QUEUED | | ata_ident->satacapabilities2 = (ATA_SUPPORT_RCVSND_FPDMA_QUEUED | | ||||
(p->ssts & ATA_SS_SPD_MASK) >> 3); | (p->ssts & ATA_SS_SPD_MASK) >> 3); | ||||
buf[80] = 0x3f0; | ata_ident->version_major = 0x3f0; | ||||
buf[81] = 0x28; | ata_ident->version_minor = 0x28; | ||||
buf[82] = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_WRITECACHE| | ata_ident->support.command1 = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_WRITECACHE | | ||||
ATA_SUPPORT_LOOKAHEAD | ATA_SUPPORT_NOP); | ATA_SUPPORT_LOOKAHEAD | ATA_SUPPORT_NOP); | ||||
buf[83] = (ATA_SUPPORT_ADDRESS48 | ATA_SUPPORT_FLUSHCACHE | | ata_ident->support.command2 = (ATA_SUPPORT_ADDRESS48 | ATA_SUPPORT_FLUSHCACHE | | ||||
ATA_SUPPORT_FLUSHCACHE48 | 1 << 14); | ATA_SUPPORT_FLUSHCACHE48 | 1 << 14); | ||||
buf[84] = (1 << 14); | ata_ident->support.extension = (1 << 14); | ||||
buf[85] = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_WRITECACHE| | ata_ident->enabled.command1 = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_WRITECACHE | | ||||
ATA_SUPPORT_LOOKAHEAD | ATA_SUPPORT_NOP); | ATA_SUPPORT_LOOKAHEAD | ATA_SUPPORT_NOP); | ||||
buf[86] = (ATA_SUPPORT_ADDRESS48 | ATA_SUPPORT_FLUSHCACHE | | ata_ident->enabled.command2 = (ATA_SUPPORT_ADDRESS48 | ATA_SUPPORT_FLUSHCACHE | | ||||
ATA_SUPPORT_FLUSHCACHE48 | 1 << 15); | ATA_SUPPORT_FLUSHCACHE48 | 1 << 15); | ||||
buf[87] = (1 << 14); | ata_ident->enabled.extension = (1 << 14); | ||||
buf[88] = 0x7f; | ata_ident->udmamodes = 0x7f; | ||||
if (p->xfermode & ATA_UDMA0) | if (p->xfermode & ATA_UDMA0) | ||||
buf[88] |= (1 << ((p->xfermode & 7) + 8)); | ata_ident->udmamodes |= (1 << ((p->xfermode & 7) + 8)); | ||||
buf[100] = sectors; | ata_ident->lba_size48_1 = sectors; | ||||
buf[101] = (sectors >> 16); | ata_ident->lba_size48_2 = (sectors >> 16); | ||||
buf[102] = (sectors >> 32); | ata_ident->lba_size48_3 = (sectors >> 32); | ||||
buf[103] = (sectors >> 48); | ata_ident->lba_size48_4 = (sectors >> 48); | ||||
if (candelete && !ro) { | if (candelete && !ro) { | ||||
buf[69] |= ATA_SUPPORT_RZAT | ATA_SUPPORT_DRAT; | ata_ident->support3 |= ATA_SUPPORT_RZAT | ATA_SUPPORT_DRAT; | ||||
buf[105] = 1; | ata_ident->max_dsm_blocks = 1; | ||||
buf[169] = ATA_SUPPORT_DSM_TRIM; | ata_ident->support_dsm = ATA_SUPPORT_DSM_TRIM; | ||||
} | } | ||||
buf[106] = 0x4000; | ata_ident->pss = ATA_PSS_VALID_VALUE; | ||||
buf[209] = 0x4000; | ata_ident->lsalign = 0x4000; | ||||
if (psectsz > sectsz) { | if (psectsz > sectsz) { | ||||
buf[106] |= 0x2000; | ata_ident->pss |= ATA_PSS_MULTLS; | ||||
buf[106] |= ffsl(psectsz / sectsz) - 1; | ata_ident->pss |= ffsl(psectsz / sectsz) - 1; | ||||
buf[209] |= (psectoff / sectsz); | ata_ident->lsalign |= (psectoff / sectsz); | ||||
} | } | ||||
if (sectsz > 512) { | if (sectsz > 512) { | ||||
buf[106] |= 0x1000; | ata_ident->pss |= ATA_PSS_LSSABOVE512; | ||||
buf[117] = sectsz / 2; | ata_ident->lss_1 = sectsz / 2; | ||||
buf[118] = ((sectsz / 2) >> 16); | ata_ident->lss_2 = ((sectsz / 2) >> 16); | ||||
} | } | ||||
buf[119] = (ATA_SUPPORT_RWLOGDMAEXT | 1 << 14); | ata_ident->support2 = (ATA_SUPPORT_RWLOGDMAEXT | 1 << 14); | ||||
buf[120] = (ATA_SUPPORT_RWLOGDMAEXT | 1 << 14); | ata_ident->enabled2 = (ATA_SUPPORT_RWLOGDMAEXT | 1 << 14); | ||||
buf[222] = 0x1020; | ata_ident->transport_major = 0x1020; | ||||
buf[255] = 0x00a5; | ata_ident->integrity = 0x00a5; | ||||
ahci_checksum((uint8_t *)buf, sizeof(buf)); | |||||
ahci_write_fis_piosetup(p); | |||||
write_prdt(p, slot, cfis, (void *)buf, sizeof(buf)); | |||||
ahci_write_fis_d2h(p, slot, cfis, ATA_S_DSC | ATA_S_READY); | |||||
} | } | ||||
} | } | ||||
static void | static void | ||||
handle_atapi_identify(struct ahci_port *p, int slot, uint8_t *cfis) | handle_atapi_identify(struct ahci_port *p, int slot, uint8_t *cfis) | ||||
{ | { | ||||
if (!p->atapi) { | if (!p->atapi) { | ||||
ahci_write_fis_d2h(p, slot, cfis, | ahci_write_fis_d2h(p, slot, cfis, | ||||
(ATA_E_ABORT << 8) | ATA_S_READY | ATA_S_ERROR); | (ATA_E_ABORT << 8) | ATA_S_READY | ATA_S_ERROR); | ||||
} else { | } else { | ||||
uint16_t buf[256]; | struct ata_params *ata_ident = &p->ata_ident; | ||||
ahci_checksum((uint8_t *)(ata_ident), sizeof(struct ata_params)); | |||||
Done Inline ActionsSame thoughts here about ahci_checksum and dropping ata_ident local variable. jhb: Same thoughts here about `ahci_checksum` and dropping `ata_ident` local variable. | |||||
memset(buf, 0, sizeof(buf)); | |||||
buf[0] = (2 << 14 | 5 << 8 | 1 << 7 | 2 << 5); | |||||
ata_string((uint8_t *)(buf+10), p->ident, 20); | |||||
ata_string((uint8_t *)(buf+23), "001", 8); | |||||
ata_string((uint8_t *)(buf+27), "BHYVE SATA DVD ROM", 40); | |||||
buf[49] = (1 << 9 | 1 << 8); | |||||
buf[50] = (1 << 14 | 1); | |||||
buf[53] = (1 << 2 | 1 << 1); | |||||
buf[62] = 0x3f; | |||||
buf[63] = 7; | |||||
if (p->xfermode & ATA_WDMA0) | |||||
buf[63] |= (1 << ((p->xfermode & 7) + 8)); | |||||
buf[64] = 3; | |||||
buf[65] = 120; | |||||
buf[66] = 120; | |||||
buf[67] = 120; | |||||
buf[68] = 120; | |||||
buf[76] = (ATA_SATA_GEN1 | ATA_SATA_GEN2 | ATA_SATA_GEN3); | |||||
buf[77] = ((p->ssts & ATA_SS_SPD_MASK) >> 3); | |||||
buf[78] = (1 << 5); | |||||
buf[80] = 0x3f0; | |||||
buf[82] = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_PACKET | | |||||
ATA_SUPPORT_RESET | ATA_SUPPORT_NOP); | |||||
buf[83] = (1 << 14); | |||||
buf[84] = (1 << 14); | |||||
buf[85] = (ATA_SUPPORT_POWERMGT | ATA_SUPPORT_PACKET | | |||||
ATA_SUPPORT_RESET | ATA_SUPPORT_NOP); | |||||
buf[87] = (1 << 14); | |||||
buf[88] = 0x7f; | |||||
if (p->xfermode & ATA_UDMA0) | |||||
buf[88] |= (1 << ((p->xfermode & 7) + 8)); | |||||
buf[222] = 0x1020; | |||||
buf[255] = 0x00a5; | |||||
ahci_checksum((uint8_t *)buf, sizeof(buf)); | |||||
ahci_write_fis_piosetup(p); | ahci_write_fis_piosetup(p); | ||||
write_prdt(p, slot, cfis, (void *)buf, sizeof(buf)); | write_prdt(p, slot, cfis, (void *)ata_ident, sizeof(struct ata_params)); | ||||
ahci_write_fis_d2h(p, slot, cfis, ATA_S_DSC | ATA_S_READY); | ahci_write_fis_d2h(p, slot, cfis, ATA_S_DSC | ATA_S_READY); | ||||
} | } | ||||
} | } | ||||
static void | static void | ||||
atapi_inquiry(struct ahci_port *p, int slot, uint8_t *cfis) | atapi_inquiry(struct ahci_port *p, int slot, uint8_t *cfis) | ||||
{ | { | ||||
uint8_t buf[36]; | uint8_t buf[36]; | ||||
▲ Show 20 Lines • Show All 1,175 Lines • ▼ Show 20 Lines | |||||
{ | { | ||||
char bident[sizeof("XX:XX:XX")]; | char bident[sizeof("XX:XX:XX")]; | ||||
struct blockif_ctxt *bctxt; | struct blockif_ctxt *bctxt; | ||||
struct pci_ahci_softc *sc; | struct pci_ahci_softc *sc; | ||||
int ret, slots, p; | int ret, slots, p; | ||||
MD5_CTX mdctx; | MD5_CTX mdctx; | ||||
u_char digest[16]; | u_char digest[16]; | ||||
char *next, *next2; | char *next, *next2; | ||||
char *bopt, *uopt, *xopts, *config; | |||||
FILE* fp; | |||||
size_t block_len; | |||||
int comma, optpos; | |||||
ret = 0; | ret = 0; | ||||
#ifdef AHCI_DEBUG | #ifdef AHCI_DEBUG | ||||
dbg = fopen("/tmp/log", "w+"); | dbg = fopen("/tmp/log", "w+"); | ||||
#endif | #endif | ||||
sc = calloc(1, sizeof(struct pci_ahci_softc)); | sc = calloc(1, sizeof(struct pci_ahci_softc)); | ||||
pi->pi_arg = sc; | pi->pi_arg = sc; | ||||
sc->asc_pi = pi; | sc->asc_pi = pi; | ||||
pthread_mutex_init(&sc->mtx, NULL); | pthread_mutex_init(&sc->mtx, NULL); | ||||
sc->ports = 0; | sc->ports = 0; | ||||
sc->pi = 0; | sc->pi = 0; | ||||
slots = 32; | slots = 32; | ||||
for (p = 0; p < MAX_PORTS && opts != NULL; p++, opts = next) { | for (p = 0; p < MAX_PORTS && opts != NULL; p++, opts = next) { | ||||
struct ata_params *ata_ident = &sc->port[p].ata_ident; | |||||
Done Inline ActionsYou don't need the cast to void * for C. Also, leave a blank line before the following comment. jhb: You don't need the cast to `void *` for C. Also, leave a blank line before the following… | |||||
Done Inline ActionsI have corrected it. wanpengqian_gmail.com: I have corrected it. | |||||
memset(ata_ident, 0, sizeof(struct ata_params)); | |||||
/* Identify and cut off type of present port. */ | /* Identify and cut off type of present port. */ | ||||
if (strncmp(opts, "hd:", 3) == 0) { | if (strncmp(opts, "hd:", 3) == 0) { | ||||
atapi = 0; | atapi = 0; | ||||
opts += 3; | opts += 3; | ||||
} else if (strncmp(opts, "cd:", 3) == 0) { | } else if (strncmp(opts, "cd:", 3) == 0) { | ||||
atapi = 1; | atapi = 1; | ||||
opts += 3; | opts += 3; | ||||
} | } | ||||
/* Find and cut off the next port options. */ | /* Find and cut off the next port options. */ | ||||
next = strstr(opts, ",hd:"); | next = strstr(opts, ",hd:"); | ||||
next2 = strstr(opts, ",cd:"); | next2 = strstr(opts, ",cd:"); | ||||
if (next == NULL || (next2 != NULL && next2 < next)) | if (next == NULL || (next2 != NULL && next2 < next)) | ||||
next = next2; | next = next2; | ||||
if (next != NULL) { | if (next != NULL) { | ||||
next[0] = 0; | next[0] = 0; | ||||
next++; | next++; | ||||
} | } | ||||
if (opts[0] == 0) | if (opts[0] == 0) | ||||
continue; | continue; | ||||
uopt = strdup(opts); | |||||
bopt = NULL; | |||||
Done Inline ActionsEh, you have passed the wrong size to calloc, and you still did a memset after the calloc? However, it is also a bit messier to try to do all this by hand. I would use a string builder that already does the memory management for you, either libsbuf or open_memstream(3), for example: FILE *fp; char *block_opts; size_t block_len; int comma; ... opt = strdup(opts); block_opts = NULL; fp = open_memstream(&block_opts, &block_len); for (opts = strtok(uopt, ","); xopts != NULL; xopts = strtok(NULL, ",")) { if ((config = strchr(xopts, "=")) != NULL) *config++ = '\0'; if (!strcmp("nmrr", xopts) { ... } else { /* Pass all other options to blockif_open. */ if (config != NULL) *--config = '='; fprintf(fp, "%s%s", comma ? "," : "", xopts); comma = 1; } } free(uopts); fclose(fp); jhb: Eh, you have passed the wrong size to calloc, and you still did a memset after the calloc? | |||||
Done Inline ActionsFixed with this suggestion. wanpengqian_gmail.com: Fixed with this suggestion. | |||||
fp = open_memstream(&bopt, &block_len); | |||||
comma = 0; | |||||
optpos = 0; | |||||
for (xopts = strtok(uopt, ","); | |||||
xopts != NULL; | |||||
xopts = strtok(NULL, ",")) { | |||||
// First option assume as block filename. | |||||
Done Inline ActionsI have a hard time parsing the first bit of this comment? sjorge_ml_blackdot.be: I have a hard time parsing the first bit of this comment?
We process only things we can use and… | |||||
Done Inline ActionsYes, for example, commandline is wanpengqian_gmail.com: Yes, for example, commandline is
-s 4:0,ahci,hd:/zones/vm/ahci0/disk0.img,nmrr=7200… | |||||
Done Inline ActionsPerhaps reword the comment to something like: sjorge_ml_blackdot.be: Perhaps reword the comment to something like:
`Consume arguments valid for ahci, pass others to… | |||||
Done Inline ActionsThanks, I will correct it next update. wanpengqian_gmail.com: Thanks, I will correct it next update. | |||||
Done Inline ActionsPlease use an older C-style comment (/* .. */) rather than //. jhb: Please use an older C-style comment (/* .. */) rather than //. | |||||
if (optpos == 0) { | |||||
/* | /* | ||||
* Create an identifier for the backing file. | |||||
* Use parts of the md5 sum of the filename | |||||
*/ | |||||
char ident[AHCI_PORT_IDENT]; | |||||
MD5Init(&mdctx); | |||||
MD5Update(&mdctx, opts, strlen(opts)); | |||||
MD5Final(digest, &mdctx); | |||||
snprintf(ident, AHCI_PORT_IDENT, | |||||
"BHYVE-%02X%02X-%02X%02X-%02X%02X", | |||||
digest[0], digest[1], digest[2], digest[3], digest[4], | |||||
digest[5]); | |||||
ata_string((uint8_t*)&ata_ident->serial, ident, 20); | |||||
ata_string((uint8_t*)&ata_ident->revision, "001", 8); | |||||
if (atapi) { | |||||
ata_string((uint8_t*)&ata_ident->model, "BHYVE SATA DVD ROM", 40); | |||||
} | |||||
else { | |||||
ata_string((uint8_t*)&ata_ident->model, "BHYVE SATA DISK", 40); | |||||
} | |||||
} | |||||
if ((config = strchr(xopts, '=')) != NULL) { | |||||
*config++ = '\0'; | |||||
if (!strcmp("nmrr", xopts)) { | |||||
ata_ident->media_rotation_rate = atoi(config); | |||||
} | |||||
else if (!strcmp("ser", xopts)) { | |||||
ata_string((uint8_t*)(&ata_ident->serial), config, 20); | |||||
} | |||||
else if (!strcmp("rev", xopts)) { | |||||
ata_string((uint8_t*)(&ata_ident->revision), config, 8); | |||||
} | |||||
else if (!strcmp("model", xopts)) { | |||||
ata_string((uint8_t*)(&ata_ident->model), config, 40); | |||||
} | |||||
else { | |||||
/* Pass all other options to blockif_open. */ | |||||
*--config = '='; | |||||
fprintf(fp, "%s%s", comma ? "," : "", xopts); | |||||
comma = 1; | |||||
} | |||||
} | |||||
else { | |||||
/* Pass all other options to blockif_open. */ | |||||
fprintf(fp, "%s%s", comma ? "," : "", xopts); | |||||
comma = 1; | |||||
} | |||||
optpos++; | |||||
} | |||||
free(uopt); | |||||
fclose(fp); | |||||
DPRINTF("%s\n", bopt); | |||||
/* | |||||
* Attempt to open the backing image. Use the PCI slot/func | * Attempt to open the backing image. Use the PCI slot/func | ||||
* and the port number for the identifier string. | * and the port number for the identifier string. | ||||
*/ | */ | ||||
snprintf(bident, sizeof(bident), "%d:%d:%d", pi->pi_slot, | snprintf(bident, sizeof(bident), "%d:%d:%d", pi->pi_slot, | ||||
pi->pi_func, p); | pi->pi_func, p); | ||||
bctxt = blockif_open(opts, bident); | bctxt = blockif_open(bopt, bident); | ||||
free(bopt); | |||||
if (bctxt == NULL) { | if (bctxt == NULL) { | ||||
sc->ports = p; | sc->ports = p; | ||||
ret = 1; | ret = 1; | ||||
goto open_fail; | goto open_fail; | ||||
} | } | ||||
sc->port[p].bctx = bctxt; | sc->port[p].bctx = bctxt; | ||||
sc->port[p].pr_sc = sc; | sc->port[p].pr_sc = sc; | ||||
sc->port[p].port = p; | sc->port[p].port = p; | ||||
sc->port[p].atapi = atapi; | sc->port[p].atapi = atapi; | ||||
/* | ata_identify_init(&sc->port[p], atapi); | ||||
Done Inline ActionsSo what I would do instead is to set these initial values up before you parse options and then let the options override them perhaps instead of checking this. Also, space after if. jhb: So what I would do instead is to set these initial values up before you parse options and then… | |||||
Done Inline ActionsAlright. I try my best to do as you said. wanpengqian_gmail.com: Alright. I try my best to do as you said.
But the serial number require block filename.
So the… | |||||
* Create an identifier for the backing file. | |||||
* Use parts of the md5 sum of the filename | |||||
*/ | |||||
MD5Init(&mdctx); | |||||
MD5Update(&mdctx, opts, strlen(opts)); | |||||
MD5Final(digest, &mdctx); | |||||
snprintf(sc->port[p].ident, AHCI_PORT_IDENT, | |||||
"BHYVE-%02X%02X-%02X%02X-%02X%02X", | |||||
digest[0], digest[1], digest[2], digest[3], digest[4], | |||||
digest[5]); | |||||
/* | /* | ||||
* Allocate blockif request structures and add them | * Allocate blockif request structures and add them | ||||
* to the free list | * to the free list | ||||
*/ | */ | ||||
pci_ahci_ioreq_init(&sc->port[p]); | pci_ahci_ioreq_init(&sc->port[p]); | ||||
sc->pi |= (1 << p); | sc->pi |= (1 << p); | ||||
▲ Show 20 Lines • Show All 84 Lines • Show Last 20 Lines |
I think the ahci_checksum()'s can move into ata_identify_init()?