Threadsichere Bitmanipulation mit Bitbanding

[Walter T. (nicolas)]

Hallo zusammen,

bislang habe ich Bitbanding, obwohl es in vielen 
ARM-Cortex-M-Beschreibungen weit vorn steht, noch nie benötigt. Aber 
jetzt habe ich wohl einen Anwendungsfall: Ich will zu einer Variable 
einzelne Bits hinzufügen oder löschen.

Threadsicher oder besser: Interrupt-Sicher muss es sein. Performance ist 
nicht ganz so wichtig - ansonsten hätte ich statt eines Bit-Arrays ein 
Array aus 32-Bit-Werten genutzt.

/** Warnungen */
enum warning_e
{
    warning_lorem = 1<<0,
    warning_ipsum = 1<<1,
    warning_dolor = 1<<2,
    warning_sit   = 1<<3,
    warning_end = warning_sit,
};

typedef struct WarnState_s
{
    enum warning_e last;
    volatile enum warning_e this;
}
WarnState_t;

WarnState_t WarnState = {};

/* Bitbanding Makros https://www.mikrocontroller.net/articles/ARM_Bitbanding */
#define BBSram1Bit(addr, bit)   (*(__typeof__(addr)*) ((SRAM1_BB_BASE + (((unsigned)&(addr) - SRAM1_BASE) << 5) + ((bit) << 2))))
#define BBitOfMask(mask)        (31 - __builtin_clz(mask))


/** Eine oder mehrere neue Warnungen hinzufuegen. */
void warning_add(enum warning_e wrn)
{
    WarnState_t *const State = &WarnState;
    #if defined( STM32F4XX )
        /* Auf STM32 Bit threadsicher ueber Bitbanding-Region sichern
           Bitbanding: Abbildung des Speicherbereichs 0x20000000-0x200FFFFF auf den
           Bereich 0x22000000-0x23FFFFFF
           clz() ist auf den Cortex M3/M4 billig. */
        assert( (uint32_t) State>=0x20000000 && (uint32_t) State<= 0x200FFFFF);

        while(wrn)
        {
            int32_t bitNo = BBitOfMask(wrn);     /* Erstes Bit der Maske */
            BBSram1Bit(State->this, bitNo) = 1;  /* Bit ueber Bitbanding-Region setzen */
            wrn &= ~(1UL<<bitNo);                /* Behandeltes Bit loeschen */
        }
    #else
        /* Auf anderen Plattformen hoffen */
        State->this = wrn;
    #endif
}


/** Eine oder mehrere Warnungen entfernen
 *
 * Es duerfen auch Warnungen entfernt werden, die nicht gesetzt sind. */
void warning_remove(enum warning_e wrn)
{
    WarnState_t *const State = &WarnState;
    #if defined( STM32F4XX )
        while(wrn)
        {
            int32_t bitNo = BBitOfMask(wrn);     /* Erstes Bit der Maske */
            BBSram1Bit(State->this, bitNo) = 0;  /* Bit ueber Bitbanding-Region loeschen*/
            wrn &= ~(1UL<<bitNo);                /* Behandeltes Bit loeschen */
        }
    #else
        /* Auf anderen Plattformen hoffen */
        State->this &= ~wrn;
    #endif
}

Habe ich das so richtig umgesetzt, oder ist da noch ein Denkfehler?
(Es funktioniert zumindest schon einmal. Ein gutes Zeichen.)

[PittyJ (Gast)]

Einfach mal so eine Anmerkung. Falls der Code jemals irgendwann auch auf 
C++ kompiliert wird, sollte man auf einen Bezeichner 'this' verzichten.

Auch wenn C++ Erfahrene sich den Source-Code anschauen, könnten sie 
dadurch verwirrt werden.


Ansonsten verstehe ich das eigentliche Problem gar nicht. Was ist das 
Problem mit der Bitmaske?

[Walter T. (nicolas)]

Eine Nacht drüber geschlafen, und eine Sache ist mir aufgefallen:

/** Eine oder mehrere neue Warnungen hinzufuegen oder loeschen
 *
 * @param[in] wrn: Bitmaske Warnungen
 * @param[in] to: true/false (setzen/loeschen)
 *
 * Auf STM32 werden die Bits threadsicher ueber die Bitbanding-Region geschrieben.
 * Bitbanding: Abbildung des Speicherbereichs 0x20000000-0x200FFFFF auf den
 * Bereich 0x22000000-0x23FFFFFF.  clz() ist auf den Cortex M3/M4 billig. */
