PRDF:Added access attribute in model of register in rule file.

Adding access attribute in register model helps to determine at run time
if register expects any read and write operation. Since data is no longer part
of register model, this additional attribute helps us to prevent unintentional
reading of a write-only register while accessing its current content ( during
cache miss ).

RTC: 51455
Change-Id: Ia2aeded8b63d675a1d4b5182171bce4c8b14d17b
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/5080
Tested-by: Jenkins Server
Reviewed-by: Christopher T. Phan <cphan@us.ibm.com>
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>
Reviewed-by: Zane Shelley <zshelle@us.ibm.com>
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/5590

20 files changed, 363 insertions, 187 deletions

diff --git a/src/usr/diag/prdf/common/framework/register/iipscr.h b/src/usr/diag/prdf/common/framework/register/iipscr.h
index 90a31f3ff..722dcff77 100755
--- a/src/usr/diag/prdf/common/framework/register/iipscr.h
+++ b/src/usr/diag/prdf/common/framework/register/iipscr.h
@@ -42,11 +42,14 @@
 //----------------------------------------------------------------------
 
 #include <iipbits.h>
+#include <iipconst.h>
 #include <iipsdbug.h>
 #include <prdfMain.H>
+#include <prdfTrace.H>
 
 namespace PRDF
 {
+
 /*--------------------------------------------------------------------*/
 /*  Forward References                                                */
 /*--------------------------------------------------------------------*/
@@ -125,7 +128,18 @@ namespace PRDF
  */
 class SCAN_COMM_REGISTER_CLASS
 {
-public:
+  public: // enums, structs, typedefs
+
+    /** The register access level */
+    enum AccessLevel
+    {
+        ACCESS_NONE = 0x0, ///< No access
+        ACCESS_RO   = 0x1, ///< Read-only access
+        ACCESS_WO   = 0x2, ///< Write-only access
+        ACCESS_RW   = 0x3, ///< Read/Write access
+    };
+
+  public: // functions
 
   /**
    Destructor
@@ -143,7 +157,7 @@ public:
    <br><b>Sideaffects: </b> Value guarenteed to be read from hardware.
    <br><b>Exceptions:  </b> None.
    <br><b>Notes:       </b> Default is to call Read().  If a child class cannot
-                            guarentee hardware access everytime Read() is called
+                            guarantee hardware access everytime Read() is called
                             then the function ForceRead() should be overridden.
    </ul><br>
    */
@@ -359,6 +373,7 @@ public:
    */
   bool BitStringIsZero()
   { return GetBitString()->IsZero(); }
+
    /**
     *@brief    Returns TYPE_NA as type of Target associated with register.Actual
     *          implementation is expected in derived class
@@ -366,6 +381,12 @@ public:
    */
   virtual TARGETING::TYPE getChipType(void)const { return TARGETING::TYPE_NA; }
 
+    /** @return The register access level (see enum AccessLevel). */
+    virtual AccessLevel getAccessLevel() const { return ACCESS_RW; }
+
+    /** @brief Sets the register access level (see enum AccessLevel). */
+    virtual void setAccessLevel( AccessLevel i_op ) {}
+
 protected:
 
   /**
diff --git a/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.C b/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.C
index 2c168e2e8..be23f2d00 100644
--- a/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.C
+++ b/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.C
@@ -43,25 +43,20 @@ RegDataCache::~RegDataCache()
 
 //------------------------------------------------------------------------------
 
-BIT_STRING_CLASS & RegDataCache::read(
-                                ExtensibleChip* i_pChip,
-                                const SCAN_COMM_REGISTER_CLASS * i_pRegister,
-                                bool & o_readStat )
+BIT_STRING_CLASS & RegDataCache::read( ExtensibleChip * i_chip,
+                                       const SCAN_COMM_REGISTER_CLASS * i_reg )
 {
-    ScomRegisterAccess l_scomAccessKey ( *i_pRegister,i_pChip );
+    ScomRegisterAccess l_scomAccessKey ( *i_reg, i_chip );
     BIT_STRING_CLASS * l_pBitString = queryCache( l_scomAccessKey );
-    o_readStat = false;
-    if( NULL == l_pBitString )
+
+    if ( NULL == l_pBitString )
     {
         // Creating new entry
-        l_pBitString = new BitStringBuffer( i_pRegister->GetBitLength( ) );
+        l_pBitString = new BitStringBuffer( i_reg->GetBitLength() );
         // Adding register in the cache
         iv_cachedRead[l_scomAccessKey] = l_pBitString;
     }
-    else
-    {
-        o_readStat = true;
-    }
+
     return *l_pBitString;
 }
 
diff --git a/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.H b/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.H
index 411937af6..648faee63 100644
--- a/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.H
+++ b/src/usr/diag/prdf/common/framework/register/prdfRegisterCache.H
@@ -72,18 +72,12 @@ class RegDataCache
 
     /**
      * @brief Returns the data buffer for the given target and address.
-     * @param i_pChip       The target associated with the register.
-     * @param i_pRegister   pointer to register to be read.
-     * @param o_readStat    Returns true if the register does not exist in
-     *                      cache. In this case, the function will create and
-     *                      add an empty BIT_STRING_CLASS to the cache.It is
-     *                      the responsibilty of the user to update the data
-     *                      buffer by reading from hardware.
+     * @param i_chip The target associated with the register.
+     * @param i_reg  Pointer to register to be read.
      * @return A reference to the data buffer associated with the register.
      */
-     BIT_STRING_CLASS & read( ExtensibleChip* i_pChip,
-                              const SCAN_COMM_REGISTER_CLASS * i_pRegister,
-                              bool & o_readStat );
+    BIT_STRING_CLASS & read( ExtensibleChip * i_chip,
+                             const SCAN_COMM_REGISTER_CLASS * i_reg );
 
     /**
      * @brief Flushes entire contents from cache.
diff --git a/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C b/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C
index e8e35ab68..8eed62ca6 100755
--- a/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C
+++ b/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C
@@ -70,11 +70,23 @@ ScanFacility & ScanFacility::Access(void)
   return sf;
 }
 //-----------------------------------------------------------------------------
-SCAN_COMM_REGISTER_CLASS & ScanFacility::GetScanCommRegister(
-    uint64_t address , uint32_t i_scomLength,TARGETING::TYPE i_type  )
+SCAN_COMM_REGISTER_CLASS & ScanFacility::GetScanCommRegister( uint64_t address,
+                                uint32_t i_scomLength, TARGETING::TYPE i_type,
+                                SCAN_COMM_REGISTER_CLASS::AccessLevel i_regOp )
 {
-  ScomRegister scrKey(address ,i_scomLength ,i_type );
-  return iv_scomRegFw.get(scrKey);
+    /* i_regOp is not used to determine uniqueness of the object for following
+      reason -
+      There can not be two registers in hardware with same address and target
+      type supporting different operations say one supports only write and
+      other both read and write.
+      */
+
+    ScomRegister scrKey( address, i_scomLength, i_type, i_regOp );
+    // in case we get a object with different default operation, we shall reset
+    // it to what it should be as per rule file.
+    ScomRegister &regCreated = iv_scomRegFw.get(scrKey);
+    regCreated.setAccessLevel( i_regOp );
+    return regCreated;
 }
 //------------------------------------------------------------------------------
 
diff --git a/src/usr/diag/prdf/common/framework/register/prdfScanFacility.H b/src/usr/diag/prdf/common/framework/register/prdfScanFacility.H
index 65494a75b..22cc41a42 100755
--- a/src/usr/diag/prdf/common/framework/register/prdfScanFacility.H
+++ b/src/usr/diag/prdf/common/framework/register/prdfScanFacility.H
@@ -83,11 +83,12 @@ public:
     * @param     i_address        address of the register
     * @param     i_scomLength     length of the bit string
     * @param     i_type           type of target associated with register
+    * @param     i_regOp          operations supported for given register
     * @return   returns reference to flyweight object from factory
     */
-   SCAN_COMM_REGISTER_CLASS & GetScanCommRegister(
-                    uint64_t address ,uint32_t i_scomLength,
-                    TARGETING::TYPE i_type );
+   SCAN_COMM_REGISTER_CLASS & GetScanCommRegister( uint64_t address,
+                                uint32_t i_scomLength, TARGETING::TYPE i_type,
+                                SCAN_COMM_REGISTER_CLASS::AccessLevel i_regOp );
 
   /**
    Get a scan ring register
diff --git a/src/usr/diag/prdf/common/framework/register/prdfScomRegister.C b/src/usr/diag/prdf/common/framework/register/prdfScomRegister.C
index 94595de71..ba7617bb4 100755
--- a/src/usr/diag/prdf/common/framework/register/prdfScomRegister.C
+++ b/src/usr/diag/prdf/common/framework/register/prdfScomRegister.C
@@ -80,91 +80,132 @@ void ScomRegister::SetBitString( const BIT_STRING_CLASS *bs )
 }
 
 
-// ---------------------------------------------------------------------
+//------------------------------------------------------------------------------
 
-const BIT_STRING_CLASS *ScomRegister::GetBitString( ATTENTION_TYPE i_type )const
+const BIT_STRING_CLASS * ScomRegister::GetBitString(ATTENTION_TYPE i_type) const
 {
-    BIT_STRING_CLASS * l_pString = NULL;
-    bool l_readStat = false;
-    //Expectation is, caller shall first call Read( ) and then GetBitString.
-    //This leaves an opportunity of mistake. One may call GetBitString without
-    //calling Read() first. As a result, a stray entry in cache gets created
-    //which  shall never be in sync with hardware.
-
-    //As a solution, first cache is queried.If the given entry exist, bitString
-    //pointer is returned else a new entry is created. This new entry is
-    //synchronized with hardware and then pointer to bit string is returned to
-    //caller.
-    RegDataCache & regDump = RegDataCache::getCachedRegisters();
-    l_pString = regDump.queryCache( getChip( ), this );
-
-    if( NULL == l_pString )
+    // Calling Read() will ensure that an entry exists in the cache and the
+    // entry has at been synched with hardware at least once. Note that we
+    // cannot read hardware for write-only registers. In this case, an entry
+    // will be created in the cache, if it does not exist, when readCache() is
+    // called below.
+    if ( ( ACCESS_NONE != iv_operationType ) &&
+            ( ACCESS_WO != iv_operationType ) )
     {
-        ForceRead( );
-        //if ForceRead fails, a dummy entry is returned that way analysis shall
-        //fail gracefully else we return a new entry which is in sync with
-        //hardware
-        l_pString = &( readCache( l_readStat ) );
-
+        Read();
     }
-    return l_pString;
+    return &(readCache());
 }
-// ---------------------------------------------------------------------
-BIT_STRING_CLASS & ScomRegister::AccessBitString( )
+
+//------------------------------------------------------------------------------
+
+BIT_STRING_CLASS & ScomRegister::AccessBitString()
 {
-    bool l_readStat = false;
-    //FIXME RTC 51455 We need to find a way to make AccessBitString independent
-    //of Read. Calling Read inside this function shall make it read even
-    //write only registers. This operation shall fail on actual hardware.
-    return ( readCache( l_readStat ) );
+    // Calling Read() will ensure that an entry exists in the cache and the
+    // entry has at been synched with hardware at least once. Note that we
+    // cannot read hardware for write-only registers. In this case, an entry
+    // will be created in the cache, if it does not exist, when readCache() is
+    // called below.
+    if ( ( ACCESS_NONE != iv_operationType ) &&
+            ( ACCESS_WO != iv_operationType ) )
+    {
+        Read();
+    }
 
+    return readCache();
 }
 
 //------------------------------------------------------------------------------
 
 uint32_t ScomRegister::Read() const
 {
-    int32_t rc = SUCCESS;
-    bool l_readStat = false;
-    readCache( l_readStat );
-    if( false == l_readStat )
+    uint32_t o_rc = SUCCESS;
+
+    // First query the cache for an existing entry.
+    if ( !queryCache() )
     {
-        //updating cache by reading hardware .So next read  need not access
-        //hardware
-        rc = ForceRead();
+        // There was not a previous entry in the cache, so do a ForceRead() to
+        // sync the cache with hardware.
+        o_rc = ForceRead();
     }
 
-    return(rc);
+    return o_rc;
 }
 
 //------------------------------------------------------------------------------
 
 uint32_t ScomRegister::ForceRead() const
 {
-    int32_t rc = SUCCESS;
-    bool l_readStat = false;
-    BIT_STRING_CLASS & bs = readCache( l_readStat );
-    rc = Access( bs,MopRegisterAccess::READ );
-    if( SUCCESS != rc )
-    {
-        ExtensibleChip* l_pChip = getChip( );
-        flushCache( l_pChip );
-    }
+    #define PRDF_FUNC "[ScomRegister::ForceRead] "
+
+    uint32_t o_rc = FAIL;
 
-    return rc;
+    do
+    {
+        // No read allowed if register access attribute is write-only or no
+        // access.
+        if ( ( ACCESS_NONE == iv_operationType ) &&
+                ( ACCESS_WO == iv_operationType ) )
+        {
+            PRDF_ERR( PRDF_FUNC"Write-only register: 0x%08x 0x%016llx",
+                      getChip()->GetId(), iv_scomAddress );
+            break;
+        }
+
+        // Read hardware.
+        o_rc = Access( readCache(), MopRegisterAccess::READ );
+        if ( SUCCESS != o_rc )
+        {
+            // The read failed. Remove the entry from the cache so a subsequent
+            // Read() will attempt to read from hardware again.
+            flushCache( getChip() );
+        }
+
+    } while (0);
+
+    return o_rc;
+
+    #undef PRDF_FUNC
 }
 
 //------------------------------------------------------------------------------
 
 uint32_t ScomRegister::Write()
 {
-    uint32_t rc = FAIL;
-    bool l_entryBeforeWrite = false;
-    BIT_STRING_CLASS & bs = readCache( l_entryBeforeWrite );
-    PRDF_ASSERT( true == l_entryBeforeWrite );
-    rc = Access( bs, MopRegisterAccess::WRITE );
+    #define PRDF_FUNC "[ScomRegister::Write] "
 
-    return(rc);
+    uint32_t o_rc = FAIL;
+
+    do
+    {
+        // No write allowed if register access attribute is read-only or no
+        // access.
+        if ( ( ACCESS_NONE == iv_operationType ) &&
+                 ( ACCESS_RO == iv_operationType ) )
+        {
+            PRDF_ERR( PRDF_FUNC"Read-only register: 0x%08x 0x%016llx",
+                      getChip()->GetId(), iv_scomAddress );
+            break;
+        }
+
+        // Query the cache for an existing entry.
+        if ( !queryCache() )
+        {
+            // Something bad happened and there was nothing in the cache to
+            // write to hardware.
+            PRDF_ERR( PRDF_FUNC"No entry found in cache: 0x%08x 0x%016llx",
+                      getChip()->GetId(), iv_scomAddress );
+            break;
+        }
+
+        // Write hardware.
+        o_rc = Access( readCache(), MopRegisterAccess::WRITE );
+
+    } while (0);
+
+    return o_rc;
+
+    #undef PRDF_FUNC
 }
 
 //------------------------------------------------------------------------------
@@ -188,15 +229,25 @@ ExtensibleChip* ScomRegister::getChip( )const
     PRDF_ASSERT( iv_chipType == l_type )
     return l_pchip;
 }
-//-----------------------------------------------------------------------------
-BIT_STRING_CLASS & ScomRegister::readCache( bool & o_existingEntry ) const
+
+//------------------------------------------------------------------------------
+
+bool ScomRegister::queryCache() const
 {
-    ExtensibleChip* l_pChip = getChip( );
-    RegDataCache & regDump = RegDataCache::getCachedRegisters();
-    return  regDump.read( l_pChip,this,o_existingEntry );
+    RegDataCache & cache = RegDataCache::getCachedRegisters();
+    BIT_STRING_CLASS * bs = cache.queryCache( getChip(), this );
+    return ( NULL != bs );
+}
 
+//------------------------------------------------------------------------------
+
+BIT_STRING_CLASS & ScomRegister::readCache() const
+{
+    RegDataCache & cache = RegDataCache::getCachedRegisters();
+    return cache.read( getChip(), this );
 }
-//-----------------------------------------------------------------------------
+
+//------------------------------------------------------------------------------
 
 void ScomRegister::flushCache( ExtensibleChip *i_pChip ) const
 {
diff --git a/src/usr/diag/prdf/common/framework/register/prdfScomRegister.H b/src/usr/diag/prdf/common/framework/register/prdfScomRegister.H
index 18e03161a..91ecfbc46 100755
--- a/src/usr/diag/prdf/common/framework/register/prdfScomRegister.H
+++ b/src/usr/diag/prdf/common/framework/register/prdfScomRegister.H
@@ -37,6 +37,7 @@
 #include <iipscr.h>
 #include <iipbits.h>
 #include <iipMopRegisterAccess.h>
+#include <prdfTrace.H>
 
 namespace PRDF
 {
@@ -46,11 +47,9 @@ class CHIP_CLASS;
 class MopsRegisterAccess;
 class ExtensibleChip;
 
-
-
 class ScomRegister : public SCAN_COMM_REGISTER_CLASS
 {
-public:
+  public:
 
     /**
      * @brief     constructor
@@ -58,25 +57,26 @@ public:
      * @param     i_bitLength      bit length of register
      * @param     i_targetType     target type associated with register
      */
-    ScomRegister( uint64_t i_address ,uint32_t i_bitLength,TARGETING::TYPE
-                    i_targetType ) :
+    ScomRegister( uint64_t i_address, uint32_t i_bitLength,
+                  TARGETING::TYPE i_targetType, AccessLevel i_access ) :
         SCAN_COMM_REGISTER_CLASS(),
         iv_bitLength( i_bitLength ),
         iv_chipType( i_targetType ),
-        iv_scomAddress( i_address )
-    { }
+        iv_scomAddress( i_address ),
+        iv_operationType( i_access )
+    {}
 
     /**
      * @brief     constructor .Added this because we save object of this type in
      * @          FlyweightS
      */
     ScomRegister():
-        SCAN_COMM_REGISTER_CLASS( ),
-        iv_bitLength(0),
-        iv_chipType(TARGETING::TYPE_NA),iv_scomAddress(0)
-    { }
-
-
+        SCAN_COMM_REGISTER_CLASS(),
+        iv_bitLength( 0 ),
+        iv_chipType( TARGETING::TYPE_NA ),
+        iv_scomAddress( 0 ),
+        iv_operationType( ACCESS_NONE )
+    {}
 
     /**
      * @brief     Returns the pointer to bit string
@@ -165,19 +165,26 @@ public:
     */
    bool operator >= ( const ScomRegister & i_rightRegister  ) const;
 
-    protected: // Functions
+    /** @return The register access level (see enum AccessLevel). */
+    virtual AccessLevel getAccessLevel() const { return iv_operationType; }
+
+    /** @brief Sets the register access level (see enum AccessLevel). */
+    virtual void setAccessLevel( AccessLevel i_op ) { iv_operationType = i_op; }
+
+  protected: // Functions
+
     /**
      * @brief     copy constructor
      * @param     i_scomRegister   scomRegister instance to be copied
      */
     ScomRegister( const SCAN_COMM_REGISTER_CLASS & i_scomRegister ):
         SCAN_COMM_REGISTER_CLASS(),
-        iv_bitLength( i_scomRegister.GetBitLength() ),
-        iv_shortId( i_scomRegister.GetId()           ),
-        iv_chipType( i_scomRegister.getChipType()    ),
-        iv_scomAddress( i_scomRegister.GetAddress()  )
-    { }
-
+        iv_bitLength(     i_scomRegister.GetBitLength()   ),
+        iv_shortId(       i_scomRegister.GetId()          ),
+        iv_chipType(      i_scomRegister.getChipType()    ),
+        iv_scomAddress(   i_scomRegister.GetAddress()     ),
+        iv_operationType( i_scomRegister.getAccessLevel() )
+    {}
 
    /**
     * @brief    Returns reference to bit string associated with register
@@ -193,37 +200,41 @@ public:
      */
     uint32_t Access( BIT_STRING_CLASS & bs,
                      MopRegisterAccess::Operation op )const;
-   /**
-    * @brief    Returns rulechip poiner associated with the register
-    * @return   Refer to function description
-   */
-
-    virtual ExtensibleChip* getChip( )const ;
 
+    /**
+     * @brief  Returns rulechip pointer associated with the register
+     * @return Refer to function description
+     */
+    virtual ExtensibleChip * getChip() const;
 
 private: // functions
 
   friend class CaptureData;
+
+    /** @return TRUE if entry for this register exist in this cache. */
+    bool queryCache() const;
+
     /**
-     * @brief     Reads register contents from cache
-     * @param     reference to bool .Expected to be set to true if entry in
-     *            cache already exist  and false if it is a  newly created
-     *            entry.
-     * @return    Reference to bit string buffer maintained in cache
+     * @brief  Reads register contents from cache.
+     * @return Reference to bit string buffer maintained in cache.
      */
-  BIT_STRING_CLASS & readCache( bool & o_existingEntry ) const;
+    BIT_STRING_CLASS & readCache() const;
+
     /**
      * @brief     Deletes one or all entry in the cache
      * @param     RuleChip pointer associated with register
      * @return    Nil
      */
-  void flushCache( ExtensibleChip *i_pChip = NULL ) const ;
+    void flushCache( ExtensibleChip *i_pChip = NULL ) const;
+
+  private: // Data
+
+    uint32_t        iv_bitLength;     // bit length of scom
+    uint16_t        iv_shortId;       // unique hash id of register
+    TARGETING::TYPE iv_chipType;      // type of target associated with register
+    uint64_t        iv_scomAddress;   // scom address associated with regiser
+    AccessLevel     iv_operationType; // Operation supported (RO, WO, or RW)
 
-private: // Data
-    uint32_t iv_bitLength;           // bit length of scom
-    uint16_t iv_shortId;              // unique hash id of register
-    TARGETING::TYPE iv_chipType ;     // type of target associated with register
-    uint64_t iv_scomAddress;          // scom address associated with regiser
 };
 
 }//namespace PRDF ends
