qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand
Date: Thu, 21 Feb 2019 17:19:48 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1

On 2/21/19 10:38 AM, Peter Maydell wrote:
> On Thu, 21 Feb 2019 at 09:22, Markus Armbruster <address@hidden> 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]