search

sergev / qemu

QEMU for PIC32
https://github.com/sergev/qemu/wiki
Other
29 stars 7 forks source link

Pass original register value to write_op from WRITEOPR macro #2

Closed chrisdearman closed 9 years ago

chrisdearman commented 9 years ago

The original value is needed for SET, CLR and INV operations to work correctly

chrisdearman commented 9 years ago

I noticed that writing a single bit to OSCCLR was actually clearing all of the writable bits

sergev commented 9 years ago

Wow... Seems like a critical issue. I wonder how it remained unnoticed for so long.

chrisdearman commented 9 years ago

I tried your sample binaries and none of them trigger this bug as far as I can see.

The mpide code does trigger the bug in an innocuous way by writing a single bit to OSCCONCLR which clears the whole register.

Here's the output from a simple program that writes to a few different SET/INV registers. This was an earlier version of the patch that displayed a message when the old calculated did not match the new one (it does set the register to the value...).

Board: chipKIT Max32 Processor: M4K RAM size: 128 kbytes Load file: '/Users/chris/Downloads/boot-max32.hex', 6720 bytes Load file: 'build/HelloWorld.cpp.hex', 16356 bytes --- I/O WRITEOPR 00000002 to OSCCONCLR offset:f004 OSCCON(romask:c0a08800 initial:01453320 -> oldfinal:00000000 newfinal:01453320)

The OSCONCLR write above is from the MPIDE support code The following ones are from my test:

Hello world! OSCCON: 1453320 After OSCCONSET = 0xffffffff => OSCCON: 3F5F77FF --- I/O WRITEOPR 00000001 to OSCCONINV offset:f00c OSCCON(romask:c0a08800 initial:3f5f77ff -> oldfinal:00000001 newfinal:3f5f77fe) After OSCCONINV = 0x00000001 => OSCCON: 3F5F77FE --- I/O WRITEOPR 00000001 to OSCCONINV offset:f00c OSCCON(romask:c0a08800 initial:3f5f77fe -> oldfinal:00000001 newfinal:3f5f77ff) After OSCCONINV = 0x00000001 => OSCCON: 3F5F77FF OSCTUN: 0 After OSCTUNSET = 0xffffffff => OSCTUN: 3F --- I/O WRITEOPR 00000001 to OSCTUNINV offset:f01c OSCTUN(romask:ffffffc0 initial:0000003f -> oldfinal:00000001 newfinal:0000003e) After OSCTUNINV = 0x00000001 => OSCTUN: 3E RCON: 0 After RCONSET = 0xffffffff => RCON: 3DF --- I/O WRITEOPR 00000001 to RCONINV offset:f60c RCON(romask:fffffc20 initial:000003df -> oldfinal:00000001 newfinal:000003de) After RCONINV = 0x00000001 => RCON: 3DE U1STA: 1610 --- I/O WRITEOPR 00000001 to U1STAINV offset:601c U1STA(romask:0000031d initial:00001610 -> oldfinal:00000210 newfinal:00001610) After U1STAINV = 0x00000001 => U1STA: 1610

sergev commented 9 years ago

Thank you for the fix. Seems like CLR registers are not widely used in *BSD code, so I just never came across this bug.