From 87f97c111c829aef35df58b25215eeaaeaedb1b5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 5 Sep 2023 10:22:18 +0200 Subject: [PATCH] Addressed reviewer's comments --- include/x86/ata.h | 8 ++++++++ options.mk | 1 - src/x86/ahci.c | 7 +++++-- src/x86/ata.c | 45 +++++++-------------------------------------- 4 files changed, 20 insertions(+), 41 deletions(-) diff --git a/include/x86/ata.h b/include/x86/ata.h index 9e23157b..aeacf9fb 100644 --- a/include/x86/ata.h +++ b/include/x86/ata.h @@ -87,6 +87,10 @@ enum ata_security_state ata_security_get_state(int); #define ATA_CMD_WRITE_DMA 0xCA #define ATA_CMD_IDENTIFY_DEVICE 0xEC +#define ATA_IDENTIFY_DEVICE_COMMAND_LEN (256 * 2) + + +/* Security feature set */ #define ATA_CMD_SECURITY_SET_PASSWORD 0xF1 #define ATA_CMD_SECURITY_UNLOCK 0xF2 #define ATA_CMD_SECURITY_ERASE_PREPARE 0xF3 @@ -94,4 +98,8 @@ enum ata_security_state ata_security_get_state(int); #define ATA_CMD_SECURITY_FREEZE_LOCK 0xF5 #define ATA_CMD_SECURITY_DISABLE_PASSWORD 0xF6 +/* Constants for security set commands */ +#define ATA_SECURITY_COMMAND_LEN (256 * 2) +#define ATA_SECURITY_PASSWORD_OFFSET (1 * 2) + #endif diff --git a/options.mk b/options.mk index a861d556..27a31722 100644 --- a/options.mk +++ b/options.mk @@ -536,7 +536,6 @@ ifeq ($(LINUX_PAYLOAD),1) endif endif - ifeq ($(64BIT),1) CFLAGS+=-DWOLFBOOT_64BIT endif diff --git a/src/x86/ahci.c b/src/x86/ahci.c index 8cb6d0f7..0efe87b5 100644 --- a/src/x86/ahci.c +++ b/src/x86/ahci.c @@ -65,8 +65,6 @@ #define PCI_REG_MAP_AHCI_MODE (0x1 << 6) #define PCI_REG_MAP_ALL_PORTS (0x1 << 5) -#define DEBUG_AHCI - #ifdef DEBUG_AHCI #define AHCI_DEBUG_PRINTF(...) wolfBoot_printf(__VA_ARGS__) #else @@ -395,12 +393,15 @@ void sata_enable(uint32_t base) { #ifdef WOLFBOOT_ATA_DISK_LOCK ata_st = ata_security_get_state(drv); + wolfBoot_printf("ATA: Security state SEC%d\r\n", ata_st); if (ata_st == ATA_SEC1) { AHCI_DEBUG_PRINTF("ATA identify: calling freeze lock\r\n", r); r = ata_security_freeze_lock(drv); AHCI_DEBUG_PRINTF("ATA security freeze lock: returned %d\r\n", r); r = ata_identify_device(drv); AHCI_DEBUG_PRINTF("ATA identify: returned %d\r\n", r); + ata_st = ata_security_get_state(drv); + wolfBoot_printf("ATA: Security disabled. State SEC%d\r\n", ata_st); } else if (ata_st == ATA_SEC4) { AHCI_DEBUG_PRINTF("ATA identify: calling device unlock\r\n", r); @@ -420,6 +421,8 @@ void sata_enable(uint32_t base) { if (ata_st != ATA_SEC6) { panic(); } + ata_st = ata_security_get_state(drv); + wolfBoot_printf("ATA: Security enabled. State SEC%d\r\n", ata_st); } #endif } diff --git a/src/x86/ata.c b/src/x86/ata.c index 60020080..b6bd8058 100644 --- a/src/x86/ata.c +++ b/src/x86/ata.c @@ -41,8 +41,6 @@ #define CACHE_INVALID 0xBADF00DBADC0FFEEULL -#define DEBUG_ATA - #ifdef DEBUG_ATA /** * @brief This macro is used to conditionally print debug messages for the ATA @@ -162,36 +160,6 @@ struct __attribute__((packed)) fis_reg_d2h { uint32_t _res4; }; -/** - * @brief This packed structure defines the format of a Data FIS used for - * PIO transfer setup. - */ -struct __attribute__((packed)) fis_pio_setup { - uint8_t fis_type; - uint8_t pmport:4, _res0:1, dir:1, i:1, _res1:1; - - uint8_t status; - uint8_t error; - - uint8_t lba0; - uint8_t lba1; - uint8_t lba2; - uint8_t device; - - - uint8_t lba3; - uint8_t lba4; - uint8_t lba5; - uint8_t _res2; - - uint8_t count_l; - uint8_t count_h; - uint8_t _res3; - uint8_t e_status; - - uint16_t transfer_count; - uint8_t _res4[2]; -}; /** * @brief This packed structure defines the format of a Data FIS used for @@ -382,7 +350,7 @@ static int security_command(int drv, uint8_t ata_cmd) struct fis_reg_h2d *cmdfis; struct ata_drive *ata = &ATA_Drv[drv]; int ret; - int slot = prepare_cmd_h2d_slot(drv, buffer, 512, 0); + int slot = prepare_cmd_h2d_slot(drv, buffer, ATA_SECURITY_COMMAND_LEN, 0); if (slot < 0) { return slot; } @@ -420,9 +388,10 @@ static int security_command_passphrase(int drv, uint8_t ata_cmd, struct fis_reg_h2d *cmdfis; struct ata_drive *ata = &ATA_Drv[drv]; int ret; - int slot = prepare_cmd_h2d_slot(drv, buffer, 512, 1); - memset(buffer, 0, 512); - memcpy(buffer + 2, passphrase, strlen(passphrase)); + int slot = prepare_cmd_h2d_slot(drv, buffer, + ATA_SECURITY_COMMAND_LEN, 1); + memset(buffer, 0, ATA_SECURITY_COMMAND_LEN); + memcpy(buffer + ATA_SECURITY_PASSWORD_OFFSET, passphrase, strlen(passphrase)); if (slot < 0) { return slot; } @@ -543,13 +512,13 @@ int ata_identify_device(int drv) uint8_t serial_no[ATA_ID_SERIAL_NO_LEN]; uint8_t model_no[ATA_ID_MODEL_NO_LEN]; int ret = 0; - int slot = prepare_cmd_h2d_slot(drv, buffer, 512, 0); + int slot = prepare_cmd_h2d_slot(drv, buffer, + ATA_IDENTIFY_DEVICE_COMMAND_LEN, 0); int s_locked, s_frozen, s_enabled, s_supported; if (slot < 0) return slot; - cmd = (struct hba_cmd_header *)(uintptr_t)ata->clb_port; cmd += slot;