Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand

[Philippe Mathieu-Daudé]

On 2/21/19 1:46 PM, Laszlo Ersek wrote:
> On 02/21/19 13:38, Peter Maydell wrote:
>> On Thu, 21 Feb 2019 at 12:07, Laszlo Ersek <address@hidden> wrote:
>>> since we're talking "reset_flash", I'll note that there is no actual
>>> reset handler for cfi.pflash01. I found out recently, via:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1678713
>>
>> Yes; this isn't uncommon for some of the really old
>> device models. It should definitely have one added.
>>
>> You are correct also that the timer in the pflash_cfi01
>> model is dead code -- it has always been so, since the
>> device was added in 2007. The reason it is there is that
>> pflash_cfi01 was created as a copy-and-hack of the
>> cfi02 device. In cfi02 we do use the timer, as a way
>> of simulating "make full-chip and sector erases take a
>> guest-visible amount of time rather than completing
>> instantaneously". cfi01 doesn't do that (and I think
>> may not implement anything other than block erase),

This time is flash-device specific and is currently hardcoded.

The guest learn from the CFI table how long it should wait
before polling/accessing the flash, or take measures in case
of timeout.

We set these values in _realize():

    ...
    /* Typical timeout for block erase (512 ms) */
    pfl->cfi_table[0x21] = 0x09;
    /* Typical timeout for full chip erase (4096 ms) */
    pfl->cfi_table[0x22] = 0x0C;
    ...
    /* Max timeout for block erase */
    pfl->cfi_table[0x25] = 0x0A;
    /* Max timeout for chip erase */
    pfl->cfi_table[0x26] = 0x0D;

The timer is triggered in _write(), where we use other hardcoded
values (which are luckily in range with the CFI announced ones):

        /* Let's wait 5 seconds before chip erase is done */
        timer_mod(pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
                  (NANOSECONDS_PER_SECOND * 5));

        /* Let's wait 1/2 second before sector erase is done */
        timer_mod(pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
                  (NANOSECONDS_PER_SECOND / 2));

When emulating embedded devices, it is very desirable to have those
timers working, and I'd prefer we fix that on CFI01 rather than simply
removing the unused timer.

Now for the case of "Virt" machines, it is probably pointless to add
flash delay and we should remove the timer use.

>> but the timer initialization code was left in rather
>> than being deleted as part of the copy-and-hack.
> 
> Thank you, I've linked this back into the RHBZ.
> 
> Laszlo
>