> On Jul 4, 2019, at 08:45, Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
>
> Cc'ing PPC/taihu_405ep and ARM/Digic4 maintainers.
>
> On 7/3/19 6:36 PM, Philippe Mathieu-Daudé wrote:
>> On 7/3/19 6:20 PM, Stephen Checkoway wrote:
>>>> On Jul 3, 2019, at 12:02, Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
>>>> On 7/3/19 5:52 PM, Stephen Checkoway wrote:
>>>>>
>>>>>
>>>>>> On Jul 1, 2019, at 20:59, Philippe Mathieu-Daudé <phi...@redhat.com>
>>>>>> wrote:
>>>>>>
>>>>>> Parallel NOR flashes are limited to 16-bit bus accesses.
>>>>>
>>>>> I don't think this is correct. The CFI spec defines an x32 interface for
>>>>> parallel NOR. CFI addresses 0x28 and 0x29 specify the interface and value
>>>>> 3 is x32 and value 5 is x16/x32.
>>>>>
>>>>> Here's an example of an x32 device
>>>>> <https://www.mouser.com/datasheet/2/100/002-00948_29CD032J_S29CD016J_S29CL032J_S29CL016J_3-1316792.pdf>.
>>>>
>>>> OK, I was not aware of these.
>>>>
>>>> QEMU never CFI-announced itself as x32 capable:
>>>>
>>>> /* Flash device interface (8 & 16 bits) */
>>>> pfl->cfi_table[0x28] = 0x02;
>>>> pfl->cfi_table[0x29] = 0x00;
>>>>
>>>> So while the commit description is incorrect, the code is safe with the
>>>> current device model.
>>>>
>>>> I am not comfortable keeping untested 32-bit mode.
>>>> Were you using it?
>>>
>>> I'm not using it. I did have some code to set these CFI values based on the
>>> parameters used to control the interleaving
>>> <https://github.com/stevecheckoway/qemu/commit/f9a79a6e18b2c7c5a05e344ff554a7d980a56042#diff-d33881bd0ef099e2f46ebd4797c653bcR599>.
>>>
>>> In general, a better testing harness would be nice though.
>>
>> We can revert it if it helps you.
>
> So after further analysis, there are not guest visible changes, because
> 32-bit accesses are still valid (.valid.max_access_size = 4) but now
> they are processed as two 16-bit accesses, shifted by
> access_with_adjusted_size().
> Well, I haven't checked (yet) when the guest and the flash are in
> different endianess, and I wish we don't use that.
>
> Now I see 2 different guests "registering" the flash in 32-bit access:
>
> - PPC/taihu_405ep
>
> The CFI id matches the S29AL008J that is a 1MB in x16, while the code
> QEMU forces it to be 2MB, and checking Linux it expects a 4MB flash
> there, Yay \o/
>
> - ARM/Digic4
>
> While the comment says "Samsung K8P3215UQB 64M Bit (4Mx16)", this
> flash is 32Mb (2MB). Also note the CFI id does not match the comment.
> Again, Yay.
>
> Anyway, better safe than sorry, so I'll revert.
>
> Thanks for following and catching this,
Great, thanks!
--
Stephen Checkoway