diff --git a/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.C b/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.C
index 3d076d16e..2e944876c 100644
--- a/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.C
+++ b/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.C
@@ -29,25 +29,11 @@ namespace PRDF
 {
 
 ScomRegisterAccess::ScomRegisterAccess(
-                    const SCAN_COMM_REGISTER_CLASS  &i_pRegister ,
-                    ExtensibleChip* i_pRuleChip
-                    )
-                    :ScomRegister( i_pRegister ),
-                     iv_containerChip( i_pRuleChip )
-{
-}
-
-//----------------------------------------------------------------------
-
-ScomRegisterAccess::ScomRegisterAccess( uint64_t i_scomAddress,
-                                        uint32_t i_bitLength,
-                                        ExtensibleChip* i_pRuleChip ):
-    ScomRegister( i_scomAddress ,i_bitLength,
-                  PlatServices::getTargetType( i_pRuleChip->GetChipHandle())),
+                    const SCAN_COMM_REGISTER_CLASS & i_pRegister,
+                    ExtensibleChip * i_pRuleChip ) :
+    ScomRegister( i_pRegister ),
     iv_containerChip( i_pRuleChip )
-{
-}
-
+{}
 
 //----------------------------------------------------------------------
 
diff --git a/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.H b/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.H
index 84dcbb930..a8317d0cd 100644
--- a/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.H
+++ b/src/usr/diag/prdf/common/framework/register/prdfScomRegisterAccess.H
@@ -58,20 +58,12 @@ class ScomRegisterAccess : public ScomRegister
      * @brief     constructor
      * @param     i_Register     Reference to flyweight register
      * @param     i_pchip        RuleChip associated with register
