Skip to content

Commit c87bd30

Browse files
Nick Bofferdingdcrowell77
authored andcommitted
Atomically latch shutdown status and TI data together in shutdown
Consolidates updating shutdown status and the TI area into the same atomic code region to prevent race conditions where one path invokes shutdown first, but another one ends up racing ahead and replacing the correct TI data with its own. Change-Id: Ic1e185e9bb6f2fc1c512a18976b89b12d47eb823 CQ: SW440180 Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/63760 Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com> Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com> Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com> Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com> Reviewed-by: ILYA SMIRNOV <ismirno@us.ibm.com> Reviewed-by: Marshall J. Wilks <mjwilks@us.ibm.com> Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
1 parent 24188d2 commit c87bd30

File tree

2 files changed

+166
-75
lines changed

2 files changed

+166
-75
lines changed

src/usr/initservice/baseinitsvc/initservice.C

Lines changed: 98 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,7 @@ InitService& InitService::getTheInstance( )
615615

616616
InitService::InitService( ) :
617617
iv_shutdownInProgress(false),
618+
iv_worst_status(false),
618619
iv_iStep( 0 ),
619620
iv_iSubStep( 0 )
620621
{
@@ -661,6 +662,49 @@ void InitService::registerBlock(void* i_vaddr, uint64_t i_size,
661662
mutex_unlock(&iv_registryMutex);
662663
}
663664

665+
bool InitService::_setShutdownStatus(
666+
const uint64_t i_status,
667+
const uint32_t i_error_info)
668+
{
669+
TRACFCOMP(g_trac_initsvc, "_setShutdownStatus(i_status=0x%016llX)",
670+
i_status);
671+
672+
// Hostboot PLIDs always start with 0x9 (32-bit)
673+
static const uint64_t PLID_MASK = 0x0000000090000000;
674+
bool first=false;
675+
676+
// Ensure no one is manpulating the registry lists
677+
mutex_lock(&iv_registryMutex);
678+
679+
// This already takes care of accepting only the first TI code, so
680+
// just always call this
681+
termWriteSRC(TI_SHUTDOWN,
682+
i_status,
683+
reinterpret_cast<uint64_t>(linkRegister()),
684+
i_error_info);
685+
686+
if (iv_shutdownInProgress)
687+
{
688+
// switch the failing status if an RC comes in after
689+
// a plid fail because we'd rather have the RC
690+
if( ((iv_worst_status & 0x00000000F0000000) == PLID_MASK)
691+
&& ((i_status & 0x00000000F0000000) != PLID_MASK)
692+
&& (i_status > SHUTDOWN_STATUS_GOOD) )
693+
{
694+
iv_worst_status = i_status;
695+
}
696+
}
697+
else
698+
{
699+
first=true;
700+
iv_worst_status = i_status;
701+
iv_shutdownInProgress = true;
702+
}
703+
704+
mutex_unlock(&iv_registryMutex);
705+
706+
return first;
707+
}
664708

665709
void doShutdown(uint64_t i_status,
666710
bool i_inBackground,
@@ -670,11 +714,27 @@ void doShutdown(uint64_t i_status,
670714
uint64_t i_masterHBInstance,
671715
uint32_t i_error_info)
672716
{
673-
termWriteSRC(TI_SHUTDOWN,
674-
i_status,
675-
reinterpret_cast<uint64_t>(linkRegister()),
676-
i_error_info);
717+
(void)InitService::doShutdown(
718+
i_status,
719+
i_inBackground,
720+
i_payload_base,
721+
i_payload_entry,
722+
i_payload_data,
723+
i_masterHBInstance,
724+
i_error_info);
725+
}
677726

727+
void InitService::doShutdown(
728+
uint64_t i_status,
729+
bool i_inBackground,
730+
uint64_t i_payload_base,
731+
uint64_t i_payload_entry,
732+
uint64_t i_payload_data,
733+
uint64_t i_masterHBInstance,
734+
uint32_t i_error_info)
735+
{
736+
bool first = Singleton<InitService>::instance()._setShutdownStatus(
737+
i_status,i_error_info);
678738
class ShutdownExecute
679739
{
680740
public:
@@ -694,7 +754,7 @@ void doShutdown(uint64_t i_status,
694754

695755
void execute()
696756
{
697-
Singleton<InitService>::instance().doShutdown(status,
757+
Singleton<InitService>::instance()._doShutdown(status,
698758
payload_base,
699759
payload_entry,
700760
payload_data,
@@ -730,15 +790,30 @@ void doShutdown(uint64_t i_status,
730790
ShutdownExecute* s = new ShutdownExecute(i_status, i_payload_base,
731791
i_payload_entry, i_payload_data,
732792
i_masterHBInstance, i_error_info);
733-
734-
if (i_inBackground)
793+
if(first)
735794
{
736-
s->startThread();
795+
if (i_inBackground)
796+
{
797+
s->startThread();
798+
}
799+
else
800+
{
801+
s->execute();
802+
while(1)
803+
{
804+
nanosleep(1,0);
805+
}
806+
}
737807
}
738808
else
739809
{
740-
s->execute();
741-
while(1) nanosleep(1,0);
810+
if(!i_inBackground)
811+
{
812+
while(1)
813+
{
814+
nanosleep(1,0);
815+
}
816+
}
742817
}
743818
}
744819

@@ -760,18 +835,17 @@ class isPriority
760835
uint32_t iv_priority;
761836
};
762837

763-
void InitService::doShutdown(uint64_t i_status,
764-
uint64_t i_payload_base,
765-
uint64_t i_payload_entry,
766-
uint64_t i_payload_data,
767-
uint64_t i_masterHBInstance,
768-
uint32_t i_error_info)
838+
void InitService::_doShutdown(uint64_t i_status,
839+
uint64_t i_payload_base,
840+
uint64_t i_payload_entry,
841+
uint64_t i_payload_data,
842+
uint64_t i_masterHBInstance,
843+
uint32_t i_error_info)
769844
{
770845
int l_rc = 0;
771846
errlHndl_t l_err = NULL;
772-
static volatile uint64_t worst_status = 0;
773847

774-
TRACFCOMP(g_trac_initsvc, "doShutdown(i_status=%.16X)",i_status);
848+
TRACFCOMP(g_trac_initsvc, "_doShutdown(i_status=%.16X)",i_status);
775849
#ifdef CONFIG_CONSOLE
776850
// check if console msg not needed or already displayed by caller
777851
if ((SHUTDOWN_STATUS_GOOD != i_status) &&
@@ -788,31 +862,7 @@ void InitService::doShutdown(uint64_t i_status,
788862
}
789863
#endif
790864

791-
// Hostboot PLIDs always start with 0x9 (32-bit)
792-
static const uint64_t PLID_MASK = 0x0000000090000000;
793-
794-
// Ensure no one is manpulating the registry lists and that only one
795-
// thread actually executes the shutdown path.
796-
mutex_lock(&iv_registryMutex);
797-
798-
if (iv_shutdownInProgress)
799-
{
800-
// switch the failing status if an RC comes in after
801-
// a plid fail because we'd rather have the RC
802-
if( ((worst_status & 0x00000000F0000000) == PLID_MASK)
803-
&& ((i_status & 0x00000000F0000000) != PLID_MASK)
804-
&& (i_status > SHUTDOWN_STATUS_GOOD) )
805-
{
806-
worst_status = i_status;
807-
}
808-
mutex_unlock(&iv_registryMutex);
809-
return;
810-
}
811-
worst_status = i_status; //first thread in is the default
812-
iv_shutdownInProgress = true;
813-
mutex_unlock(&iv_registryMutex);
814-
815-
TRACFCOMP(g_trac_initsvc, "doShutdown> status=%.16X",worst_status);
865+
TRACFCOMP(g_trac_initsvc, "_doShutdown> status=%.16X",iv_worst_status);
816866

817867
// sort the queue by priority before sending..
818868
std::sort( iv_regMsgQ.begin(), iv_regMsgQ.end());
@@ -828,7 +878,7 @@ void InitService::doShutdown(uint64_t i_status,
828878
{
829879
TRACFCOMP(g_trac_initsvc,"notify priority=0x%x, queue=0x%x", i->msgPriority, i->msgQ );
830880
l_msg->type = i->msgType;
831-
l_msg->data[0] = worst_status;
881+
l_msg->data[0] = iv_worst_status;
832882
(void)msg_sendrecv(i->msgQ,l_msg);
833883
}
834884

@@ -878,7 +928,7 @@ void InitService::doShutdown(uint64_t i_status,
878928
{
879929
TRACDCOMP(g_trac_initsvc,"notify priority=0x%x, queue=0x%x", i->msgPriority, i->msgQ );
880930
l_msg->type = i->msgType;
881-
l_msg->data[0] = worst_status;
931+
l_msg->data[0] = iv_worst_status;
882932
(void)msg_sendrecv(i->msgQ,l_msg);
883933
}
884934
msg_free(l_msg);
@@ -891,17 +941,17 @@ void InitService::doShutdown(uint64_t i_status,
891941
task_yield();
892942
nanosleep(0,TEN_CTX_SWITCHES_NS);
893943

894-
TRACFCOMP(g_trac_initsvc, "doShutdown> Final status=%.16X",worst_status);
944+
TRACFCOMP(g_trac_initsvc, "_doShutdown> Final status=%.16X",iv_worst_status);
895945
MAGIC_INST_PRINT_ISTEP(21,4);
896946

897947
// Update the HB TI area with the worst status.
898948
termWriteSRC(TI_SHUTDOWN,
899-
worst_status,
949+
iv_worst_status,
900950
reinterpret_cast<uint64_t>(linkRegister()),
901951
i_error_info,
902952
true); // Force write
903953

904-
shutdown(worst_status,
954+
shutdown(iv_worst_status,
905955
i_payload_base,
906956
i_payload_entry,
907957
i_payload_data,

src/usr/initservice/baseinitsvc/initservice.H

Lines changed: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -214,32 +214,6 @@ public:
214214
*/
215215
bool unregisterShutdownEvent(msg_q_t i_msgQ);
216216

217-
/**
218-
* @brief Perform necessary shut down steps.
219-
*
220-
* @param[in] i_status - Shutdown status to be passed along on shutdown
221-
* @param[in] i_payload_base - The base address (target HRMOR) of the
222-
* payload.
223-
* @param[in] i_payload_entry - The offset from base address of the
224-
* payload entry-point.
225-
* @param[in] i_payload_entry - HRMOR adjusted address of any payload data
226-
* placed in r3
227-
* @param[in] i_masterHBInstance - master hostboot instance number (node)
228-
* Needed when starting payload on a
229-
* multi-node system.
230-
* @param[in] i_error_info - Additional error data to be included in TI data
231-
*
232-
* @return Nothing
233-
* @note This calls registered services to notify them of shutdown and it
234-
* flushes the virtual memory.
235-
*/
236-
void doShutdown ( uint64_t i_status,
237-
uint64_t i_payload_base = 0,
238-
uint64_t i_payload_entry = 0,
239-
uint64_t i_payload_data = 0,
240-
uint64_t i_masterHBInstance = 0xffffffffffffffffull,
241-
uint32_t i_error_info = 0);
242-
243217
/**
244218
* @brief Save Istep Step and Substep for use by error logging
245219
* @param[in] i_step, Istep Step
@@ -258,6 +232,20 @@ public:
258232
void GetIstepData( uint8_t & o_step,
259233
uint8_t & o_subStep );
260234

235+
/**
236+
* @brief Shut down Hostboot in a controlled way
237+
*
238+
* @note: See the API documentation @
239+
* src/include/usr/initservice/initserviceif.H
240+
*/
241+
static void doShutdown (
242+
uint64_t i_status,
243+
bool i_inBackground = false,
244+
uint64_t i_payload_base = 0,
245+
uint64_t i_payload_entry = 0,
246+
uint64_t i_payload_data = 0,
247+
uint64_t i_masterHBInstance = THIS_NODE_NO_PAYLOAD,
248+
uint32_t i_error_info = 0);
261249

262250
protected:
263251

@@ -271,14 +259,64 @@ protected:
271259
*/
272260
~InitService();
273261

274-
275262
private:
276263
/**
277264
* Disable copy constructor and assignment operator
278265
*/
279266
InitService(const InitService& i_right);
280267
InitService& operator=(const InitService& i_right);
281268

269+
/**
270+
* @brief Atomically sets the shutdown status and TI data associated with
271+
* a shutdown request
272+
*
273+
* @par Detailed Description
274+
* Atomically sets the shutdown status and TI data associated with
275+
* a shutdown request. Any number of callers may initiate parallel
276+
* shutdown requests, however this interface ensures that the first
277+
* caller atomically updates the initial TI area and shutdown status.
278+
* Subsequent callers will only be able to lock in a new status (TI
279+
* area remains untouched) if it is a non-success reason code and the
280+
* initial status was a PLID.
281+
*
282+
* @warning Should only be called by INITSERVICE::doShutdown
283+
*
284+
* @param[in] i_status Shutdown status to be passed to primary shutdown
285+
* handler
286+
* @param[in] i_error_info Additional error data to be included in TI data
287+
*
288+
* @return bool Boolean indicating whether this was the first shutdown
289+
* to be requested
290+
*/
291+
bool _setShutdownStatus(
292+
uint64_t i_status,
293+
uint32_t i_error_info);
294+
295+
/**
296+
* @brief Perform necessary shut down steps.
297+
*
298+
* @param[in] i_status - Shutdown status to be passed along on shutdown
299+
* @param[in] i_payload_base - The base address (target HRMOR) of the
300+
* payload.
301+
* @param[in] i_payload_entry - The offset from base address of the
302+
* payload entry-point.
303+
* @param[in] i_payload_entry - HRMOR adjusted address of any payload data
304+
* placed in r3
305+
* @param[in] i_masterHBInstance - master hostboot instance number (node)
306+
* Needed when starting payload on a
307+
* multi-node system.
308+
* @param[in] i_error_info - Additional error data to be included in TI data
309+
*
310+
* @return Nothing
311+
* @note This calls registered services to notify them of shutdown and it
312+
* flushes the virtual memory.
313+
*/
314+
void _doShutdown (uint64_t i_status,
315+
uint64_t i_payload_base = 0,
316+
uint64_t i_payload_entry = 0,
317+
uint64_t i_payload_data = 0,
318+
uint64_t i_masterHBInstance = 0xffffffffffffffffull,
319+
uint32_t i_error_info = 0);
282320

283321
/**
284322
* Check and load module associated with this task or function
@@ -325,6 +363,9 @@ private:
325363
mutex_t iv_registryMutex;
326364
bool iv_shutdownInProgress;
327365

366+
// Worst shutdown status accumulated during shutdown processing
367+
uint64_t iv_worst_status;
368+
328369
uint8_t iv_iStep; // shadow of current istep / substep
329370
uint8_t iv_iSubStep; // for error logs
330371

0 commit comments

Comments
 (0)