Add support for fatal error monitoring #98

abinayaddhandapani · 2024-11-13T16:13:36Z

- The application monitors APML_ALERT_L and upon assertion of the
  GPIO PIN , BMC collects the MCA MSR dump and creates CPER record.

- Depending on the user configuration , the application initiates
  system recovery.

- ras-config.json contains the configuration parameters with
  default value. User can get and set the configuration parameters
  using d-bus calls to the methods getAttribute and setAttribute.

- The application also contains harvesting of last transaction address,
  debug log ID's.

- The application is intended to be supported for 1P and 2P platforms.

Tested fields:

Tested in sp5 platform

root@sp5:/var/lib/amd-ras# ls

current_index ras-error0.cper ras-error2.cper ras-error4.cper ras-error6.cper ras-error8.cper
ras-config.json ras-error1.cper ras-error3.cper ras-error5.cper ras-error7.cper ras-error9.cper
root@sp5:/var/lib/amd-ras# hexdump -C ras-error0.cper
00000000 43 50 45 52 00 01 ff ff ff ff 02 00 01 00 00 00 |CPER............|
00000010 03 00 00 00 10 01 02 00 07 1a 11 01 0f 0b 18 15 |................|
00000020 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000040 ac 3f fa 61 80 cb 92 42 8b fb d6 43 b1 de 17 f4 |.?.a...B...C....|
00000050 fe 6f f5 e8 9c 91 c5 4c ba 88 65 ab e1 49 13 bb |.o.....L..e..I..|
00000060 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000080 10 01 00 00 00 00 01 00 01 11 03 00 01 00 00 00 |................|
00000090 78 0c ac 32 23 26 f6 48 b0 d0 73 65 72 5f d6 ae |x..2#&.H..ser_..|
000000a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
000000b0 01 00 00 00 50 30 00 00 00 00 00 00 00 00 00 00 |....P0..........|
000000c0 00 00 00 00 00 00 00 00 10 01 01 00 00 00 01 00 |................|
000000d0 01 11 03 00 01 00 00 00 78 0c ac 32 23 26 f6 48 |........x..2#&.H|
000000e0 b0 d0 73 65 72 5f d6 ae 00 00 00 00 00 00 00 00 |..ser_..........|
000000f0 00 00 00 00 00 00 00 00 01 00 00 00 50 31 00 00 |............P1..|
00000100 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000110 07 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000120 11 00 00 00 0b 00 ff 00 00 00 00 00 00 00 00 00 |................|
00000130 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00000150 e0 00 00 4d 00 00 00 00 00 70 b1 13 18 00 00 00 |...M.....p......|
00000160 0b 01 00 00 00 00 a0 ba 00 00 00 00 00 00 00 00 |................|
00000170 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00000190 01 00 80 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
000001a0 3f 00 00 00 00 00 00 00 0b 01 00 00 00 00 a0 ba |?...............|
000001b0 00 00 00 00 00 00 00 00 00 00 00 00 f6 0f 0c d0 |................|
000001c0 fd 01 00 00 27 00 00 00 00 70 b1 13 18 00 00 00 |....'....p......|
000001d0 e0 00 00 4d 00 00 00 00 00 00 00 00 00 00 00 00 |...M............|
000001e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00020110

ojayanth · 2024-11-18T16:51:34Z

@abinayaddhandapani Can you please run ./../openbmc-build-scripts/scripts/format-code.sh tool . In your devbox you can do docker setup and run this tool from docker.

ojayanth · 2024-11-18T17:45:06Z