-    */
+     */
     ScomRegisterAccess( const SCAN_COMM_REGISTER_CLASS & i_Register,
                         ExtensibleChip* i_pchip );
     /**
      * @brief     constructor
-     * @param     i_scomAddress   scom address of register
-     * @param     i_bitLength     scom length
-     * @param     i_pchip         RuleChip to which register belongs
-    */
-    ScomRegisterAccess( uint64_t i_scomAddress,uint32_t i_bitLength,
-                        ExtensibleChip* i_pchip );
-    /**
-     * @brief     constructor
-    */
+     */
     ScomRegisterAccess():ScomRegister( ),iv_containerChip ( NULL ){ };
 
     /**
diff --git a/src/usr/diag/prdf/common/framework/rule/prdfRuleMetaData.C b/src/usr/diag/prdf/common/framework/rule/prdfRuleMetaData.C
index 39a919f9b..5be48a503 100644
--- a/src/usr/diag/prdf/common/framework/rule/prdfRuleMetaData.C
+++ b/src/usr/diag/prdf/common/framework/rule/prdfRuleMetaData.C
@@ -40,6 +40,7 @@
 #include <utilfile.H>  // for UtilFile
 #include <UtilHash.H> // for Util::hashString
 #include <prdfPfa5Data.h>
+#include <prdrCommon.H>
 
 namespace PRDF
 {
@@ -61,12 +62,27 @@ struct ResetAndMaskTransformer
     virtual ResetAndMaskErrorRegister::ResetRegisterStruct
         operator()( const Prdr::Register::ResetOrMaskStruct & i )
     {
+        /*
+          These reset and mask registers, which are associated with a FIR, are
+          created along with the FIR. They are needed to clear/mask FIR bits at
+          the end of analysis. Let us call it scenario X.
+
+          There are situations where we need a separate instance of an AND
+          register. These are used in plugins to do some special analysis which
+          is not possible through framework code. Let us call it scenario Y.
+
+          Since, factory actually doesn't use operations (e.g. READ or WRITE)
+          supported to determine whether a register instance is new or already
+          available it shall return us the AND register instance created in
+          scenario X.
+
+          AND registers are write only. If we don't specify WRITE here, even in
+          scenario Y, we shall get object created during scenario X, which may
+          not be write-only.
+        */
         ResetAndMaskErrorRegister::ResetRegisterStruct o;
