qemu-arm
[Top][All Lists]
Advanced

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

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


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-arm] [PATCH-for-4.1 v5 3/3] hw/ssi/xilinx_spips: Avoid out-of-bound access to lqspi_buf[]
Date: Tue, 9 Jul 2019 12:58:05 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0

Hi Peter,

On 7/8/19 6:03 PM, Francisco Iglesias wrote:
> Hi Philippe,
> 
> On [2019 Jul 08] Mon 16:58:29, Philippe Mathieu-Daudé wrote:
>> 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':
> 
> I just clicked on the link and ended up into this version of the UG1085:
> 
> 'UG1085 (v1.9) January 17, 2019'
> 
> But your right its probably better to refer to a specific version of the
> UG1085 instead? Then both should be ok, either to write the way you
> suggest above or refer to the chapter number in that UG1085 version (as it
> was before).
> 
> Best regards,
> Francisco
> 
>>
>>>
>>> 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)

Are you OK to take this series via your ARM tree?

If so, do you want me to respin fixing the comment and adding Francisco
tags?

Thanks,

Phil.

>>
>> 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
>>>>



reply via email to

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