amd-bmc-ras$ ./../openbmc-build-scripts/scripts/format-code.sh
Formatting code under /mnt/work2/ojayanth/Code/amd-bmc-ras
clang_format: cannot find config .clang-format
eslint: cannot find .eslintrc.json; using /mnt/work2/ojayanth/Code/openbmc-build-scripts/config/eslint-global-config.json
markdownlint: cannot find .markdownlint.yaml; using /mnt/work2/ojayanth/Code/openbmc-build-scripts/config/markdownlint.yaml
prettier: cannot find .prettierrc.yaml; using /mnt/work2/ojayanth/Code/openbmc-build-scripts/config/prettierrc.yaml
Running commit_gitlint
-: UC1 Line exceeds max length (96>72).
It's possible you intended to use one of the following exceptions:
1. Put logs or shell script in a quoted section with triple quotes (''')
before and after the section
2. Put a long link at the bottom in a footnote.
example: [1] https://my_long_link.com
Line that was too long:
: "NAME TYPE SIGNATURE RESULT/VALUE FLAGS"
Running commit_spelling
codespell-dictionary - misspelling count >> 0
generic-dictionary - misspelling count >> 0
Running eslint
Running markdownlint
README.md:1:1 MD018/no-missing-space-atx No space after hash on atx style heading [Context: "#AMD BMC RAS"]
README.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "#AMD BMC RAS"]
Failed markdownlint; temporarily ignoring.
Running prettier
config/ras-config.json 49ms
README.md 23ms
commit_gitlint: FAILED

ojayanth · 2024-11-18T18:02:28Z

@abinayaddhandapani Direction on Header file inclusion:

Including headers directly in the source file makes it clear which dependencies are needed for that specific file. This improves readability and maintainability.
Relying on another header file to include necessary headers can lead to hidden dependencies, making the codebase harder to understand and maintain. Direct inclusion avoids this issue.
Please add header files, which is necessary for the specific source file,
Example : inc/ras.hpp add boot dependency and not used anywhere in the header. .
Some additional points also need to considering defining header file contents.
- Create multiple headers based on the scope and responsibility of the code.
- One Header per Source File: Typically, each .cpp file should have a corresponding .h file. This makes it clear which declarations belong to which implementations and helps in managing dependencies.

Refer https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md for more details.

abinayaddhandapani · 2024-11-19T14:51:46Z

@abinayaddhandapani Can you please run ./../openbmc-build-scripts/scripts/format-code.sh tool . In your devbox you can do docker setup and run this tool from docker.

@ojayanth

adhandap@ebcc56e935b6:/mnt/work2/adhandap/Code/amd-bmc-ras$ ../openbmc-build-scripts/scripts/format-code.sh
Formatting code under /mnt/work2/adhandap/Code/amd-bmc-ras
eslint: cannot find .eslintrc.json; using /mnt/work2/adhandap/Code/openbmc-build-scripts/config/eslint-global-config.json
markdownlint: cannot find .markdownlint.yaml; using /mnt/work2/adhandap/Code/openbmc-build-scripts/config/markdownlint.yaml
prettier: cannot find .prettierrc.yaml; using /mnt/work2/adhandap/Code/openbmc-build-scripts/config/prettierrc.yaml
Running commit_gitlint
Running commit_spelling
codespell-dictionary - misspelling count >> 0
generic-dictionary - misspelling count >> 0
Running clang_format
Running eslint
Running markdownlint
Running prettier
config/ras-config.json 43ms (unchanged)
README.md 25ms (unchanged)
Result differences...
Format: PASSED

abinayaddhandapani · 2024-11-19T14:52:11Z

amd-bmc-ras$ ./../openbmc-build-scripts/scripts/format-code.sh Formatting code under /mnt/work2/ojayanth/Code/amd-bmc-ras clang_format: cannot find config .clang-format eslint: cannot find .eslintrc.json; using /mnt/work2/ojayanth/Code/openbmc-build-scripts/config/eslint-global-config.json markdownlint: cannot find .markdownlint.yaml; using /mnt/work2/ojayanth/Code/openbmc-build-scripts/config/markdownlint.yaml prettier: cannot find .prettierrc.yaml; using /mnt/work2/ojayanth/Code/openbmc-build-scripts/config/prettierrc.yaml Running commit_gitlint -: UC1 Line exceeds max length (96>72). It's possible you intended to use one of the following exceptions: 1. Put logs or shell script in a quoted section with triple quotes (''') before and after the section 2. Put a long link at the bottom in a footnote. example: [1] https://my_long_link.com Line that was too long: : "NAME TYPE SIGNATURE RESULT/VALUE FLAGS" Running commit_spelling codespell-dictionary - misspelling count >> 0 generic-dictionary - misspelling count >> 0 Running eslint Running markdownlint README.md:1:1 MD018/no-missing-space-atx No space after hash on atx style heading [Context: "#AMD BMC RAS"] README.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "#AMD BMC RAS"] Failed markdownlint; temporarily ignoring. Running prettier config/ras-config.json 49ms README.md 23ms commit_gitlint: FAILED