-        o.read = & cv_scanFactory.GetScanCommRegister( i.addr_r ,cv_scomlen,
-                                                        iv_chipType );
-
-        o.write = & cv_scanFactory.GetScanCommRegister( i.addr_w ,cv_scomlen ,
-                                                        iv_chipType );
+        SCAN_COMM_REGISTER_CLASS::AccessLevel l_readRegAccess =
+                                            SCAN_COMM_REGISTER_CLASS::ACCESS_WO;
 
 
         switch ( i.op )
@@ -81,6 +97,7 @@ struct ResetAndMaskTransformer
 
             case Prdr::XOR:
                 o.op = getStaticResetOperator<XorOperator<Type> >();
+                l_readRegAccess = SCAN_COMM_REGISTER_CLASS::ACCESS_RW;
                 break;
 
             case Prdr::NOT:
@@ -92,6 +109,14 @@ struct ResetAndMaskTransformer
                 break;
         }
 
+        o.read = & cv_scanFactory.GetScanCommRegister( i.addr_r, cv_scomlen,
+                                        iv_chipType,
+                                        l_readRegAccess );
+
+        o.write = & cv_scanFactory.GetScanCommRegister( i.addr_w, cv_scomlen,
+                                        iv_chipType,
+                                        SCAN_COMM_REGISTER_CLASS::ACCESS_WO );
+
         return o;
     };
 
