On 2/21/19 10:38 AM, Peter Maydell wrote:
> On Thu, 21 Feb 2019 at 09:22, Markus Armbruster <arm...@redhat.com> wrote:
>> Double-checking... you want me to keep goto reset_flash, like this:
>>
>> @@ -623,8 +617,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>> pfl->wcycle = 0;
>> pfl->status |= 0x80;
>> } else {
>> - DPRINTF("%s: unknown command for \"write block\"\n",
>> __func__);
>> - PFLASH_BUG("Write block confirm");
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "unknown command for \"write block\"\n");
>> goto reset_flash;
>> }
>> break;
>
> Yes. (We seem to handle most kinds of guest errors in programming
> the flash by reset_flash.)
Oh I missed the context of the patch here.
So for the case of the Multi-WRITE command (0xe8):
1/ On first write cycle we have
- address = flash_page_address (we store it in pfl->counter)
- data = flash_command (0xe8: enter Multi-WRITE)
2/ Second cycle:
- address = flash_page_address
We should check it matches flash_page_address
of cycle 1/, but we don't.
- data: N
"N is the number of elements (bytes / words / double words),
minus one, to be written to the write buffer. Expected count
ranges are N = 00h to N = 7Fh (e.g., 1 to 128 bytes) in 8-bit
mode, N = 00h to N = 003Fh in 16-bit mode, and N = 00h to
N = 1Fh in 32-bit mode. Bus cycles 3 and higher are for writing
data into the write buffer. The confirm command (D0h) is
expected after exactly N + 1 write cycles; any other command at
that point in the sequence will prevent the transfer of the
buffer to the array (the write will be aborted)."
Instead of starting to write the data in a buffer, we write it
directly to the block backend.
Instead of starting to write from cycle 3+, we write now in 2,
and keep cycle count == 2 (pfl->wcycle) until all data is
written, where we increment at 3.
3/ Cycles 3+
- address = word index (relative to the page address)
- data = word value
We check for the CONFIRM command, and switch the device back
to READ mode (setting Status), or reset the device (which is
modelled the same way).
Very silly: If the guest cancelled and never sent the CONFIRM
command, the data is already written/flushed back.
So back to the previous code snippet, regardless the value, this
is neither a hw_error() nor a GUEST_ERROR. This code is simply not
correct. Eventually the proper (polite) error message should be:
qemu_log_mask(LOG_UNIMP, "MULTI_WRITE: Abort not implemented,"
" the data is already written"
" on storage!\n"
"Nevertheless resetting the flash"
" into READ mode.\n");
Regards,
Phil.
