-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Guard #14
base: main
Are you sure you want to change the base?
Guard #14
Conversation
libguard/guard_interface.cpp
Outdated
@@ -118,6 +118,34 @@ int guardNext(GuardFile& file, int pos, GuardRecord& guard) | |||
return pos; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit message:
Please elaborate on the problem and add test case details.
we could split this commit as 2 commit
- New flag and API (it can talk about the actual issue and solution)
- libguard using that API to support BMC flow too (The reason for updating the flag in the libguard too)
libguard/guard_interface.cpp
Outdated
if (set) | ||
{ | ||
// Setting write bit in guard header | ||
guardRecord.iv_flags = GARD_FLAG_MASK_PNOR_WRITE_IN_PROGRESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will hit the endianness issue here. Can please confirm with the HB team?
BMC is little-endian and HB is big-endian (pnor), HB uses the 7th bit from the flag but, BMC will set in the 0th bit in the flag.
The same comment is applicable to the else
block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unit8_t no endianness issue for 1 byte data.
libguard/guard_interface.cpp
Outdated
@@ -158,6 +186,8 @@ GuardRecord create(const EntityPath& entityPath, uint32_t eId, uint8_t eType, | |||
memset(&existGuard, 0xff, sizeOfGuard); | |||
|
|||
GuardFile file(guardFilePath); | |||
toggleWriteFlag(file, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why cannot just use it inside the GuardFile::write() instead of calling many places? any specific reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the guard_file.cpp only do basic read and write operation, any external operation should be done in interface.cpp i.e. writing to header etc.
Change: Adding GardFlags and GardFlagPnorWriteInProgress enums to handle GUARD partition write. Signed-off-by: Chirag Sharma <[email protected]>
Change: Adding new function to set/reset the write flag. While creating guard record or re-writing a guard record write flag will be set to 0x00 and once the write completes, set the flag to 0x01. Tested: While writing the guard data the flag is set as 0x00 and once the write is finished its set back as 0x01. Created guard records to check if data is coming as expected or not. Signed-off-by: Chirag Sharma <[email protected]>
Change: Adding function checkWriteflag, which will read the write flag. Tested: In ipl code calling this function and its working as expected i.e. if flag is set to 0x00 return true else false. Signed-off-by: Chirag Sharma <[email protected]>
@@ -333,10 +372,12 @@ static void invalidateRecord(const guardRecordParam& value) | |||
(existGuard.targetId == entityPath)) && | |||
(existGuard.recordId != GUARD_RESOLVED)) | |||
{ | |||
toggleWriteFlag(file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not correct, these chain operatoins are dangerous you look for CustoFD class you need to follow that, constructor of CustomFd opens the file and destructor closes the fd.
{
CustomFD fd;
.
.
}
when it reaches here fd will be closed
Please include summary in the PR why these chnages introduced. PR name "Huard" is not making any sense here. |
No description provided.