@@ -169,12 +194,37 @@ errlHndl_t RuleMetaData::loadRuleFile( ScanFacility & i_scanFactory ,
         for (int i = 0; i < l_chip->cv_regCount; i++)
         {
             uint16_t hashId = l_chip->cv_registers[i].cv_name;
+            SCAN_COMM_REGISTER_CLASS::AccessLevel l_regAccess;
+
+            uint32_t l_accessLvls =
+                        Prdr::PRDR_REGISTER_READ |
+                        Prdr::PRDR_REGISTER_WRITE |
+                        Prdr::PRDR_REGISTER_ACCESS_NIL;
+
+            uint32_t l_accessType =
+                        l_chip->cv_registers[i].cv_flags & l_accessLvls;
 
+            switch( l_accessType )
+            {
+                case Prdr::PRDR_REGISTER_ACCESS_NIL:
+                    l_regAccess = SCAN_COMM_REGISTER_CLASS::ACCESS_NONE;
+                    break;
+                case Prdr::PRDR_REGISTER_WRITE:
+                    l_regAccess = SCAN_COMM_REGISTER_CLASS::ACCESS_WO;
+                    break;
+                case Prdr::PRDR_REGISTER_READ:
+                    l_regAccess = SCAN_COMM_REGISTER_CLASS::ACCESS_RO;
+                    break;
+                default:
+                    l_regAccess = SCAN_COMM_REGISTER_CLASS::ACCESS_RW;
+            }
+            //If more operations are incorporated in register, it can be
+            //specified in same way as above.
             l_regMap[l_id] = cv_hwRegs[hashId]
                                 = &i_scanFactory.GetScanCommRegister(
                                         l_chip->cv_registers[i].cv_scomAddr,
                                         l_chip->cv_registers[i].cv_scomLen,
-                                        i_type );
+                                        i_type, l_regAccess );
             l_regMap[l_id]->SetId(hashId);
 
             // Copy reset registers.
diff --git a/src/usr/diag/prdf/common/framework/rule/prdrCommon.H b/src/usr/diag/prdf/common/framework/rule/prdrCommon.H
index c96ecc24f..644f96661 100755
--- a/src/usr/diag/prdf/common/framework/rule/prdrCommon.H
+++ b/src/usr/diag/prdf/common/framework/rule/prdrCommon.H
@@ -60,14 +60,13 @@ namespace Prdr
      */
     enum RegisterFlags
     {
-            /** Non-default scomlen */
-        PRDR_REGISTER_SCOMLEN = 0x1,
-            /** Resets defined */
-        PRDR_REGISTER_RESETS = 0x2,
-            /** Masks defined */
-        PRDR_REGISTER_MASKS = 0x4,
-            /** Capture requirements */
-        PRDR_REGISTER_CAPTURE = 0x8,
+        PRDR_REGISTER_SCOMLEN       = 0x01, ///< Non-default scomlen
+        PRDR_REGISTER_RESETS        = 0x02, ///< Resets defined
+        PRDR_REGISTER_MASKS         = 0x04, ///< Masks defined
+        PRDR_REGISTER_CAPTURE       = 0x08, ///< Capture requirements
+        PRDR_REGISTER_ACCESS_NIL    = 0x10, ///< FFDC only register
+        PRDR_REGISTER_READ          = 0x20, ///< Read only register
+        PRDR_REGISTER_WRITE         = 0x40, ///< Write only register
     };
 
     /**
diff --git a/src/usr/diag/prdf/common/framework/rule/prdrCompile.lex b/src/usr/diag/prdf/common/framework/rule/prdrCompile.lex
index 1b9673b2a..67d8445ed 100755
--- a/src/usr/diag/prdf/common/framework/rule/prdrCompile.lex
+++ b/src/usr/diag/prdf/common/framework/rule/prdrCompile.lex
@@ -101,6 +101,10 @@ register    { return PRDR_REGISTER; }
 name        { return PRDR_NAME_KW; }
 scomaddr    { return PRDR_SCOMADDR; }
 scomlen     { return PRDR_SCOMLEN; }
+access      { return PRDR_REGISTER_ACCESS; }
+read_only   { return PRDR_REGISTER_READ_ACCESS; }
+write_only  { return PRDR_REGISTER_WRITE_ACCESS; }
+no_access   { return PRDR_REGISTER_NO_ACCESS; }
 bit         { return PRDR_BIT_KW; }
 reset       { return PRDR_RESET_ADDR; }
 mask        { return PRDR_MASK_ADDR; }
diff --git a/src/usr/diag/prdf/common/framework/rule/prdrCompile.y b/src/usr/diag/prdf/common/framework/rule/prdrCompile.y
index 2e6f0b725..1c270217d 100755
--- a/src/usr/diag/prdf/common/framework/rule/prdrCompile.y
+++ b/src/usr/diag/prdf/common/framework/rule/prdrCompile.y
@@ -117,6 +117,10 @@ using namespace PRDR_COMPILER;
 %token PRDR_NAME_KW
 %token PRDR_SCOMADDR
 %token PRDR_SCOMLEN
+%token PRDR_REGISTER_ACCESS
+%token PRDR_REGISTER_READ_ACCESS
+%token PRDR_REGISTER_WRITE_ACCESS
+%token PRDR_REGISTER_NO_ACCESS
 %token PRDR_RESET_ADDR
 %token PRDR_MASK_ADDR
 %token PRDR_BIT_KW
@@ -431,6 +435,21 @@ regline:    { $$ = NULL; }
 
         $$->cv_captures.push_back(tmp);
     }
+        | PRDR_REGISTER_ACCESS PRDR_REGISTER_READ_ACCESS
+    {
+        $$ = new Register();
+        $$->cv_flags |= Prdr::PRDR_REGISTER_READ;
+    }
+        | PRDR_REGISTER_ACCESS PRDR_REGISTER_WRITE_ACCESS
+    {
+        $$ = new Register();
+        $$->cv_flags |= Prdr::PRDR_REGISTER_WRITE;
+    }
+        | PRDR_REGISTER_ACCESS PRDR_REGISTER_NO_ACCESS
+    {
+        $$ = new Register();
+        $$->cv_flags |= Prdr::PRDR_REGISTER_ACCESS_NIL;
+    }
 ;
 
     /* Define the possible reset/mask instructions. */
diff --git a/src/usr/diag/prdf/common/framework/rule/prdrRegister.H b/src/usr/diag/prdf/common/framework/rule/prdrRegister.H
index af6d57649..a19e8aa4a 100755
--- a/src/usr/diag/prdf/common/framework/rule/prdrRegister.H
+++ b/src/usr/diag/prdf/common/framework/rule/prdrRegister.H
@@ -176,16 +176,26 @@ class Register
 
     static void merge(Register * i_l, Register * i_r)
     {
-        if (NULL != i_r->cv_sname)
+        if ( NULL != i_r->cv_sname )
+        {
             i_l->cv_sname = i_r->cv_sname;
-        if (&i_r->cv_name_default != i_r->cv_name)
+        }
+        if ( &i_r->cv_name_default != i_r->cv_name )
+        {
             i_l->cv_name = i_r->cv_name;
-        if (0 != i_r->cv_flags)
-            i_l->cv_flags = i_r->cv_flags;
-        if (0 != i_r->cv_scomaddr)
+        }
+        if ( 0 != i_r->cv_flags )
+        {
+            i_l->cv_flags |= i_r->cv_flags;
+        }
+        if ( 0 != i_r->cv_scomaddr )
+        {
             i_l->cv_scomaddr = i_r->cv_scomaddr;
-        if (0 != i_r->cv_scomlen)
+        }
+        if ( 0 != i_r->cv_scomlen )
+        {
             i_l->cv_scomlen = i_r->cv_scomlen;
+        }
         if (0 != i_r->cv_resets.size())
         {
             std::copy( i_r->cv_resets.begin(),
@@ -214,7 +224,20 @@ class Register
         cout << "\tLong Name: " << *cv_name << endl;
         cout << "\tScom Addr: " << cv_scomaddr << endl;
         cout << "\tScom Len:  " << cv_scomlen << endl;
-    };
+        cout << "\tAccess:    ";
+        uint32_t accessLevels =
+                        Prdr::PRDR_REGISTER_WRITE |
+                        Prdr::PRDR_REGISTER_READ |
+                        Prdr::PRDR_REGISTER_ACCESS_NIL;
+
+        switch( cv_flags & accessLevels )
+        {
+            case Prdr::PRDR_REGISTER_ACCESS_NIL:    cout << "No access";  break;
+            case Prdr::PRDR_REGISTER_WRITE:         cout << "Write only"; break;
+            case Prdr::PRDR_REGISTER_READ:          cout << "Read only";  break;
+            default:                                cout << "Read/Write";
+        }
+    }
 
     int output( FILE * l_file, uint16_t i_sigOff )
     {
@@ -247,6 +270,17 @@ class Register
         if (0 != cv_captures.size())
             cv_flags |= Prdr::PRDR_REGISTER_CAPTURE;
 
+        uint32_t readWriteAccess =
+                            Prdr::PRDR_REGISTER_WRITE |
+                            Prdr::PRDR_REGISTER_READ  |
+                            Prdr::PRDR_REGISTER_ACCESS_NIL;
+
+        if ( 0 == (readWriteAccess & cv_flags) )
+        {
+            // No access has been specified so default to Read/Write access.
+            cv_flags |= readWriteAccess;
+        }
+
         // output data
         uint32_t l_temp;
         uint16_t l_temp16;
diff --git a/src/usr/diag/prdf/common/plat/pegasus/Mba.rule b/src/usr/diag/prdf/common/plat/pegasus/Mba.rule
index 9c5353316..586ed609d 100755
--- a/src/usr/diag/prdf/common/plat/pegasus/Mba.rule
+++ b/src/usr/diag/prdf/common/plat/pegasus/Mba.rule
@@ -126,6 +126,7 @@ chip Mba
         name        "DPHY01.PHY01_DDRPHY_FIR_REG_AND";
         scomaddr    0x800200910301143F;
         capture     group never;
+        access      write_only;
     };
 
     register MBADDRPHYFIR_MASK
@@ -205,6 +206,7 @@ chip Mba
         name        "MBU.MBA01.MBA_MCBIST.SCOMFIR.MBSPAQ_AND";
         scomaddr    0x03010612;
         capture     group never;
+        access      write_only;
     };
 
     register MBASPA_MASK
diff --git a/src/usr/diag/prdf/common/plat/pegasus/Mcs.rule b/src/usr/diag/prdf/common/plat/pegasus/Mcs.rule
index 91f47d8a9..f130e852b 100755
--- a/src/usr/diag/prdf/common/plat/pegasus/Mcs.rule
+++ b/src/usr/diag/prdf/common/plat/pegasus/Mcs.rule
@@ -82,6 +82,7 @@ chip Mcs
         name        "MC0.MCS0.RIGHT.MCI.SCOMFIR.MCIFIRQ AND";
         scomaddr    0x02011841;
         capture     group never;
+        access      write_only;
     };
 
     register MCIFIR_MASK
