awokd@danwin1210.me has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38238 )
Change subject: vc/amd/agesa/[...]/Proc/Fch/Sata: Avoid Coverity Warnings ......................................................................
vc/amd/agesa/[...]/Proc/Fch/Sata: Avoid Coverity Warnings
Although the original code worked, it caused Coverity to warn about pointer arithmetic potentially allowing out-of-bounds access. Simplify code using SATA initialization from SB900 as an example. Tested on Lenovo G505s.
Change-Id: I6829a55d6c029abbf2da36e255e499c158c00e0d Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1241886 --- M src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c 2 files changed, 74 insertions(+), 112 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/38238/1
diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c index 26ff51d..cfb3df9 100644 --- a/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c +++ b/src/vendorcode/amd/agesa/f15tn/Proc/Fch/Sata/Family/Hudson2/Hudson2SataEnvService.c @@ -59,9 +59,6 @@ // // Local Routine // -VOID FchSataCombineControlDataByte (IN UINT8 *ControlReg); -VOID FchSataCombineControlDataWord (IN UINT16 *ControlReg); - SATA_PHY_SETTING SataPhyTable[] = { //Gen3 @@ -94,8 +91,6 @@ IN VOID *FchDataPtr ) { - UINT8 *PortRegByte; - UINT16 *PortRegWord; FCH_DATA_BLOCK *LocalCfgPtr; AMD_CONFIG_PARAMS *StdHeader;
@@ -104,14 +99,42 @@ // // Caculate SataPortReg for SATA_ESP_PORT // - PortRegByte = &(LocalCfgPtr->Sata.SataEspPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegByte = &(LocalCfgPtr->Sata.SataPortPower.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegWord = &(LocalCfgPtr->Sata.SataPortMd.SataPortMode); - FchSataCombineControlDataWord (PortRegWord); - PortRegByte = &(LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); + LocalCfgPtr->Sata.SataEspPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataEspPort.Port0 \ + + (LocalCfgPtr->Sata.SataEspPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataEspPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataEspPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataEspPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataEspPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataEspPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataEspPort.Port7 << 7); + LocalCfgPtr->Sata.SataPortPower.SataPortReg = \ + + LocalCfgPtr->Sata.SataPortPower.Port0 \ + + (LocalCfgPtr->Sata.SataPortPower.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataPortPower.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataPortPower.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataPortPower.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataPortPower.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataPortPower.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataPortPower.Port7 << 7); + LocalCfgPtr->Sata.SataPortMd.SataPortMode = \ + + LocalCfgPtr->Sata.SataPortMd.Port0 \ + + (LocalCfgPtr->Sata.SataPortMd.Port1 << 2) \ + + (LocalCfgPtr->Sata.SataPortMd.Port2 << 4) \ + + (LocalCfgPtr->Sata.SataPortMd.Port3 << 6) \ + + (LocalCfgPtr->Sata.SataPortMd.Port4 << 8) \ + + (LocalCfgPtr->Sata.SataPortMd.Port5 << 10) \ + + (LocalCfgPtr->Sata.SataPortMd.Port6 << 12) \ + + (LocalCfgPtr->Sata.SataPortMd.Port7 << 14); + LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port0 \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port7 << 7);
// // Set Sata PCI Configuration Space Write enable @@ -156,52 +179,12 @@ }
/** - * FchSataCombineControlDataByte - Combine port control options - * to one control byte. + * FchProgramSataPhy - Program Sata PHY registers * - * - * @param[in] *ControlReg - Data pointer for control byte. + * @param[in] StdHeader * */ VOID -FchSataCombineControlDataByte ( - IN UINT8 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( ControlReg + 1 + Index ); - *ControlReg |= PortControl << Index; - } -} -/** - * FchSataCombineControlDataWord - Combine port control options - * to one control Word. - * - * - * @param[in] *ControlReg - Data pointer for control byte. - * - */ -VOID -FchSataCombineControlDataWord ( - IN UINT16 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( (UINT8 *)ControlReg + 2 + Index ); - *ControlReg |= PortControl << (Index * 2); - } -} - - -VOID FchProgramSataPhy ( IN AMD_CONFIG_PARAMS *StdHeader ) diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c index 0ae0993..161c758 100644 --- a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c +++ b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataEnvService.c @@ -72,8 +72,6 @@ // // Local Routine // -VOID FchSataCombineControlDataByte (IN UINT8 *ControlReg); -VOID FchSataCombineControlDataWord (IN UINT16 *ControlReg);
/** @@ -89,8 +87,6 @@ IN VOID *FchDataPtr ) { - UINT8 *PortRegByte; - UINT16 *PortRegWord; FCH_DATA_BLOCK *LocalCfgPtr; AMD_CONFIG_PARAMS *StdHeader;
@@ -99,14 +95,42 @@ // //Caculate SataPortReg for SATA_ESP_PORT // - PortRegByte = &(LocalCfgPtr->Sata.SataEspPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegByte = &(LocalCfgPtr->Sata.SataPortPower.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); - PortRegWord = &(LocalCfgPtr->Sata.SataPortMd.SataPortMode); - FchSataCombineControlDataWord (PortRegWord); - PortRegByte = &(LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg); - FchSataCombineControlDataByte (PortRegByte); + LocalCfgPtr->Sata.SataEspPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataEspPort.Port0 \ + + (LocalCfgPtr->Sata.SataEspPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataEspPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataEspPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataEspPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataEspPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataEspPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataEspPort.Port7 << 7); + LocalCfgPtr->Sata.SataPortPower.SataPortReg = \ + + LocalCfgPtr->Sata.SataPortPower.Port0 \ + + (LocalCfgPtr->Sata.SataPortPower.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataPortPower.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataPortPower.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataPortPower.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataPortPower.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataPortPower.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataPortPower.Port7 << 7); + LocalCfgPtr->Sata.SataPortMd.SataPortMode = \ + + LocalCfgPtr->Sata.SataPortMd.Port0 \ + + (LocalCfgPtr->Sata.SataPortMd.Port1 << 2) \ + + (LocalCfgPtr->Sata.SataPortMd.Port2 << 4) \ + + (LocalCfgPtr->Sata.SataPortMd.Port3 << 6) \ + + (LocalCfgPtr->Sata.SataPortMd.Port4 << 8) \ + + (LocalCfgPtr->Sata.SataPortMd.Port5 << 10) \ + + (LocalCfgPtr->Sata.SataPortMd.Port6 << 12) \ + + (LocalCfgPtr->Sata.SataPortMd.Port7 << 14); + LocalCfgPtr->Sata.SataHotRemovalEnhPort.SataPortReg = \ + + LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port0 \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port1 << 1) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port2 << 2) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port3 << 3) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port4 << 4) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port5 << 5) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port6 << 6) \ + + (LocalCfgPtr->Sata.SataHotRemovalEnhPort.Port7 << 7);
// // Set Sata PCI Configuration Space Write enable @@ -140,51 +164,6 @@ }
/** - * FchSataCombineControlDataByte - Combine port control options - * to one control byte. - * - * - * @param[in] *ControlReg - Data pointer for control byte. - * - */ -VOID -FchSataCombineControlDataByte ( - IN UINT8 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( ControlReg + 1 + Index ); - *ControlReg |= PortControl << Index; - } -} -/** - * FchSataCombineControlDataWord - Combine port control options - * to one control Word. - * - * - * @param[in] *ControlReg - Data pointer for control byte. - * - */ -VOID -FchSataCombineControlDataWord ( - IN UINT16 *ControlReg - ) -{ - UINT8 Index; - UINT8 PortControl; - - *ControlReg = 0; - for ( Index = 0; Index < 8; Index++ ) { - PortControl = *( (UINT8 *)ControlReg + 2 + Index ); - *ControlReg |= PortControl << (Index * 2); - } -} - -/** * FchProgramSataPhy - Program Sata PHY registers * * @param[in] StdHeader
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38238 )
Change subject: vc/amd/agesa/[...]/Proc/Fch/Sata: Avoid Coverity Warnings ......................................................................
Patch Set 1: Code-Review+1
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/38238?usp=email )
Change subject: vc/amd/agesa/[...]/Proc/Fch/Sata: Avoid Coverity Warnings ......................................................................
Abandoned
-
Name of user not set (Code Review) -
Paul Menzel (Code Review)
-
Stefan Reinauer (Code Review)