Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

fsi: occ: Improve response status checking

If the driver sequence number coincidentally equals the previous
command response sequence number, the driver may proceed with
fetching the entire buffer before the OCC has processed the current
command. To be sure the correct response is obtained, check the
command type and also retry if any of the response parameters have
changed when the rest of the buffer is fetched. Also initialize the
driver with a random sequence number in order to reduce the chances
of this happening.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/20220208152235.19686-1-eajames@linux.ibm.com
Signed-off-by: Joel Stanley <joel@jms.id.au>

authored by

Eddie James and committed by
Joel Stanley
3dcf3c84 ab1b7915

+56 -31
+56 -31
drivers/fsi/fsi-occ.c
··· 451 451 return rc; 452 452 } 453 453 454 + static bool fsi_occ_response_not_ready(struct occ_response *resp, u8 seq_no, 455 + u8 cmd_type) 456 + { 457 + return resp->return_status == OCC_RESP_CMD_IN_PRG || 458 + resp->return_status == OCC_RESP_CRIT_INIT || 459 + resp->seq_no != seq_no || resp->cmd_type != cmd_type; 460 + } 461 + 454 462 int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, 455 463 void *response, size_t *resp_len) 456 464 { ··· 469 461 struct occ_response *resp = response; 470 462 size_t user_resp_len = *resp_len; 471 463 u8 seq_no; 464 + u8 cmd_type; 472 465 u16 checksum = 0; 473 466 u16 resp_data_length; 474 467 const u8 *byte_request = (const u8 *)request; 475 - unsigned long start; 468 + unsigned long end; 476 469 int rc; 477 470 size_t i; 478 471 ··· 486 477 dev_dbg(dev, "Bad resplen %zd\n", user_resp_len); 487 478 return -EINVAL; 488 479 } 480 + 481 + cmd_type = byte_request[1]; 489 482 490 483 /* Checksum the request, ignoring first byte (sequence number). */ 491 484 for (i = 1; i < req_len - 2; ++i) ··· 520 509 if (rc) 521 510 goto done; 522 511 523 - /* Read occ response header */ 524 - start = jiffies; 525 - do { 512 + end = jiffies + timeout; 513 + while (true) { 514 + /* Read occ response header */ 526 515 rc = occ_getsram(occ, 0, resp, 8); 527 516 if (rc) 528 517 goto done; 529 518 530 - if (resp->return_status == OCC_RESP_CMD_IN_PRG || 531 - resp->return_status == OCC_RESP_CRIT_INIT || 532 - resp->seq_no != seq_no) { 533 - rc = -ETIMEDOUT; 534 - 535 - if (time_after(jiffies, start + timeout)) { 536 - dev_err(occ->dev, "resp timeout status=%02x " 537 - "resp seq_no=%d our seq_no=%d\n", 519 + if (fsi_occ_response_not_ready(resp, seq_no, cmd_type)) { 520 + if (time_after(jiffies, end)) { 521 + dev_err(occ->dev, 522 + "resp timeout status=%02x seq=%d cmd=%d, our seq=%d cmd=%d\n", 538 523 resp->return_status, resp->seq_no, 539 - seq_no); 524 + resp->cmd_type, seq_no, cmd_type); 525 + rc = -ETIMEDOUT; 540 526 goto done; 541 527 } 542 528 543 529 set_current_state(TASK_UNINTERRUPTIBLE); 544 530 schedule_timeout(wait_time); 531 + } else { 532 + /* Extract size of response data */ 533 + resp_data_length = 534 + get_unaligned_be16(&resp->data_length); 535 + 536 + /* 537 + * Message size is data length + 5 bytes header + 2 538 + * bytes checksum 539 + */ 540 + if ((resp_data_length + 7) > user_resp_len) { 541 + rc = -EMSGSIZE; 542 + goto done; 543 + } 544 + 545 + /* 546 + * Get the entire response including the header again, 547 + * in case it changed 548 + */ 549 + if (resp_data_length > 1) { 550 + rc = occ_getsram(occ, 0, resp, 551 + resp_data_length + 7); 552 + if (rc) 553 + goto done; 554 + 555 + if (!fsi_occ_response_not_ready(resp, seq_no, 556 + cmd_type)) 557 + break; 558 + } else { 559 + break; 560 + } 545 561 } 546 - } while (rc); 547 - 548 - /* Extract size of response data */ 549 - resp_data_length = get_unaligned_be16(&resp->data_length); 550 - 551 - /* Message size is data length + 5 bytes header + 2 bytes checksum */ 552 - if ((resp_data_length + 7) > user_resp_len) { 553 - rc = -EMSGSIZE; 554 - goto done; 555 562 } 556 563 557 564 dev_dbg(dev, "resp_status=%02x resp_data_len=%d\n", 558 565 resp->return_status, resp_data_length); 559 - 560 - /* Grab the rest */ 561 - if (resp_data_length > 1) { 562 - /* already got 3 bytes resp, also need 2 bytes checksum */ 563 - rc = occ_getsram(occ, 8, &resp->data[3], resp_data_length - 1); 564 - if (rc) 565 - goto done; 566 - } 567 566 568 567 occ->client_response_size = resp_data_length + 7; 569 568 rc = occ_verify_checksum(occ, resp, resp_data_length); ··· 619 598 occ->version = (uintptr_t)of_device_get_match_data(dev); 620 599 occ->dev = dev; 621 600 occ->sbefifo = dev->parent; 622 - occ->sequence_number = 1; 601 + /* 602 + * Quickly derive a pseudo-random number from jiffies so that 603 + * re-probing the driver doesn't accidentally overlap sequence numbers. 604 + */ 605 + occ->sequence_number = (u8)((jiffies % 0xff) + 1); 623 606 mutex_init(&occ->occ_lock); 624 607 625 608 if (dev->of_node) {