diff --git a/src/usr/diag/prdf/common/plat/pegasus/Membuf.rule b/src/usr/diag/prdf/common/plat/pegasus/Membuf.rule
index 1aaaef3de..809e8f2a5 100755
--- a/src/usr/diag/prdf/common/plat/pegasus/Membuf.rule
+++ b/src/usr/diag/prdf/common/plat/pegasus/Membuf.rule
@@ -96,7 +96,7 @@ chip Membuf
     {
         name        "Bit map 0-63 of failed lanes read from VPD";
         scomaddr    0xFFFF0001;
-
+        access      no_access;
         capture     group never;
     };
 
@@ -104,7 +104,7 @@ chip Membuf
     {
         name        "Bit map 64-127 of failed lanes read from VPD";
         scomaddr    0xFFFF0002;
-
+        access      no_access;
         capture     group never;
     };
 
@@ -112,7 +112,7 @@ chip Membuf
     {
         name        "Bit map 0-63 of failed lanes from io_read_erepair";
         scomaddr    0xFFFF0003;
-
+        access      no_access;
         capture     group never;
     };
 
@@ -120,7 +120,7 @@ chip Membuf
     {
         name        "Bit map 64-127 of failed lanes from io_read_erepair";
         scomaddr    0xFFFF0004;
-
+        access      no_access;
         capture     group never;
     };
 