static_inline void warning_setTo(enum warning_e wrn, bool to)
{
  #if defined( STM32F4XX )
    /* Bitbanding-Region SRAM1 sicherstellen */
    /* Die Assertion ist teuer und verhindert Optimierungen.
       Mit einem Schreiben ins struct anstelle der Adresse des structs kann auch ohne
       Assertion sichergestellt werden, dass keine Flash-Region versehentlich ueber
       Bitbandig anzusprechen versucht wird. */
    //assert( (uint32_t) &WarnState>=0x20000000 && (uint32_t) &WarnState<= 0x200FFFFF);

    while(wrn)
    {
      int32_t bitNo = BBitOfMask(wrn);        /* Erstes Bit der Maske */
      BBSram1Bit(WarnState.this, bitNo) = to; /* Bit ueber Bitbanding-Region setzen */
      wrn &= ~(1UL<<bitNo);                   /* Behandeltes Bit loeschen */
    }

  #else
    /* Auf anderen Plattformen hoffen */
    if( v )
      WarnState.this |= wrn;
    else
      WarnState.this &= ~wrn;
  #endif
}

Wenn ich statt in einen Zeiger auf die Variable auf die Variable 
schreibe, warnt mich der Compiler auch ohne Assertion davon, eine 
Variable außerhalb des Bitbanding-Bereichs zu beschreiben. Eigentlich 
sind ja nur Flash-Variablen nicht bitbandbar, aber sie sind sowieso 
konstant und das wird mit einem Compiler-Fehler quittiert.

PittyJ schrieb:
> Ansonsten verstehe ich das eigentliche Problem gar nicht. Was ist das
> Problem mit der Bitmaske?

Ich will threadsicher Flags setzen und löschen.

PittyJ schrieb:
> Einfach mal so eine Anmerkung. Falls der Code jemals irgendwann auch auf
> C++ kompiliert wird, sollte man auf einen Bezeichner 'this' verzichten.

Stimmt. Wenn ich auf C++ umstellen sollte, ist das noch eine der 
kleinsten Umstellungen. Was ist eigentlich in C++ das übliche Idiom für 
"delta = this - last" oder "delta = new - this" ?

[Programmierer (Gast)]

Du könntest auch atomic_fetch_or verwenden, das ist ebenfalls 
thread/interrupt-sicher aber dafür standardisiert und portabel:

https://en.cppreference.com/w/c/atomic/atomic_fetch_or

[Walter T. (nicolas)]

Programmierer schrieb:
> Du könntest auch atomic_fetch_or verwenden

Danke, das passt gut in den #else-Zweig.

[Purzel H. (hacky)]

Ein einzelnes Bit schreiben ist ja sowieso atomar...

[Programmierer (Gast)]

Purzel H. schrieb:
> Ein einzelnes Bit schreiben ist ja sowieso atomar...

Nein, nicht im Adressraum beim ARM. Ein Bit zu setzen bedeutet nämlich:
- Einen 8/16/32-Bit-Wert aus dem Adressraum/RAM in ein 
32bit-Prozessorregister laden
- Das Bit im Prozessorregister setzen
- Die 8/16/32 Bit des Registers zurück in den Adressraum/RAM schreiben

Und genau das macht der Compiler auch daraus. Wenn bei Schritt 2 ein 
Interrupt oder Thread-Kontext-Wechsel kommt und dann ein anderes Bit im 
8/16/32-Bit Wert geändert wird, wird das im Schritt drei wieder 
zurückgesetzt.

Daher braucht es atomische Operationen.

[EAF (Gast)]

Programmierer schrieb:
> Nein, nicht im Adressraum beim ARM.

Auch nicht beim AVR.
Zumindest nicht im gesamten Adressraum

Beispiel:

volatile unsigned z = 42;
z |= (1<<5);
cout << z << endl;

Das ist nicht atomar, darum wirft es auch wohl in moderneren C++:

warning: compound assignment with 'volatile'-qualified left operand is deprecated [-Wvolatile]

In dem z Fall ist es klar, aber in anderen Fällen sieht man nicht auf 
den ersten Blick, ob der Zugriff jetzt atomar erfolgt, oder eben nicht.
Ein Quell für Fehler.
Der wird jetzt ausgemerzt.