@ojayanth adhandap@ebcc56e935b6:/mnt/work2/adhandap/Code/amd-bmc-ras$ ../openbmc-build-scripts/scripts/format-code.sh
Formatting code under /mnt/work2/adhandap/Code/amd-bmc-ras
eslint: cannot find .eslintrc.json; using /mnt/work2/adhandap/Code/openbmc-build-scripts/config/eslint-global-config.json
markdownlint: cannot find .markdownlint.yaml; using /mnt/work2/adhandap/Code/openbmc-build-scripts/config/markdownlint.yaml
prettier: cannot find .prettierrc.yaml; using /mnt/work2/adhandap/Code/openbmc-build-scripts/config/prettierrc.yaml
Running commit_gitlint
Running commit_spelling
codespell-dictionary - misspelling count >> 0
generic-dictionary - misspelling count >> 0
Running clang_format
Running eslint
Running markdownlint
Running prettier
config/ras-config.json 43ms (unchanged)
README.md 25ms (unchanged)
Result differences...
Format: PASSED

abinayaddhandapani · 2024-11-19T14:53:02Z

@abinayaddhandapani Direction on Header file inclusion:

Including headers directly in the source file makes it clear which dependencies are needed for that specific file. This improves readability and maintainability.

Relying on another header file to include necessary headers can lead to hidden dependencies, making the codebase harder to understand and maintain. Direct inclusion avoids this issue.
Please add header files, which is necessary for the specific source file,
Example : inc/ras.hpp add boot dependency and not used anywhere in the header. .
Some additional points also need to considering defining header file contents.

Create multiple headers based on the scope and responsibility of the code.

One Header per Source File: Typically, each .cpp file should have a corresponding .h file. This makes it clear which declarations belong to which implementations and helps in managing dependencies.

Refer https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md for more details.

@ojayanth Moved the header files to the corresponding source files instead of adding it in ras.hpp file.

ojayanth

Review still in progress

README.md

OWNERS

LICENSE

config/ras-config.json

meson.build

ojayanth · 2024-11-19T13:45:50Z

meson.build