diff --git a/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_NEST.rule b/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_NEST.rule
index 8b8ffc0a0..6bf4bd177 100755
--- a/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_NEST.rule
+++ b/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_NEST.rule
@@ -236,6 +236,7 @@
         name        "MBU.MBS.ECC01.MBECCFIR_AND";
         scomaddr    0x02011441;
         capture     group never;
+        access      write_only;
     };
 
     register MBA0_MBSECCFIR_MASK
@@ -285,6 +286,7 @@
         name        "MBU.MBS.ECC23.MBECCFIR_AND";
         scomaddr    0x02011481;
         capture     group never;
+        access      write_only;
     };
 
     register MBA1_MBSECCFIR_MASK
diff --git a/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_TP.rule b/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_TP.rule
index 8f58ac120..cc107a380 100755
--- a/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_TP.rule
+++ b/src/usr/diag/prdf/common/plat/pegasus/Membuf_regs_TP.rule
@@ -68,6 +68,7 @@
         name        "TPTOP.TPC.LOCAL_FIR_AND";
         scomaddr    0x0104000b;
         capture     group never;
+        access      write_only;
     };
 
     register TP_LFIR_MASK
@@ -83,6 +84,7 @@
         name        "TPTOP.TPC.EPS.FIR.LOCAL_FIR_MASK_OR";
         scomaddr    0x0104000f;
         capture     group never;
