[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() i
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand |
Date: |
Thu, 21 Feb 2019 17:55:18 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 02/21/19 17:39, Philippe Mathieu-Daudé wrote:
> 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.
If the timer logic can be completed in such a way that existent OVMF
code sees no change at all, on the machine types that it currently runs
on (that is, on *unversioned* i440fx and q35), I'm OK with the idea.
Thanks,
Laszlo
>
>>> 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
>>
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, (continued)
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Philippe Mathieu-Daudé, 2019/02/19
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Peter Maydell, 2019/02/19
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Markus Armbruster, 2019/02/19
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Markus Armbruster, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Peter Maydell, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Laszlo Ersek, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Peter Maydell, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Laszlo Ersek, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Philippe Mathieu-Daudé, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand,
Laszlo Ersek <=
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Philippe Mathieu-Daudé, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Markus Armbruster, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Alex Bennée, 2019/02/21
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Markus Armbruster, 2019/02/22
- Re: [Qemu-block] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Alex Bennée, 2019/02/21
[Qemu-block] [PATCH 01/10] pflash: Rename pflash_t to PFlashCFI01, PFlashCFI02, Markus Armbruster, 2019/02/18