+endif
+
+executable(
+  'amd-bmc-ras',


Can get better executable name , not a project a name.

amd-bmc-ras seems to be more suitable for the application. Few more options would be ADDC, amd-crashdump.
If you have any better suggestion , please suggest

meson.build

meson_options.txt

ojayanth · 2024-11-19T17:32:55Z

@abinayaddhandapani Direction on Header file inclusion:

Including headers directly in the source file makes it clear which dependencies are needed for that specific file. This improves readability and maintainability.

Relying on another header file to include necessary headers can lead to hidden dependencies, making the codebase harder to understand and maintain. Direct inclusion avoids this issue.
Please add header files, which is necessary for the specific source file,
Example : inc/ras.hpp add boot dependency and not used anywhere in the header. .
Some additional points also need to considering defining header file contents.

Create multiple headers based on the scope and responsibility of the code.

One Header per Source File: Typically, each .cpp file should have a corresponding .h file. This makes it clear which declarations belong to which implementations and helps in managing dependencies.

Refer https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md for more details.

@ojayanth Moved the header files to the corresponding source files instead of adding it in ras.hpp file.

Still seeing lot of unused constants varible definition in the commit some
can you review these variables , and keep usage specific , unless there is strong reason for keeping everything at one place.
Also keep the variable/constants only used by this commit .

inc/ras.hpp

ojayanth · 2024-11-19T17:24:28Z

inc/ras.hpp

+static const int EPYC_PROG_SEG_ID = 0x01;
+static const int MI_PROG_SEG_ID = 0x02;
+static const int MAX_ERROR_FILE = 10;
+static const std::string RAS_DIR = "/var/lib/amd-ras/";


Move this as part of meson option , will help user to modify path if required. , and other varibales can define as exxtension of this

ojayanth · 2024-11-20T03:11:04Z

@abinayaddhandapani please run format-code.sh commit specific and strt fixing format issues specific to commit . Also move clang cofig file before adding source code commit.

inc/ras.hpp

ojayanth · 2024-11-20T13:39:40Z

inc/interface_manager_base.hpp

@@ -0,0 +1,61 @@
+#include "config_manager.hpp"


OpenBMC CPP Coding guide line available here https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md, please try to follow the guidelines for File/Function/Variables Naming , prologue etc.
Eg:
/**

@brief Sets the threshold value for the temperature sensor.

This function sets the threshold value for the temperature sensor. The

threshold value is used to trigger alerts when the temperature exceeds

the specified limit.

@param[in] sensorId The ID of the sensor.

@param[in] threshold The threshold value to set.

@return True if the threshold was set successfully, false otherwise.
*/
bool setTemperatureThreshold(int sensorId, int threshold)
{
// Function implementation
}

Not addressed in the latest patch.

I have added the comments as metioned in the above format wherever possible.
The comments where added as part of the function declaration in the classes.

src/main.cpp

ojayanth · 2024-11-20T14:07:25Z

src/main.cpp

+    rasManagerObj->configure();
+
+#endif
+    /* TODO: Enable PLDM : Create rasManagerObj pointer


Either you should report build failure or Log error message saying support is not available till PLDM support is in place . This code can messup , when user enabled PLDM falg.

src/main.cpp

supven01 · 2024-11-20T16:40:16Z

OWNERS

+# It is expected that these 4 sections will be listed in the order above and
+# data within them will be kept sorted.
+
+owners:


Missing. Add your's and Jayanth's name

OWNERS

supven01 · 2024-11-20T16:41:18Z

LICENSE

@@ -0,0 +1,201 @@
+                                 Apache License


Apache licensing is fine as we have used that before.

supven01 · 2024-11-20T16:49:32Z

inc/ras.hpp

+static const int BLOCK_ID_38 = 38;
+static const int BLOCK_ID_39 = 39;
+static const int BLOCK_ID_40 = 40;
+static const int MI300A_MODEL_NUMBER = 0x90;


This does not belong in generic ras.hpp file. Need a platform file to define this

supven01 · 2024-11-20T16:49:44Z

inc/ras.hpp

+
+static const int RESERVED = 0;
+static const int BASE_16 = 16;
+static const int TWO_SOCKET = 2;


This does not belong in generic ras.hpp file. Need a platform file to define this

supven01 · 2024-11-20T16:50:05Z

inc/ras.hpp

+static const int BLOCK_ID_39 = 39;
+static const int BLOCK_ID_40 = 40;
+static const int MI300A_MODEL_NUMBER = 0x90;
+static const int MI300C_MODEL_NUMBER = 0x80;


This does not belong in generic ras.hpp file. Need a platform file to define this

supven01 · 2024-11-20T16:50:19Z

inc/ras.hpp

+static const int BLOCK_ID_40 = 40;
+static const int MI300A_MODEL_NUMBER = 0x90;
+static const int MI300C_MODEL_NUMBER = 0x80;
+static const int TURIN_FAMILY_ID = 0x1A;


This does not belong in generic ras.hpp file. Need a platform file to define this

supven01 · 2024-11-20T16:50:25Z

inc/ras.hpp

+static const int MI300A_MODEL_NUMBER = 0x90;
+static const int MI300C_MODEL_NUMBER = 0x80;
+static const int TURIN_FAMILY_ID = 0x1A;
+static const int GENOA_FAMILY_ID = 0x19;


This does not belong in generic ras.hpp file. Need a platform file to define this

supven01 · 2024-11-20T16:50:47Z

inc/ras.hpp

+static const int MI300C_MODEL_NUMBER = 0x80;
+static const int TURIN_FAMILY_ID = 0x1A;
+static const int GENOA_FAMILY_ID = 0x19;
+static const int EPYC_PROG_SEG_ID = 0x01;


This does not belong in generic ras.hpp file. Need a platform file to define this.

supven01 · 2024-11-20T16:50:52Z

inc/ras.hpp

+static const int TURIN_FAMILY_ID = 0x1A;
+static const int GENOA_FAMILY_ID = 0x19;
+static const int EPYC_PROG_SEG_ID = 0x01;
+static const int MI_PROG_SEG_ID = 0x02;


This does not belong in generic ras.hpp file. Need a platform file to define this

- Added README.md with project overview. - Added OWNERS file to define project ownership and contributors. - Added LICENSE file to specify project licensing terms. Signed-off-by: Abinaya Dhandapani <[email protected]>

Added clang-18 formatter. Signed-off-by: Abinaya Dhandapani <[email protected]>

ojayanth

Looks like still missing Key reveiw comments from previosu reveiws , can you please look my comments and respond with your plan , like ack/reject ?

ojayanth · 2024-11-22T13:11:50Z

config/ras_config.json

+            "ApmlRetries": {
+                "Description": "Number of APML retry count",
+                "Value": 10,
+                "MaxBoundLimit": "50"


Really going to try 50 times?

We should allow max bound limit as 50 as internal validation team may set that as high for some rare case failures and debug it.

ojayanth · 2024-11-22T13:12:29Z

config/ras_config.json

+            }
+        },
+        {
+            "SystemRecovery": {


SystemRecoveryMode ?

The RAS twiki page mentions SystemRecovery every wherehttps://twiki.amd.com/twiki/bin/view/SIG/Sp5OobRasSpec.
I hope we should also align with the same naming

ojayanth · 2024-11-22T13:14:06Z

config/ras_config.json

+        },
+        {
+            "ResetSignal": {
+                "Description": "Reset Signal Type",


is it Type or Name ? if it is type it should support mutiple right?

Reset signal can be of 2 types - SYS_RST or RSMRST reset. By default it is SYS_RST

ojayanth · 2024-11-22T14:00:43Z

config/ras_config.json

+                    "0x2c",
+                    "0x08",
+                    "0x0c",
+                    "null",


what is null indicates here?

These are the deafult values [30h, 34h, 28h, 2Ch, 08h, 0Ch, null, null], which specifies an ordered list of 4-byte aligned offsets to compute the failure signature ID

inc/ras.hpp

ojayanth · 2024-11-22T14:05:54Z

inc/interface_manager_base.hpp

@@ -0,0 +1,61 @@
+#include "config_manager.hpp"


Not addressed in the latest patch.

ojayanth · 2024-11-22T14:07:17Z

inc/apml_manager.hpp

@@ -0,0 +1,48 @@
+#include "cper_generator.hpp"


Will continue reveiw Only after addressing all the previous comments related coding style specific.

- The application monitors APML_ALERT_L and upon assertion of the GPIO PIN , BMC collects the MCA MSR dump and creates CPER record. - Depending on the user configuration , the application initiates system recovery. - ras-config.json contains the configuration parameters with default value. User can get and set the configuration parameters using d-bus calls to the methods getAttribute and setAttribute. - The application also contains harvesting of last transaction address, debug log ID's. - The application is intended to be supported for 1P and 2P platforms. Tested fields: Tested in sp5 platform. root@sp5:/var/lib/amd-ras# ls current_index ras-error0.cper ras-error2.cper ras-error4.cper ras-error6.cper ras-error8.cper ras-config.json ras-error1.cper ras-error3.cper ras-error5.cper ras-error7.cper ras-error9.cper Signed-off-by: Abinaya Dhandapani <[email protected]>

ojayanth

Making some Good progress , still need some more changes to get OPenBMC coding style . Added high level functional comments. Still seeing updated function prologues, . better get organize the code now and you can start fixing the previous comments. Also noticed Not follwing Naming conventions. - refer https://grok.openbmc.org/xref/openbmc/bmcweb/.clang-tidy?r=0771a264#378 for expeceted format.

ojayanth · 2024-11-26T12:49:02Z

src/main.cpp

+ limitations under the License.
+*/
+
+#include "apml_manager.hpp"


This file should be part if #ifdef APML

ojayanth · 2024-11-26T15:25:16Z

src/apml_manager.cpp

+
+#include <fstream>
+
+constexpr int EPYC_PROG_SEG_ID = 0x01;


General direction on constants handling ;

Use constexpr for Compile-Time Constants
Definition: Use constexpr for constants that can be evaluated at compile time.
Scope: Define constexpr constants in the smallest scope possible, such as within a function or a class, to limit their visibility.

Use static constexpr for Class-Level Constants
Definition: Use static constexpr for constants that belong to a class but should not be tied to any instance of the class.

Namespace-Level Constants
Definition: Use namespaces to group related constants together, which helps in organizing code and avoiding name conflicts.

Inline Variables for Header Files (C++17 and Later)
Definition: Use inline with constexpr for constants defined in header files to ensure there is only one definition across translation units.

Avoid Magic Numbers
Definition: Replace magic numbers with named constants to improve code readability and maintainability.

Use const for Non-Compile-Time Constants
Definition: Use const for constants that cannot be evaluated at compile time but should not change during runtime.

ojayanth · 2024-11-27T09:24:53Z

inc/interface_manager_base.hpp

+   Serviceability) configurations.*/
+class RasManagerBase : public RasConfiguration
+{
+  public:


Define default constructors explicitly if needed, and delete constructors that should not be used. Refer OpenBMCsource code related to class definition header files.

ojayanth · 2024-11-27T09:38:56Z

inc/interface_manager_base.hpp

+     *
+     * Virtual destructor ensures proper cleanup of derived class objects.
+     */
+    virtual ~RasManagerBase();


Nit: It will be good to keep, just after std constructor list , refer openbmc code.

ojayanth · 2024-11-27T09:45:38Z

inc/interface_manager_base.hpp

+  protected:
+    boost::asio::io_service& io;
+    uint8_t numOfCpu;
+    CpuId* cpuId;


General: Replaced raw pointers with std::unique_ptr for cpuId, uCode, and ppin to manage ownership and prevent memory leaks.

ojayanth · 2024-11-27T09:53:50Z

inc/interface_manager_base.hpp

+     */
+    virtual ~RasManagerBase();
+
+    /**


File Scope: This approach seems to be Not extendable design, Like creating processor specific function, What is the limitation for creating common function, and based on processor number act on it?

ojayanth · 2024-11-27T10:18:13Z

src/interface_manager_base.cpp

+constexpr char CPU_INVENTORY_INTERFACE[] =
+    "xyz.openbmc_project.Inventory.Item.Cpu";
+constexpr char COMMAND_NUM_OF_CPU[] = "/sbin/fw_printenv -n num_of_cpu";
+static const std::string COMMAND_BOARD_ID = "/sbin/fw_printenv -n board_id";


Replaced static const std::string with constexpr std::string_view for COMMAND_BOARD_ID. std::string_view is a lightweight, non-owning view of a string, which is more efficient for read-only string data2.

ojayanth · 2024-11-27T13:28:04Z

inc/apml_manager.hpp

+     *
+     * @return True if there is a match, false otherwise.
+     */
+    bool compare_with_bitwise_AND(const uint32_t* values,


Move to utility

ojayanth · 2024-11-27T13:30:45Z

inc/apml_manager.hpp

+     *
+     * @return A vector containing converted values.
+     */
+    std::vector<uint32_t> hexstring_to_vector(const std::string& hexString);


Move to util

ojayanth · 2024-11-27T13:33:11Z

inc/apml_manager.hpp

@@ -0,0 +1,181 @@
+#include "cper_generator.hpp"
+#include "interface_manager_base.hpp"


General: Enable Namespace based Code organization:
OpenBMC code is extensively used Namespaces in C++, a powerful feature for organizing code and preventing name conflicts, especially in large projects. Here’s a detailed look at namespace-based design in C++: Please refer any of the OpenBMC repo to see this approach .

Example:

// File: sensor_manager.h
namespace openbmc_project {
namespace sensor {
namespace manager {

class SensorManager { public: SensorManager(); void initialize(); private: void readSensorData(); }; } // namespace manager

} // namespace sensor
} // names

ojayanth · 2024-11-27T13:35:52Z

inc/apml_manager.hpp

+#include "cper_generator.hpp"
+#include "interface_manager_base.hpp"
+
+class ApmlInterfaceManager : public RasManagerBase


This class should use for implementing Interface function Logic using APML library API's
APML Librarry API's should move to APML library class/namaespace based on implmention front. this approch will help to organize and manage beter way.

ojayanth · 2024-11-27T13:39:36Z

inc/cper_generator.hpp

+template <typename T>
+class CperGenerator
+{
+  protected:


Be consistent on public/private/protectd definition sequence in the repo.

abinayaddhandapani requested review from supven01, srgovard, aasboddu, ojayanth, rajaganeshr, ashok2ku, modolaty and mahkurap November 13, 2024 16:14

abinayaddhandapani force-pushed the ras_Refactor branch from 7287f07 to 8c8c3ab Compare November 18, 2024 07:21

abinayaddhandapani force-pushed the ras_Refactor branch 2 times, most recently from 2b48bca to a1de8ec Compare November 19, 2024 14:49

ojayanth suggested changes Nov 19, 2024

View reviewed changes

abinayaddhandapani force-pushed the ras_Refactor branch 2 times, most recently from 7a3942d to 6a1908b Compare November 20, 2024 08:35

ojayanth suggested changes Nov 20, 2024

View reviewed changes

inc/ras.hpp Outdated Show resolved Hide resolved

ojayanth reviewed Nov 20, 2024

View reviewed changes

supven01 requested changes Nov 20, 2024

View reviewed changes

Abinaya Dhandapani added 2 commits November 21, 2024 05:40

Add README, OWNERS, and LICENSE files

3e2c8ed

- Added README.md with project overview. - Added OWNERS file to define project ownership and contributors. - Added LICENSE file to specify project licensing terms. Signed-off-by: Abinaya Dhandapani <[email protected]>

Add clang-format and prettier

bf04998

Added clang-18 formatter. Signed-off-by: Abinaya Dhandapani <[email protected]>

abinayaddhandapani force-pushed the ras_Refactor branch 2 times, most recently from 6b63ae9 to f831b26 Compare November 21, 2024 11:49

abinayaddhandapani force-pushed the ras_Refactor branch 2 times, most recently from d274822 to d0b4bf4 Compare November 21, 2024 11:52

ojayanth suggested changes Nov 22, 2024

View reviewed changes

abinayaddhandapani force-pushed the ras_Refactor branch from d0b4bf4 to 639b848 Compare November 25, 2024 14:06

abinayaddhandapani force-pushed the ras_Refactor branch from 639b848 to a13485a Compare November 25, 2024 14:10

ojayanth suggested changes Nov 26, 2024

View reviewed changes

ojayanth suggested changes Nov 27, 2024

View reviewed changes

ojayanth mentioned this pull request Dec 1, 2024

Initial patch for RAS configuration settings #99

Merged

		@@ -0,0 +1,181 @@
		#include "cper_generator.hpp"
		#include "interface_manager_base.hpp"

Add support for fatal error monitoring #98

Are you sure you want to change the base?

Add support for fatal error monitoring #98

Conversation

abinayaddhandapani commented Nov 13, 2024 • edited Loading

ojayanth commented Nov 18, 2024 • edited Loading

ojayanth commented Nov 18, 2024

ojayanth commented Nov 18, 2024 • edited Loading

abinayaddhandapani commented Nov 19, 2024

abinayaddhandapani commented Nov 19, 2024

abinayaddhandapani commented Nov 19, 2024

ojayanth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojayanth commented Nov 19, 2024

Choose a reason for hiding this comment

ojayanth commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojayanth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojayanth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abinayaddhandapani commented Nov 13, 2024 •

edited

Loading

ojayanth commented Nov 18, 2024 •

edited

Loading

ojayanth commented Nov 18, 2024 •

edited

Loading