Re: [Qemu-devel] [PATCH-for-4.1 v2] hw/ssi/xilinx_spips: Avoid AXI writes to the LQSPI linear memory

[Francisco Iglesias]

Hi Philippe,

On [2019 Jul 05] Fri 12:42:55, Philippe Mathieu-Daudé wrote:
> Lei Sun found while auditing the code than a CPU write would
> trigger a NULL pointer deference.
> 
> From UG1085 datasheet [*] AXI writes in this region are ignored
> and generates an External Slave Error (SLVERR).
> 
> Fix by checking the access is a READ before calling the region
> callback.
> 
> [*] 
> https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf
> 
> Reported-by: Lei Sun <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01238.html
> ---
>  hw/ssi/xilinx_spips.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 8115bb6d46..0c9ccd12bf 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -1202,6 +1202,18 @@ static void lqspi_load_cache(void *opaque, hwaddr addr)
>      }
>  }
>  
> +static bool lqspi_accepts(void *opaque, hwaddr addr,
> +                          unsigned size, bool is_write,
> +                          MemTxAttrs attrs)
> +{
> +    /*
> +     * From UG1085, Chapter 24 (Quad-SPI controllers):
> +     * - Writes are ignored
> +     * - AXI writes generate an external AXI slave error (SLVERR)
> +     */
> +    return !is_write;

What I can see above generates an AXI Decode error (MEMTX_DECODE_ERROR),
MEMTX_ERROR is the one that maps to AXI slave error. What about swapping to the
solution you proposed yesterday? (but changing return value of the write
function to be MEMTX_ERROR)

Best regards,
Francisco Iglesias

> +}
> +
>  static uint64_t
>  lqspi_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> @@ -1225,6 +1237,7 @@ static const MemoryRegionOps lqspi_ops = {
>      .read = lqspi_read,
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
> +        .accepts = lqspi_accepts,
>          .min_access_size = 1,
>          .max_access_size = 4
>      }
> -- 
> 2.20.1
>