Skip to content
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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Guard #14

wants to merge 3 commits into from

Conversation

chiragsibm
Copy link

No description provided.

@@ -118,6 +118,34 @@ int guardNext(GuardFile& file, int pos, GuardRecord& guard)
return pos;
Copy link
Collaborator

@RameshIyyar RameshIyyar May 19, 2022

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 Show resolved Hide resolved
libguard/guard_interface.cpp Outdated Show resolved Hide resolved
libguard/guard_interface.cpp Outdated Show resolved Hide resolved
libguard/guard_interface.cpp Outdated Show resolved Hide resolved
libguard/include/guard_record.hpp Show resolved Hide resolved
if (set)
{
// Setting write bit in guard header
guardRecord.iv_flags = GARD_FLAG_MASK_PNOR_WRITE_IN_PROGRESS;
Copy link
Collaborator

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.

Copy link
Author

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.

@@ -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);
Copy link
Collaborator

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?

Copy link
Author

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.

libguard/guard_interface.cpp Outdated Show resolved Hide resolved
libguard/guard_interface.cpp Outdated Show resolved Hide resolved
Chirag Sharma added 3 commits May 26, 2022 00:59
Change:
Adding GardFlags and GardFlagPnorWriteInProgress enums to handle
GUARD partition write.

Signed-off-by: Chirag Sharma <chirshar@in.ibm.com>
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 <chirshar@in.ibm.com>
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 <chirshar@in.ibm.com>
@@ -333,10 +372,12 @@ static void invalidateRecord(const guardRecordParam& value)
(existGuard.targetId == entityPath)) &&
(existGuard.recordId != GUARD_RESOLVED))
{
toggleWriteFlag(file);
Copy link
Collaborator

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

@ojayanth
Copy link

Please include summary in the PR why these chnages introduced. PR name "Huard" is not making any sense here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants