Re: [Qemu-arm] [PATCH-for-4.1 v5 3/3] hw/ssi/xilinx_spips: Avoid out-of-bound access to lqspi_buf[]

[Philippe Mathieu-Daudé]

Hi Francisco,

On 7/8/19 4:26 PM, Francisco Iglesias wrote:
> Hi Philippe,
> 
> On [2019 Jul 08] Mon 12:47:50, Philippe Mathieu-Daudé wrote:
>> Both lqspi_read() and lqspi_load_cache() expect a 32-bit
>> aligned address.
>>
>> From UG1085 datasheet [*] Chapter 22: Quad-SPI Controller:
> 
> s/22/24/

I'm confuse, Chapter 24 is "SD/SDIO Controller", so it seems this Xilinx
datasheet is not stable to refer to. Maybe I should simply use:

  From UG1085 datasheet [*] Chapter on 'Quad-SPI Controller':

> 
> After above change:
> 
> Reviewed-by: Francisco Iglesias <address@hidden> 
> Tested-by: Francisco Iglesias <address@hidden>
> 
> (I tested all three patches so the Tested-by tag can also be appended on the
> other two)

Thanks!

> 
> Best regards,
> Francisco Iglesias
> 
>>
>>   Transfer Size Limitations
>>
>>     Because of the 32-bit wide TX, RX, and generic FIFO, all
>>     APB/AXI transfers must be an integer multiple of 4-bytes.
>>     Shorter transfers are not possible.
>>
>> Set MemoryRegionOps.impl values to force 32-bit accesses,
>> this way we are sure we do not access the lqspi_buf[] array
>> out of bound.
>>
>> [*] 
>> https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf
>>
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>> v5: add datasheet reference, drop RFC prefix, fix build (Francisco)
>> ---
>>  hw/ssi/xilinx_spips.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
>> index 3c4e8365ee..b29e0a4a89 100644
>> --- a/hw/ssi/xilinx_spips.c
>> +++ b/hw/ssi/xilinx_spips.c
>> @@ -1239,6 +1239,10 @@ static const MemoryRegionOps lqspi_ops = {
>>      .read_with_attrs = lqspi_read,
>>      .write_with_attrs = lqspi_write,
>>      .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .impl = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4,
>> +    },
>>      .valid = {
>>          .min_access_size = 1,
>>          .max_access_size = 4
>> -- 
>> 2.20.1
>>