* [PATCH 0/4] hw/ufs: ufs device testing function added and modified [not found] <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p4> @ 2024-08-21 2:30 ` 정유찬 [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p1> ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: 정유찬 @ 2024-08-21 2:30 UTC (permalink / raw) To: 김제욱 Cc: thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com, 정유찬 [-- Attachment #1: Type: text/html, Size: 10852 bytes --] [-- Attachment #2: Type: image/png, Size: 22957 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p1>]
* [PATCH 0/4] hw/ufs: ufs device testing function added and modified [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p1> @ 2024-08-21 2:27 ` 정유찬 2024-08-21 2:35 ` [PATCH 3/4] hw/ufs: ufs attribute read/write test implemented 정유찬 2024-08-22 1:39 ` [PATCH 4/4] hw/ufs: ufs descriptor read " Yoochan Jeong 2 siblings, 0 replies; 10+ messages in thread From: 정유찬 @ 2024-08-21 2:27 UTC (permalink / raw) To: 김제욱 Cc: thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com From 936ef0a907bcf16809f9980c2d37e8fcb13697d2 Mon Sep 17 00:00:00 2001 From: Yoochan Jeong <yc01.jeong@samsung.com> Date: Wed, 21 Aug 2024 10:45:28 +0900 Subject: [PATCH 0/4] hw/ufs: ufs device testing function added and modified Previously, it was only able to test virtual UFS devices if they properly read and write storage data. In this patch, three test functions are added to test if virtual UFS devices properly read and write its metadata. Each functions test reading and writing flags, attributes and descriptors. Related minor bugs and errors are also fixed. Yoochan Jeong (4): hw/ufs: minor bug fixes related to ufs-test hw/ufs: ufs flag read/write test implemented hw/ufs: ufs attribute read/write test implemented hw/ufs: ufs descriptor read test implemented hw/ufs/ufs.c | 26 ++- tests/qtest/ufs-test.c | 414 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 432 insertions(+), 8 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] hw/ufs: ufs attribute read/write test implemented [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p1> 2024-08-21 2:27 ` 정유찬 @ 2024-08-21 2:35 ` 정유찬 2024-08-22 1:39 ` [PATCH 4/4] hw/ufs: ufs descriptor read " Yoochan Jeong 2 siblings, 0 replies; 10+ messages in thread From: 정유찬 @ 2024-08-21 2:35 UTC (permalink / raw) To: 김제욱 Cc: 정유찬, thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com From 3341c347a70783c5e60a963501e63cb5da8f2e1f Mon Sep 17 00:00:00 2001 From: Yoochan Jeong <yc01.jeong@samsung.com> Date: Wed, 21 Aug 2024 09:09:32 +0900 Subject: [PATCH 3/4] hw/ufs: ufs attribute read/write test implemented New test function "ufstest_attr_request" added, which can check one's virtual UFS device can properly read and write its attribute data. It tests if reading and writing attributes work properly. There are some testcases that are intended to make an error caused by writing an invalid value, allocating an invalid selector and permission issues. Based on: 20240802051902epcms2p319bc095a15eaef8de4e6955f6718371d@epcms2p3 Signed-off-by: Yoochan Jeong <yc01.jeong@samsung.com> --- tests/qtest/ufs-test.c | 166 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c index f93de9f1f2..a8fd2f1acc 100644 --- a/tests/qtest/ufs-test.c +++ b/tests/qtest/ufs-test.c @@ -620,6 +620,171 @@ static void ufstest_flag_request(void *obj, void *data, QGuestAllocator *alloc) ufs_exit(ufs, alloc); } +static void ufstest_attr_request(void *obj, void *data, QGuestAllocator *alloc) +{ + QUfs *ufs = obj; + + UtpTransferReqDesc utrd; + UtpUpiuRsp rsp_upiu; + ufs_init(ufs, alloc); + + /* Read Readable Attributes*/ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_BOOT_LU_EN, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.opcode, ==, UFS_UPIU_QUERY_OPCODE_READ_ATTR); + g_assert_cmpuint(rsp_upiu.qr.idn, ==, UFS_QUERY_ATTR_IDN_BOOT_LU_EN); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_BKOPS_STATUS, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + /* Write Writable Attributes & Read Again */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0x03, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x03)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_EE_CONTROL, 0, 0, 0x07, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x07)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x03)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_EE_CONTROL, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x07)); + + /* Write Invalid Value (Intended Error) */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0x10, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_INVALID_VALUE); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x03)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_CNTX_CONF, 0, 15, 0x10, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_INVALID_VALUE); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_CNTX_CONF, 0, 15, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + /* Read Write-Only Attribute (Intended Error) */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_SECONDS_PASSED, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_NOT_READABLE); + + /* Write Read-Only Attribute (Intended Error) */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_POWER_MODE, 0, 0, 0x01, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_NOT_WRITEABLE); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_POWER_MODE, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + /* Invalid Selector (Intended Error)*/ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_CNTX_CONF, 0, 16, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_INVALID_SELECTOR); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_CNTX_CONF, 0, 15, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + /* Reset Written Attributes */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_WRITE_ATTR, + UFS_QUERY_ATTR_IDN_EE_CONTROL, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_ATTR, + UFS_QUERY_ATTR_IDN_EE_CONTROL, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00)); + + ufs_exit(ufs, alloc); +} + static void drive_destroy(void *path) { unlink(path); @@ -688,6 +853,7 @@ static void ufs_register_nodes(void) qos_add_test("init", "ufs", ufstest_init, NULL); qos_add_test("read-write", "ufs", ufstest_read_write, &io_test_opts); qos_add_test("flag read-write", "ufs", ufstest_flag_request, &io_test_opts); + qos_add_test("attr read-write", "ufs", ufstest_attr_request, &io_test_opts); } libqos_init(ufs_register_nodes); -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] hw/ufs: ufs descriptor read test implemented [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p1> 2024-08-21 2:27 ` 정유찬 2024-08-21 2:35 ` [PATCH 3/4] hw/ufs: ufs attribute read/write test implemented 정유찬 @ 2024-08-22 1:39 ` Yoochan Jeong 2024-08-22 5:40 ` Jeuk Kim 2 siblings, 1 reply; 10+ messages in thread From: Yoochan Jeong @ 2024-08-22 1:39 UTC (permalink / raw) To: Jeuk Kim Cc: Yoochan Jeong, thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com From 936ef0a907bcf16809f9980c2d37e8fcb13697d2 Mon Sep 17 00:00:00 2001 From: Yoochan Jeong <yc01.jeong@samsung.com> Date: Wed, 21 Aug 2024 09:09:54 +0900 Subject: [PATCH 4/4] hw/ufs: ufs descriptor read test implemented New test function "ufstest_desc_request" added, which can check one's virtual UFS device can properly read and its descriptor data. (Writing descriptors are not implemented yet.) The testcases attempt to read all kinds of descriptors at least once, except for configuration descriptors (which are not implemented yet.) There are some testcases that are intended to make an error caused by an invalid index value or an invalid selector value. Based on: 20240802051902epcms2p319bc095a15eaef8de4e6955f6718371d@epcms2p3 Signed-off-by: Yoochan Jeong <yc01.jeong@samsung.com> --- tests/qtest/ufs-test.c | 155 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c index a8fd2f1acc..f96061f922 100644 --- a/tests/qtest/ufs-test.c +++ b/tests/qtest/ufs-test.c @@ -785,6 +785,160 @@ static void ufstest_attr_request(void *obj, void *data, QGuestAllocator *alloc) ufs_exit(ufs, alloc); } +static void ufstest_desc_request(void *obj, void *data, QGuestAllocator *alloc) +{ + QUfs *ufs = obj; + + UtpTransferReqDesc utrd; + UtpUpiuRsp rsp_upiu; + ufs_init(ufs, alloc); + + /* Write Descriptor is not supported yet */ + + /* Read Device Descriptor */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_DEVICE, + 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.opcode, ==, UFS_UPIU_QUERY_OPCODE_READ_DESC); + g_assert_cmpuint(rsp_upiu.qr.idn, ==, UFS_QUERY_DESC_IDN_DEVICE); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(DeviceDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_DEVICE); + + /* Read Configuration Descriptor is not supported yet*/ + + /* Read Unit Descriptor */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 0, + 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(UnitDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT); + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 0); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 1, + 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(UnitDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT); + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 1); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, + UFS_UPIU_RPMB_WLUN, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(RpmbUnitDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT); + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, UFS_UPIU_RPMB_WLUN); + + /* Read Interconnect Descriptor */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, + UFS_QUERY_DESC_IDN_INTERCONNECT, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(InterconnectDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_INTERCONNECT); + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 0x01); + g_assert_cmpuint(rsp_upiu.qr.data[3], ==, 0x80); + g_assert_cmpuint(rsp_upiu.qr.data[4], ==, 0x04); + g_assert_cmpuint(rsp_upiu.qr.data[5], ==, 0x10); + + /* Read String Descriptor */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, + 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, 0x12); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_STRING); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, + 1, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, 0x22); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_STRING); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, + 4, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, 0x0a); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_STRING); + + /* Read Geometry Descriptor */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_GEOMETRY, + 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(GeometryDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_GEOMETRY); + + /* Read Power Descriptor */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_POWER, 0, + 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, + sizeof(PowerParametersDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_POWER); + + /* Read Health Descriptor */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_HEALTH, + 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(DeviceHealthDescriptor)); + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_HEALTH); + + /* Invalid Index (Intended Failure) */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 4, + 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_INVALID_INDEX); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, + 5, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_INVALID_INDEX); + + /* Invalid Selector (Intended Failure) */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_DEVICE, + 0, 1, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_INVALID_SELECTOR); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, + 0, 1, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_INVALID_SELECTOR); + + ufs_exit(ufs, alloc); +} + static void drive_destroy(void *path) { unlink(path); @@ -854,6 +1008,7 @@ static void ufs_register_nodes(void) qos_add_test("read-write", "ufs", ufstest_read_write, &io_test_opts); qos_add_test("flag read-write", "ufs", ufstest_flag_request, &io_test_opts); qos_add_test("attr read-write", "ufs", ufstest_attr_request, &io_test_opts); + qos_add_test("desc read-write", "ufs", ufstest_desc_request, &io_test_opts); } libqos_init(ufs_register_nodes); -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] hw/ufs: ufs descriptor read test implemented 2024-08-22 1:39 ` [PATCH 4/4] hw/ufs: ufs descriptor read " Yoochan Jeong @ 2024-08-22 5:40 ` Jeuk Kim 0 siblings, 0 replies; 10+ messages in thread From: Jeuk Kim @ 2024-08-22 5:40 UTC (permalink / raw) To: yc01.jeong, Jeuk Kim Cc: thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com On 8/22/2024 10:39 AM, Yoochan Jeong wrote: > From 936ef0a907bcf16809f9980c2d37e8fcb13697d2 Mon Sep 17 00:00:00 2001 > From: Yoochan Jeong <yc01.jeong@samsung.com> > Date: Wed, 21 Aug 2024 09:09:54 +0900 > Subject: [PATCH 4/4] hw/ufs: ufs descriptor read test implemented Remove it. > > New test function "ufstest_desc_request" added, which can check one's > virtual UFS device can properly read and its descriptor data. > (Writing descriptors are not implemented yet.) > The testcases attempt to read all kinds of descriptors at least once, > except for configuration descriptors (which are not implemented yet.) > There are some testcases that are intended to make an error caused by > an invalid index value or an invalid selector value. > > Based on: 20240802051902epcms2p319bc095a15eaef8de4e6955f6718371d@epcms2p3 > Signed-off-by: Yoochan Jeong <yc01.jeong@samsung.com> > --- > tests/qtest/ufs-test.c | 155 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 155 insertions(+) > > diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c > index a8fd2f1acc..f96061f922 100644 > --- a/tests/qtest/ufs-test.c > +++ b/tests/qtest/ufs-test.c > @@ -785,6 +785,160 @@ static void ufstest_attr_request(void *obj, void *data, QGuestAllocator *alloc) > ufs_exit(ufs, alloc); > } > > +static void ufstest_desc_request(void *obj, void *data, QGuestAllocator *alloc) > +{ > + QUfs *ufs = obj; > + > + UtpTransferReqDesc utrd; > + UtpUpiuRsp rsp_upiu; > + ufs_init(ufs, alloc); > + > + /* Write Descriptor is not supported yet */ > + > + /* Read Device Descriptor */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_DEVICE, > + 0, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.opcode, ==, UFS_UPIU_QUERY_OPCODE_READ_DESC); > + g_assert_cmpuint(rsp_upiu.qr.idn, ==, UFS_QUERY_DESC_IDN_DEVICE); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(DeviceDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_DEVICE); > + > + /* Read Configuration Descriptor is not supported yet*/ > + > + /* Read Unit Descriptor */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 0, > + 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(UnitDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT); > + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 0); > + > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 1, > + 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(UnitDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT); > + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 1); > + > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, > + UFS_UPIU_RPMB_WLUN, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(RpmbUnitDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT); > + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, UFS_UPIU_RPMB_WLUN); > + > + /* Read Interconnect Descriptor */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, > + UFS_QUERY_DESC_IDN_INTERCONNECT, 0, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(InterconnectDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_INTERCONNECT); > + g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 0x01); Depending on the ufs version, the unipro version can vary. Although they are currently 0x180 & 0x410, they should be 0x200 & 0x500 in ufs v4.0 So I don't think it's a good idea to test these with fixed values. Please remove it. > + g_assert_cmpuint(rsp_upiu.qr.data[3], ==, 0x80); > + g_assert_cmpuint(rsp_upiu.qr.data[4], ==, 0x04); > + g_assert_cmpuint(rsp_upiu.qr.data[5], ==, 0x10); > + > + /* Read String Descriptor */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, > + 0, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, 0x12); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_STRING); > + > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, > + 1, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, 0x22); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_STRING); > + > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, > + 4, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, 0x0a); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_STRING); > + > + /* Read Geometry Descriptor */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_GEOMETRY, > + 0, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(GeometryDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_GEOMETRY); > + > + /* Read Power Descriptor */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_POWER, 0, > + 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, > + sizeof(PowerParametersDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_POWER); > + > + /* Read Health Descriptor */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_HEALTH, > + 0, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); > + g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(DeviceHealthDescriptor)); > + g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_HEALTH); > + > + /* Invalid Index (Intended Failure) */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 4, > + 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, > + UFS_OCS_INVALID_CMD_TABLE_ATTR); > + g_assert_cmpuint(rsp_upiu.header.response, ==, > + UFS_QUERY_RESULT_INVALID_INDEX); > + > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, > + 5, 0, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, > + UFS_OCS_INVALID_CMD_TABLE_ATTR); > + g_assert_cmpuint(rsp_upiu.header.response, ==, > + UFS_QUERY_RESULT_INVALID_INDEX); > + > + /* Invalid Selector (Intended Failure) */ > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_DEVICE, > + 0, 1, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, > + UFS_OCS_INVALID_CMD_TABLE_ATTR); > + g_assert_cmpuint(rsp_upiu.header.response, ==, > + UFS_QUERY_RESULT_INVALID_SELECTOR); > + > + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > + UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING, > + 0, 1, 0, &utrd, &rsp_upiu); > + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, > + UFS_OCS_INVALID_CMD_TABLE_ATTR); > + g_assert_cmpuint(rsp_upiu.header.response, ==, > + UFS_QUERY_RESULT_INVALID_SELECTOR); > + > + ufs_exit(ufs, alloc); > +} > + > static void drive_destroy(void *path) > { > unlink(path); > @@ -854,6 +1008,7 @@ static void ufs_register_nodes(void) > qos_add_test("read-write", "ufs", ufstest_read_write, &io_test_opts); > qos_add_test("flag read-write", "ufs", ufstest_flag_request, &io_test_opts); > qos_add_test("attr read-write", "ufs", ufstest_attr_request, &io_test_opts); > + qos_add_test("desc read-write", "ufs", ufstest_desc_request, &io_test_opts); Why don't we use `ufstest_query_flag_request` & `ufstest_query_attr_request` & `ufstest_query_desc_request`? It looks more clear to me. > } > > libqos_init(ufs_register_nodes); ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p3>]
* [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p3> @ 2024-08-21 2:32 ` 정유찬 2024-08-22 4:50 ` Jeuk Kim [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p6> 0 siblings, 2 replies; 10+ messages in thread From: 정유찬 @ 2024-08-21 2:32 UTC (permalink / raw) To: 김제욱 Cc: 정유찬, thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com From d0ae8e25aec4ae7d222a2ea667d5ddb61f14fe02 Mon Sep 17 00:00:00 2001 From: Yoochan Jeong <yc01.jeong@samsung.com> Date: Wed, 21 Aug 2024 09:03:06 +0900 Subject: [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test Minor bugs and errors related to ufs-test are resolved. Some permissions and code implementations that are not synchronized with the ufs spec are edited. Based on: 20240802051902epcms2p319bc095a15eaef8de4e6955f6718371d@epcms2p3 Signed-off-by: Yoochan Jeong <yc01.jeong@samsung.com> --- hw/ufs/ufs.c | 26 +++++++++++++++++++++----- tests/qtest/ufs-test.c | 12 +++++++++--- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c index ce2c96aeea..9472a3c14a 100644 --- a/hw/ufs/ufs.c +++ b/hw/ufs/ufs.c @@ -971,7 +971,7 @@ static const int attr_permission[UFS_QUERY_ATTR_IDN_COUNT] = { UFS_QUERY_ATTR_READ | UFS_QUERY_ATTR_WRITE, [UFS_QUERY_ATTR_IDN_EE_STATUS] = UFS_QUERY_ATTR_READ, [UFS_QUERY_ATTR_IDN_SECONDS_PASSED] = UFS_QUERY_ATTR_WRITE, - [UFS_QUERY_ATTR_IDN_CNTX_CONF] = UFS_QUERY_ATTR_READ, + [UFS_QUERY_ATTR_IDN_CNTX_CONF] = UFS_QUERY_ATTR_READ | UFS_QUERY_ATTR_WRITE, [UFS_QUERY_ATTR_IDN_FFU_STATUS] = UFS_QUERY_ATTR_READ, [UFS_QUERY_ATTR_IDN_PSA_STATE] = UFS_QUERY_ATTR_READ | UFS_QUERY_ATTR_WRITE, [UFS_QUERY_ATTR_IDN_PSA_DATA_SIZE] = @@ -1038,7 +1038,7 @@ static QueryRespCode ufs_exec_query_flag(UfsRequest *req, int op) } *(((uint8_t *)&u->flags) + idn) = value; - req->rsp_upiu.qr.value = cpu_to_be32(value); + req->rsp_upiu.qr.value = value; return UFS_QUERY_RESULT_SUCCESS; } @@ -1148,8 +1148,11 @@ static QueryRespCode ufs_exec_query_attr(UfsRequest *req, int op) { UfsHc *u = req->hc; uint8_t idn = req->req_upiu.qr.idn; + uint8_t selector = req->req_upiu.qr.selector; uint32_t value; QueryRespCode ret; + const uint8_t UFS_QUERY_ATTR_CNTX_CONF_SELECTOR = 15; + const uint32_t UFS_QUERY_ATTR_MAXVALUE = 0x0F; ret = ufs_attr_check_idn_valid(idn, op); if (ret) { @@ -1159,10 +1162,20 @@ static QueryRespCode ufs_exec_query_attr(UfsRequest *req, int op) if (op == UFS_QUERY_ATTR_READ) { value = ufs_read_attr_value(u, idn); } else { - value = be32_to_cpu(req->req_upiu.qr.value); + value = req->req_upiu.qr.value; + if (idn == UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL && + value > UFS_QUERY_ATTR_MAXVALUE) { + return UFS_QUERY_RESULT_INVALID_VALUE; + } + if (idn == UFS_QUERY_ATTR_IDN_CNTX_CONF) { + if (selector != UFS_QUERY_ATTR_CNTX_CONF_SELECTOR) { + return UFS_QUERY_RESULT_INVALID_SELECTOR; + } else if (value == 0x00 || value > UFS_QUERY_ATTR_MAXVALUE) { + return UFS_QUERY_RESULT_INVALID_VALUE; + } + } ufs_write_attr_value(u, idn, value); } - req->rsp_upiu.qr.value = cpu_to_be32(value); return UFS_QUERY_RESULT_SUCCESS; } @@ -1287,9 +1300,12 @@ static QueryRespCode ufs_read_desc(UfsRequest *req) UfsHc *u = req->hc; QueryRespCode status; uint8_t idn = req->req_upiu.qr.idn; + uint8_t selector = req->req_upiu.qr.selector; uint16_t length = be16_to_cpu(req->req_upiu.qr.length); InterconnectDescriptor desc; - + if (selector != 0) { + return UFS_QUERY_RESULT_INVALID_SELECTOR; + } switch (idn) { case UFS_QUERY_DESC_IDN_DEVICE: memcpy(&req->rsp_upiu.qr.data, &u->device_desc, sizeof(u->device_desc)); diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c index 82ec3f0671..d70c2ee4a3 100644 --- a/tests/qtest/ufs-test.c +++ b/tests/qtest/ufs-test.c @@ -119,8 +119,10 @@ static void ufs_send_nop_out(QUfs *ufs, uint8_t slot, static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function, uint8_t query_opcode, uint8_t idn, uint8_t index, + uint8_t selector, uint32_t attr_value, UtpTransferReqDesc *utrd_out, UtpUpiuRsp *rsp_out) { + const uint16_t UFS_QUERY_DESC_MAXLENGTH = 0x62; /* Build up utp transfer request descriptor */ UtpTransferReqDesc utrd = ufs_build_req_utrd(ufs->cmd_desc_addr, slot, UFS_UTP_NO_DATA_TRANSFER, 0); @@ -136,13 +138,16 @@ static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function, req_upiu.header.query_func = query_function; req_upiu.header.task_tag = slot; /* - * QEMU UFS does not currently support Write descriptor and Write attribute, + * QEMU UFS does not currently support Write descriptor, * so the value of data_segment_length is always 0. */ req_upiu.header.data_segment_length = 0; req_upiu.qr.opcode = query_opcode; req_upiu.qr.idn = idn; req_upiu.qr.index = index; + req_upiu.qr.selector = selector; + req_upiu.qr.value = attr_value; + req_upiu.qr.length = UFS_QUERY_DESC_MAXLENGTH; qtest_memwrite(ufs->dev.bus->qts, req_upiu_addr, &req_upiu, sizeof(req_upiu)); @@ -344,7 +349,7 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc) /* Set fDeviceInit flag via query request */ ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, UFS_UPIU_QUERY_OPCODE_SET_FLAG, - UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, &utrd, &rsp_upiu); + UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, &rsp_upiu); g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); /* Wait for device to reset */ @@ -353,7 +358,8 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc) qtest_clock_step(ufs->dev.bus->qts, 100); ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, UFS_UPIU_QUERY_OPCODE_READ_FLAG, - UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, &utrd, &rsp_upiu); + UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, + &rsp_upiu); } while (be32_to_cpu(rsp_upiu.qr.value) != 0 && g_get_monotonic_time() < end_time); g_assert_cmpuint(be32_to_cpu(rsp_upiu.qr.value), ==, 0); -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test 2024-08-21 2:32 ` [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test 정유찬 @ 2024-08-22 4:50 ` Jeuk Kim [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p6> 1 sibling, 0 replies; 10+ messages in thread From: Jeuk Kim @ 2024-08-22 4:50 UTC (permalink / raw) To: yc01.jeong, 김제욱 Cc: thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com On 8/21/2024 11:32 AM, 정유찬 wrote: > From d0ae8e25aec4ae7d222a2ea667d5ddb61f14fe02 Mon Sep 17 00:00:00 2001 > From: Yoochan Jeong <yc01.jeong@samsung.com> > Date: Wed, 21 Aug 2024 09:03:06 +0900 > Subject: [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test > > Minor bugs and errors related to ufs-test are resolved. Some > permissions and code implementations that are not synchronized > with the ufs spec are edited. > > Based on: 20240802051902epcms2p319bc095a15eaef8de4e6955f6718371d@epcms2p3 > Signed-off-by: Yoochan Jeong <yc01.jeong@samsung.com> > --- > hw/ufs/ufs.c | 26 +++++++++++++++++++++----- > tests/qtest/ufs-test.c | 12 +++++++++--- > 2 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c > index ce2c96aeea..9472a3c14a 100644 > --- a/hw/ufs/ufs.c > +++ b/hw/ufs/ufs.c > @@ -971,7 +971,7 @@ static const int attr_permission[UFS_QUERY_ATTR_IDN_COUNT] = { > UFS_QUERY_ATTR_READ | UFS_QUERY_ATTR_WRITE, > [UFS_QUERY_ATTR_IDN_EE_STATUS] = UFS_QUERY_ATTR_READ, > [UFS_QUERY_ATTR_IDN_SECONDS_PASSED] = UFS_QUERY_ATTR_WRITE, > - [UFS_QUERY_ATTR_IDN_CNTX_CONF] = UFS_QUERY_ATTR_READ, > + [UFS_QUERY_ATTR_IDN_CNTX_CONF] = UFS_QUERY_ATTR_READ | UFS_QUERY_ATTR_WRITE, Although the spec defines UFS_QUERY_ATTR_IDN_CNTX_CONF as configurable, the qemu ufs does not yet implement related functionality. So it seems reasonable to leave it as not configurable to me. > [UFS_QUERY_ATTR_IDN_FFU_STATUS] = UFS_QUERY_ATTR_READ, > [UFS_QUERY_ATTR_IDN_PSA_STATE] = UFS_QUERY_ATTR_READ | UFS_QUERY_ATTR_WRITE, > [UFS_QUERY_ATTR_IDN_PSA_DATA_SIZE] = > @@ -1038,7 +1038,7 @@ static QueryRespCode ufs_exec_query_flag(UfsRequest *req, int op) > } > > *(((uint8_t *)&u->flags) + idn) = value; > - req->rsp_upiu.qr.value = cpu_to_be32(value); > + req->rsp_upiu.qr.value = value; req->rsp_upiu.qr.value uses big endian. Please check the spec > return UFS_QUERY_RESULT_SUCCESS; > } > > @@ -1148,8 +1148,11 @@ static QueryRespCode ufs_exec_query_attr(UfsRequest *req, int op) > { > UfsHc *u = req->hc; > uint8_t idn = req->req_upiu.qr.idn; > + uint8_t selector = req->req_upiu.qr.selector; > uint32_t value; > QueryRespCode ret; > + const uint8_t UFS_QUERY_ATTR_CNTX_CONF_SELECTOR = 15; > + const uint32_t UFS_QUERY_ATTR_MAXVALUE = 0x0F; The name UFS_QUERY_ATTR_MAXVALUE does not seem appropriate. Rename it to mean the maximum value of the ICC. > > ret = ufs_attr_check_idn_valid(idn, op); > if (ret) { > @@ -1159,10 +1162,20 @@ static QueryRespCode ufs_exec_query_attr(UfsRequest *req, int op) > if (op == UFS_QUERY_ATTR_READ) { > value = ufs_read_attr_value(u, idn); > } else { > - value = be32_to_cpu(req->req_upiu.qr.value); > + value = req->req_upiu.qr.value; > + if (idn == UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL && > + value > UFS_QUERY_ATTR_MAXVALUE) { Move this condition check inside the ufs_write_attr_value() function > + return UFS_QUERY_RESULT_INVALID_VALUE; > + } > + if (idn == UFS_QUERY_ATTR_IDN_CNTX_CONF) { Remove it. We haven't implemented the UFS_QUERY_ATTR_IDN_CNTX_CONF function yet. > + if (selector != UFS_QUERY_ATTR_CNTX_CONF_SELECTOR) { > + return UFS_QUERY_RESULT_INVALID_SELECTOR; > + } else if (value == 0x00 || value > UFS_QUERY_ATTR_MAXVALUE) { > + return UFS_QUERY_RESULT_INVALID_VALUE; > + } > + } > ufs_write_attr_value(u, idn, value); > } > - > req->rsp_upiu.qr.value = cpu_to_be32(value); > return UFS_QUERY_RESULT_SUCCESS; > } > @@ -1287,9 +1300,12 @@ static QueryRespCode ufs_read_desc(UfsRequest *req) > UfsHc *u = req->hc; > QueryRespCode status; > uint8_t idn = req->req_upiu.qr.idn; > + uint8_t selector = req->req_upiu.qr.selector; > uint16_t length = be16_to_cpu(req->req_upiu.qr.length); > InterconnectDescriptor desc; > - > + if (selector != 0) { > + return UFS_QUERY_RESULT_INVALID_SELECTOR; > + } > switch (idn) { > case UFS_QUERY_DESC_IDN_DEVICE: > memcpy(&req->rsp_upiu.qr.data, &u->device_desc, sizeof(u->device_desc)); > diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c > index 82ec3f0671..d70c2ee4a3 100644 > --- a/tests/qtest/ufs-test.c > +++ b/tests/qtest/ufs-test.c > @@ -119,8 +119,10 @@ static void ufs_send_nop_out(QUfs *ufs, uint8_t slot, > > static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function, > uint8_t query_opcode, uint8_t idn, uint8_t index, > + uint8_t selector, uint32_t attr_value, We use ufs_send_query() not only for attributes, but also descriptors and flags. Please rename `attr_value` to `value`. > UtpTransferReqDesc *utrd_out, UtpUpiuRsp *rsp_out) > { > + const uint16_t UFS_QUERY_DESC_MAXLENGTH = 0x62; How did you determine that the maximum size of a descriptor is 62? > /* Build up utp transfer request descriptor */ > UtpTransferReqDesc utrd = ufs_build_req_utrd(ufs->cmd_desc_addr, slot, > UFS_UTP_NO_DATA_TRANSFER, 0); > @@ -136,13 +138,16 @@ static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function, > req_upiu.header.query_func = query_function; > req_upiu.header.task_tag = slot; > /* > - * QEMU UFS does not currently support Write descriptor and Write attribute, > + * QEMU UFS does not currently support Write descriptor, We might need to check condition here that `query_opcode != UFS_UPIU_QUERY_OPCODE_WRITE_DESC`, since it is not implemented yet. > * so the value of data_segment_length is always 0. > */ > req_upiu.header.data_segment_length = 0; > req_upiu.qr.opcode = query_opcode; > req_upiu.qr.idn = idn; > req_upiu.qr.index = index; > + req_upiu.qr.selector = selector; > + req_upiu.qr.value = attr_value; > + req_upiu.qr.length = UFS_QUERY_DESC_MAXLENGTH; > qtest_memwrite(ufs->dev.bus->qts, req_upiu_addr, &req_upiu, > sizeof(req_upiu)); > > @@ -344,7 +349,7 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc) > /* Set fDeviceInit flag via query request */ > ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, > UFS_UPIU_QUERY_OPCODE_SET_FLAG, > - UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, &utrd, &rsp_upiu); > + UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, &rsp_upiu); > g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); > > /* Wait for device to reset */ > @@ -353,7 +358,8 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc) > qtest_clock_step(ufs->dev.bus->qts, 100); > ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, > UFS_UPIU_QUERY_OPCODE_READ_FLAG, > - UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, &utrd, &rsp_upiu); > + UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, > + &rsp_upiu); > } while (be32_to_cpu(rsp_upiu.qr.value) != 0 && > g_get_monotonic_time() < end_time); > g_assert_cmpuint(be32_to_cpu(rsp_upiu.qr.value), ==, 0); ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p6>]
* Re: Re: [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p6> @ 2024-08-22 6:00 ` Yoochan Jeong 2024-08-22 6:08 ` Jeuk Kim 0 siblings, 1 reply; 10+ messages in thread From: Yoochan Jeong @ 2024-08-22 6:00 UTC (permalink / raw) To: Jeuk Kim, Jeuk Kim Cc: thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com On 8/22/2024, Jeuk Kim wrote: > On 8/21/2024 11:32 AM, 정유찬 wrote: >> From d0ae8e25aec4ae7d222a2ea667d5ddb61f14fe02 Mon Sep 17 00:00:00 2001 >> From: Yoochan Jeong <yc01.jeong@samsung.com> >> Date: Wed, 21 Aug 2024 09:03:06 +0900 >> Subject: [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test >> >> Minor bugs and errors related to ufs-test are resolved. Some >> permissions and code implementations that are not synchronized >> with the ufs spec are edited. >> >> Based on: 20240802051902epcms2p319bc095a15eaef8de4e6955f6718371d@epcms2p3 >> Signed-off-by: Yoochan Jeong <yc01.jeong@samsung.com> >> --- >> hw/ufs/ufs.c 26 +++++++++++++++++++++----- >> tests/qtest/ufs-test.c 12 +++++++++--- >> 2 files changed, 30 insertions(+), 8 deletions(-) >> >> diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c >> index ce2c96aeea..9472a3c14a 100644 >> --- a/hw/ufs/ufs.c >> +++ b/hw/ufs/ufs.c >> @@ -971,7 +971,7 @@ static const int attr_permission[UFS_QUERY_ATTR_IDN_COUNT] = { >> UFS_QUERY_ATTR_READ UFS_QUERY_ATTR_WRITE, >> [UFS_QUERY_ATTR_IDN_EE_STATUS] = UFS_QUERY_ATTR_READ, >> [UFS_QUERY_ATTR_IDN_SECONDS_PASSED] = UFS_QUERY_ATTR_WRITE, >> - [UFS_QUERY_ATTR_IDN_CNTX_CONF] = UFS_QUERY_ATTR_READ, >> + [UFS_QUERY_ATTR_IDN_CNTX_CONF] = UFS_QUERY_ATTR_READ UFS_QUERY_ATTR_WRITE, > > Although the spec defines UFS_QUERY_ATTR_IDN_CNTX_CONF as configurable, > the qemu ufs does not yet implement related functionality. > So it seems reasonable to leave it as not configurable to me. > Okay. Some testcases in patch 3/4 also involve this attribute, so I will also edit them in the next version. > >> [UFS_QUERY_ATTR_IDN_FFU_STATUS] = UFS_QUERY_ATTR_READ, >> [UFS_QUERY_ATTR_IDN_PSA_STATE] = UFS_QUERY_ATTR_READ UFS_QUERY_ATTR_WRITE, >> [UFS_QUERY_ATTR_IDN_PSA_DATA_SIZE] = >> @@ -1038,7 +1038,7 @@ static QueryRespCode ufs_exec_query_flag(UfsRequest *req, int op) >> } >> >> *(((uint8_t *)&u->flags) + idn) = value; >> - req->rsp_upiu.qr.value = cpu_to_be32(value); >> + req->rsp_upiu.qr.value = value; > > req->rsp_upiu.qr.value uses big endian. Please check the spec It seems that the original code is correct, sorry for the confusion. I will edit some testcases in test functions to check be32_to_cpu values. > > >> return UFS_QUERY_RESULT_SUCCESS; >> } >> >> @@ -1148,8 +1148,11 @@ static QueryRespCode ufs_exec_query_attr(UfsRequest *req, int op) >> { >> UfsHc *u = req->hc; >> uint8_t idn = req->req_upiu.qr.idn; >> + uint8_t selector = req->req_upiu.qr.selector; >> uint32_t value; >> QueryRespCode ret; >> + const uint8_t UFS_QUERY_ATTR_CNTX_CONF_SELECTOR = 15; >> + const uint32_t UFS_QUERY_ATTR_MAXVALUE = 0x0F; > The name UFS_QUERY_ATTR_MAXVALUE does not seem appropriate. Rename it to > mean the maximum value of the ICC. I named it this way because CNTX_CONF attribute also had the same range. But since that attribute is not writable now, it seems that changing its name including ICC would be better. Also, I will move this constant to ufs.h file. >> >> ret = ufs_attr_check_idn_valid(idn, op); >> if (ret) { >> @@ -1159,10 +1162,20 @@ static QueryRespCode ufs_exec_query_attr(UfsRequest *req, int op) >> if (op == UFS_QUERY_ATTR_READ) { >> value = ufs_read_attr_value(u, idn); >> } else { >> - value = be32_to_cpu(req->req_upiu.qr.value); >> + value = req->req_upiu.qr.value; >> + if (idn == UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL && >> + value > UFS_QUERY_ATTR_MAXVALUE) { > Move this condition check inside the ufs_write_attr_value() function I will change ufs_write_attr_value to return QueryRespCode and edit this function slightly. >> + return UFS_QUERY_RESULT_INVALID_VALUE; >> + } >> + if (idn == UFS_QUERY_ATTR_IDN_CNTX_CONF) { > Remove it. We haven't implemented the UFS_QUERY_ATTR_IDN_CNTX_CONF > function yet. I will edit it that way. >> + if (selector != UFS_QUERY_ATTR_CNTX_CONF_SELECTOR) { >> + return UFS_QUERY_RESULT_INVALID_SELECTOR; >> + } else if (value == 0x00 value > UFS_QUERY_ATTR_MAXVALUE) { >> + return UFS_QUERY_RESULT_INVALID_VALUE; >> + } >> + } >> ufs_write_attr_value(u, idn, value); >> } >> - >> req->rsp_upiu.qr.value = cpu_to_be32(value); >> return UFS_QUERY_RESULT_SUCCESS; >> } >> @@ -1287,9 +1300,12 @@ static QueryRespCode ufs_read_desc(UfsRequest *req) >> UfsHc *u = req->hc; >> QueryRespCode status; >> uint8_t idn = req->req_upiu.qr.idn; >> + uint8_t selector = req->req_upiu.qr.selector; >> uint16_t length = be16_to_cpu(req->req_upiu.qr.length); >> InterconnectDescriptor desc; >> - >> + if (selector != 0) { >> + return UFS_QUERY_RESULT_INVALID_SELECTOR; >> + } >> switch (idn) { >> case UFS_QUERY_DESC_IDN_DEVICE: >> memcpy(&req->rsp_upiu.qr.data, &u->device_desc, sizeof(u->device_desc)); >> diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c >> index 82ec3f0671..d70c2ee4a3 100644 >> --- a/tests/qtest/ufs-test.c >> +++ b/tests/qtest/ufs-test.c >> @@ -119,8 +119,10 @@ static void ufs_send_nop_out(QUfs *ufs, uint8_t slot, >> >> static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function, >> uint8_t query_opcode, uint8_t idn, uint8_t index, >> + uint8_t selector, uint32_t attr_value, > > We use ufs_send_query() not only for attributes, but also descriptors > and flags. > > Please rename `attr_value` to `value`. > > I think this parameter name is okay, because this "value" in UPIU is only used when writing attributes. Writing flags do not require an actual value, and descriptor data will be stored in data segmentation area. >> UtpTransferReqDesc *utrd_out, UtpUpiuRsp *rsp_out) >> { >> + const uint16_t UFS_QUERY_DESC_MAXLENGTH = 0x62; > How did you determine that the maximum size of a descriptor is 62? It would be better to use UFS_QUERY_DESC_MAX_SIZE in ufs.h. I will edit it that way. >> /* Build up utp transfer request descriptor */ >> UtpTransferReqDesc utrd = ufs_build_req_utrd(ufs->cmd_desc_addr, slot, >> UFS_UTP_NO_DATA_TRANSFER, 0); >> @@ -136,13 +138,16 @@ static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function, >> req_upiu.header.query_func = query_function; >> req_upiu.header.task_tag = slot; >> /* >> - * QEMU UFS does not currently support Write descriptor and Write attribute, >> + * QEMU UFS does not currently support Write descriptor, > > We might need to check condition here that `query_opcode != > UFS_UPIU_QUERY_OPCODE_WRITE_DESC`, since > > it is not implemented yet. > in ufs_exec_query_write function in ufs.c, it already checks if it is trying to write a descriptor. Is there any particular reason that we should check it here in advance? >> * so the value of data_segment_length is always 0. >> */ >> req_upiu.header.data_segment_length = 0; >> req_upiu.qr.opcode = query_opcode; >> req_upiu.qr.idn = idn; >> req_upiu.qr.index = index; >> + req_upiu.qr.selector = selector; >> + req_upiu.qr.value = attr_value; >> + req_upiu.qr.length = UFS_QUERY_DESC_MAXLENGTH; >> qtest_memwrite(ufs->dev.bus->qts, req_upiu_addr, &req_upiu, >> sizeof(req_upiu)); >> >> @@ -344,7 +349,7 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc) >> /* Set fDeviceInit flag via query request */ >> ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, >> UFS_UPIU_QUERY_OPCODE_SET_FLAG, >> - UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, &utrd, &rsp_upiu); >> + UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, &rsp_upiu); >> g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); >> >> /* Wait for device to reset */ >> @@ -353,7 +358,8 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc) >> qtest_clock_step(ufs->dev.bus->qts, 100); >> ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, >> UFS_UPIU_QUERY_OPCODE_READ_FLAG, >> - UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, &utrd, &rsp_upiu); >> + UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, >> + &rsp_upiu); >> } while (be32_to_cpu(rsp_upiu.qr.value) != 0 && >> g_get_monotonic_time() < end_time); >> g_assert_cmpuint(be32_to_cpu(rsp_upiu.qr.value), ==, 0); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test 2024-08-22 6:00 ` Yoochan Jeong @ 2024-08-22 6:08 ` Jeuk Kim 0 siblings, 0 replies; 10+ messages in thread From: Jeuk Kim @ 2024-08-22 6:08 UTC (permalink / raw) To: yc01.jeong, Jeuk Kim Cc: thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com On 8/22/2024 3:00 PM, Yoochan Jeong wrote: > On 8/22/2024, Jeuk Kim wrote: >> On 8/21/2024 11:32 AM, 정유찬 wrote: >>> static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function, >>> uint8_t query_opcode, uint8_t idn, uint8_t index, >>> + uint8_t selector, uint32_t attr_value, >> We use ufs_send_query() not only for attributes, but also descriptors >> and flags. >> >> Please rename `attr_value` to `value`. >> >> > > I think this parameter name is okay, because this "value" in UPIU is > only used when writing attributes. Writing flags do not require an > actual value, and descriptor data will be stored in data segmentation > area. Okay. That's reasonable. > >>> >> We might need to check condition here that `query_opcode != >> UFS_UPIU_QUERY_OPCODE_WRITE_DESC`, since >> >> it is not implemented yet. >> > in ufs_exec_query_write function in ufs.c, it already checks > if it is trying to write a descriptor. Is there any particular > reason that we should check it here in advance? You're right. We don't need to check it here. ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p5>]
* [PATCH 2/4] hw/ufs: ufs flag read/write test implemented [not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p5> @ 2024-08-21 2:34 ` 정유찬 0 siblings, 0 replies; 10+ messages in thread From: 정유찬 @ 2024-08-21 2:34 UTC (permalink / raw) To: 김제욱 Cc: 정유찬, thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, jeongyuchan0629@gmail.com From fa2187660e82e7772c2061b22ba9eab95e92f77c Mon Sep 17 00:00:00 2001 From: Yoochan Jeong <yc01.jeong@samsung.com> Date: Wed, 21 Aug 2024 09:08:18 +0900 Subject: [PATCH 2/4] hw/ufs: ufs flag read/write test implemented New test function "ufstest_flag_request" added, which can check one's virtual UFS device can properly read and write its flag data. It tests if reading, setting, clearing and toggling flags work properly. There are some testcases that are intended to make an error caused by permission issues. Based on: 20240802051902epcms2p319bc095a15eaef8de4e6955f6718371d@epcms2p3 Signed-off-by: Yoochan Jeong <yc01.jeong@samsung.com> --- tests/qtest/ufs-test.c | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c index d70c2ee4a3..f93de9f1f2 100644 --- a/tests/qtest/ufs-test.c +++ b/tests/qtest/ufs-test.c @@ -540,6 +540,86 @@ static void ufstest_read_write(void *obj, void *data, QGuestAllocator *alloc) ufs_exit(ufs, alloc); } +static void ufstest_flag_request(void *obj, void *data, QGuestAllocator *alloc) +{ + QUfs *ufs = obj; + + UtpTransferReqDesc utrd; + UtpUpiuRsp rsp_upiu; + ufs_init(ufs, alloc); + + /* Read read-only flag */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_FLAG, + UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.opcode, ==, UFS_UPIU_QUERY_OPCODE_READ_FLAG); + g_assert_cmpuint(rsp_upiu.qr.idn, ==, UFS_QUERY_FLAG_IDN_FDEVICEINIT); + g_assert_cmpuint(rsp_upiu.qr.value, ==, 0); + + /* Flag Set, Clear, Toggle Test with fDeviceLifeSpanModeEn */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_FLAG, + UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, 0); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_SET_FLAG, + UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, 1); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_CLEAR_FLAG, + UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, 0); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_TOGGLE_FLAG, + UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, 1); + + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_TOGGLE_FLAG, + UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd, + &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS); + g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS); + g_assert_cmpuint(rsp_upiu.qr.value, ==, 0); + + /* Read Write-only Flag (Intended Failure) */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST, + UFS_UPIU_QUERY_OPCODE_READ_FLAG, + UFS_QUERY_FLAG_IDN_PURGE_ENABLE, 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_NOT_READABLE); + + /* Write Read-Only Flag (Intended Failure) */ + ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST, + UFS_UPIU_QUERY_OPCODE_SET_FLAG, UFS_QUERY_FLAG_IDN_BUSY_RTC, + 0, 0, 0, &utrd, &rsp_upiu); + g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, + UFS_OCS_INVALID_CMD_TABLE_ATTR); + g_assert_cmpuint(rsp_upiu.header.response, ==, + UFS_QUERY_RESULT_NOT_WRITEABLE); + + ufs_exit(ufs, alloc); +} + static void drive_destroy(void *path) { unlink(path); @@ -607,6 +687,7 @@ static void ufs_register_nodes(void) } qos_add_test("init", "ufs", ufstest_init, NULL); qos_add_test("read-write", "ufs", ufstest_read_write, &io_test_opts); + qos_add_test("flag read-write", "ufs", ufstest_flag_request, &io_test_opts); } libqos_init(ufs_register_nodes); -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-08-22 6:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p4>
2024-08-21 2:30 ` [PATCH 0/4] hw/ufs: ufs device testing function added and modified 정유찬
[not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p1>
2024-08-21 2:27 ` 정유찬
2024-08-21 2:35 ` [PATCH 3/4] hw/ufs: ufs attribute read/write test implemented 정유찬
2024-08-22 1:39 ` [PATCH 4/4] hw/ufs: ufs descriptor read " Yoochan Jeong
2024-08-22 5:40 ` Jeuk Kim
[not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p3>
2024-08-21 2:32 ` [PATCH 1/4] hw/ufs: minor bug fixes related to ufs-test 정유찬
2024-08-22 4:50 ` Jeuk Kim
[not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p6>
2024-08-22 6:00 ` Yoochan Jeong
2024-08-22 6:08 ` Jeuk Kim
[not found] ` <CGME20240821022726epcms1p127d8cd71ca3e1354592de8a4a5c97a10@epcms1p5>
2024-08-21 2:34 ` [PATCH 2/4] hw/ufs: ufs flag read/write test implemented 정유찬
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).