[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 3/8] m68k: Add NeXTcube machine
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PULL 3/8] m68k: Add NeXTcube machine |
Date: |
Wed, 18 Sep 2019 11:27:04 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 16/09/2019 12.48, Peter Maydell wrote:
> On Sat, 7 Sep 2019 at 16:47, Thomas Huth <address@hidden> wrote:
>>
>> It is still quite incomplete (no SCSI, no floppy emulation, no network,
>> etc.), but the firmware already shows up the debug monitor prompt in the
>> framebuffer display, so at least the very basics are already working.
>>
>> This code has been taken from Bryce Lanham's GSoC 2011 NeXT branch at
>>
>> https://github.com/blanham/qemu-NeXT/blob/next-cube/hw/next-cube.c
>>
>> and altered quite a bit to fit the latest interface and coding conventions
>> of the current QEMU.
>>
>> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>> Message-Id: <address@hidden>
>> Signed-off-by: Thomas Huth <address@hidden
>
> Hi; Coverity spotted an issue in this function
> (CID 1405664):
>
>
>> +static void nextscr2_write(NeXTState *s, uint32_t val, int size)
>> +{
>> + static int led;
>> + static int phase;
>> + static uint8_t old_scr2;
>> + static uint8_t rtc_command;
>> + static uint8_t rtc_value;
>> + static uint8_t rtc_status = 0x90;
>> + static uint8_t rtc_return;
>> + uint8_t scr2_2;
>> +
>
>
>> + /* read the status 0x31 */
>> + if (rtc_command == 0x31) {
>> + scr2_2 = scr2_2 & (~SCR2_RTDATA);
>> + /* for now 0x00 */
>> + if (0x00 & (0x80 >> (phase - 8))) {
>
> 0 & anything can never be true, so the line below here is dead code.
Right. I'm going to have a closer look at this at the weekend, to see
what's the best way to fix it or whether it should simply be removed.
>> + scr2_2 |= SCR2_RTDATA;
>> + }
>> + rtc_return = (rtc_return << 1) |
>> + ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
>> + }
>> +
>
> Incidentally, I see that this file has quite a lot of
> what seems to be essentially device emulation code in it
> (a bunch of IO MemoryRegions defined locally) -- ideally
> these could be split out into proper device objects at
> some point.
Yeah, it's all old code from 2011 ... I'll keep this in mind for future
clean-ups!
Thanks,
Thomas
- [Qemu-devel] [PULL 6/8] m68k: Add an entry for the NeXTcube machine to the MAINTAINERS file, (continued)
- [Qemu-devel] [PULL 6/8] m68k: Add an entry for the NeXTcube machine to the MAINTAINERS file, Thomas Huth, 2019/09/07
- [Qemu-devel] [PULL 4/8] escc: introduce a selector for the register bit, Thomas Huth, 2019/09/07
- [Qemu-devel] [PULL 8/8] .travis.yml: Let the avocado job run the NeXTcube tests, Thomas Huth, 2019/09/07
- [Qemu-devel] [PULL 5/8] m68k: Add serial controller to the NeXTcube machine, Thomas Huth, 2019/09/07
- [Qemu-devel] [PULL 7/8] tests/acceptance: Add test of NeXTcube framebuffer using OCR, Thomas Huth, 2019/09/07
- [Qemu-devel] [PULL 3/8] m68k: Add NeXTcube machine, Thomas Huth, 2019/09/07
- Re: [Qemu-devel] [PULL 0/8] Add the m68k next-cube machine, Peter Maydell, 2019/09/09