+        access      write_only;
     };
 
     register TP_LFIR_ACT0
diff --git a/src/usr/diag/prdf/common/plat/pegasus/Proc.rule b/src/usr/diag/prdf/common/plat/pegasus/Proc.rule
index 37e55a9b6..4628ceb40 100755
--- a/src/usr/diag/prdf/common/plat/pegasus/Proc.rule
+++ b/src/usr/diag/prdf/common/plat/pegasus/Proc.rule
@@ -107,7 +107,7 @@ chip Proc
     {
         name        "Bit map 0-63 of failed lanes read from VPD";
         scomaddr    0xFFFF0001;
-
+        access      no_access;
         capture     group never;
     };
 
@@ -115,7 +115,7 @@ chip Proc
     {
         name        "Bit map 64-127 of failed lanes read from VPD";
         scomaddr    0xFFFF0002;
-
+        access      no_access;
         capture     group never;
     };
 
@@ -123,7 +123,7 @@ chip Proc
     {
         name        "Bit map 0-63 of failed lanes from io_read_erepair";
         scomaddr    0xFFFF0003;
-
+        access      no_access;
         capture     group never;
     };
 
@@ -131,7 +131,7 @@ chip Proc
     {
         name        "Bit map 64-127 of failed lanes from io_read_erepair";
         scomaddr    0xFFFF0004;
-
+        access      no_access;
         capture     group never;
     };