Activity log for bug #1637974

Date Who What changed Old value New value Message
2016-10-31 10:48:36 jeyyej bug added bug
2016-10-31 10:52:46 jeyyej description pl080 is arm dma controller virtual device. source code path: hw/dma/pl080.c I find there are two same dead code in pl080_read and pl080_write, Here's the code, comments are my opinion: ========================= 240 static uint64_t pl080_read(void *opaque, hwaddr offset, 241 unsigned size) 242 { 243 PL080State *s = (PL080State *)opaque; 244 uint32_t i; 245 uint32_t mask; 246 247 if (offset >= 0xfe0 && offset < 0x1000) { 248 if (s->nchannels == 8) { 249 return pl080_id[(offset - 0xfe0) >> 2]; 250 } else { 251 return pl081_id[(offset - 0xfe0) >> 2]; 252 } 253 } 254 if (offset >= 0x100 && offset < 0x200) { //// here offset is limited in 0x100~0x200 255 i = (offset & 0xe0) >> 5; 256 if (i >= s->nchannels) 257 goto bad_offset; 258 switch (offset >> 2) { //// then here, offset>>2 is in range 64~128 259 case 0: /* SrcAddr */ //// while the switch case is 0,1,2,3,4, 260 return s->chan[i].src; //// so, NONE of the switch case would be selected ! 261 case 1: /* DestAddr */ //// this switch is A DEAD CODE, it is contradictory with if. 262 return s->chan[i].dest; 263 case 2: /* LLI */ 264 return s->chan[i].lli; 265 case 3: /* Control */ 266 return s->chan[i].ctrl; 267 case 4: /* Configuration */ 268 return s->chan[i].conf; 269 default: 270 goto bad_offset; 271 } 272 } ..................................... ============================================= I guess, switch statement should like this : switch((offset-0x100)>>2) { ... } pl080 is arm dma controller virtual device. source code path: hw/dma/pl080.c I find there are two same dead code in pl080_read and pl080_write, Here's the code, comments are my opinion: ========================= 240 static uint64_t pl080_read(void *opaque, hwaddr offset, 241 unsigned size) 242 { 243 PL080State *s = (PL080State *)opaque; 244 uint32_t i; 245 uint32_t mask; 246 247 if (offset >= 0xfe0 && offset < 0x1000) { 248 if (s->nchannels == 8) { 249 return pl080_id[(offset - 0xfe0) >> 2]; 250 } else { 251 return pl081_id[(offset - 0xfe0) >> 2]; 252 } 253 } 254 if (offset >= 0x100 && offset < 0x200) { //// here offset is limited in 0x100~0x200 255 i = (offset & 0xe0) >> 5; 256 if (i >= s->nchannels) 257 goto bad_offset; 258 switch (offset >> 2) { //// then here, offset>>2 is in range 64~128 259 case 0: /* SrcAddr */ //// while the switch case is 0,1,2,3,4, 260 return s->chan[i].src; //// so, NONE of the switch case would be selected ! 261 case 1: /* DestAddr */ //// this switch is A DEAD CODE, it is contradictory with if. 262 return s->chan[i].dest; 263 case 2: /* LLI */ 264 return s->chan[i].lli; 265 case 3: /* Control */ 266 return s->chan[i].ctrl; 267 case 4: /* Configuration */ 268 return s->chan[i].conf; 269 default: 270 goto bad_offset; 271 } 272 }         ..................................... ============================================= I guess, switch statement should like this : switch((offset-0x100)>>2) { ... } if this DEADCODE is not the author's original purpose, Then there must be something in logic goes wrong, pl080 have NEVER works correctly ?
2017-11-06 15:22:41 Peter Maydell qemu: status New Confirmed
2018-10-04 12:34:23 Thomas Huth qemu: status Confirmed Fix Committed
2018-12-12 08:36:44 Thomas Huth qemu: status Fix Committed Fix Released