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 implementing the write_with_attrs() handler.
Return MEMTX_ERROR  when the region is accessed  (this error maps
to an AXI slave error).

[*] https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf

Reported-by: Lei Sun <slei.casper@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ssi/xilinx_spips.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index e80619aece..4c0b0aa3c9 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -1221,8 +1221,24 @@ static MemTxResult lqspi_read(void *opaque, hwaddr addr, uint64_t *value,
     return MEMTX_OK;
 }
 
+static MemTxResult lqspi_write(void *opaque, hwaddr offset, uint64_t value,
+                               unsigned size, MemTxAttrs attrs)
+{
+    /*
+     * From UG1085, Chapter 24 (Quad-SPI controllers):
+     * - Writes are ignored
+     * - AXI writes generate an external AXI slave error (SLVERR)
+     */
+    qemu_log_mask(LOG_GUEST_ERROR, "%s Unexpected %u-bit access to 0x%" PRIx64
+                                   " (value: 0x%" PRIx64 "\n",
+                  __func__, size << 3, offset, value);
+
+    return MEMTX_ERROR;
+}
+
 static const MemoryRegionOps lqspi_ops = {
     .read_with_attrs = lqspi_read,
+    .write_with_attrs = lqspi_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 1,
-- 
2.20.1

Hi Philippe,

On [2019 Jul 05] Fri 17:08:50, Philippe Mathieu-Daudé wrote:
> Lei Sun found while auditing the code than a CPU write would

s/than/that/

> trigger a NULL pointer deference.

s/deference/dereference/

> 
> From UG1085 datasheet [*] AXI writes in this region are ignored
> and generates an External Slave Error (SLVERR).

s/External/AXI/

> 
> Fix by implementing the write_with_attrs() handler.
> Return MEMTX_ERROR  when the region is accessed  (this error maps

There is an extra whitespace after 'MEMTX_ERROR' and also after
'accessed'.

Sorry for not mentioning above before, after correcting this cosmetica:

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

Best regards,
Francisco Iglesias

> to an AXI slave error).
> 
> [*] https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf
> 
> Reported-by: Lei Sun <slei.casper@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/ssi/xilinx_spips.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index e80619aece..4c0b0aa3c9 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -1221,8 +1221,24 @@ static MemTxResult lqspi_read(void *opaque, hwaddr addr, uint64_t *value,
>      return MEMTX_OK;
>  }
>  
> +static MemTxResult lqspi_write(void *opaque, hwaddr offset, uint64_t value,
> +                               unsigned size, MemTxAttrs attrs)
> +{
> +    /*
> +     * From UG1085, Chapter 24 (Quad-SPI controllers):
> +     * - Writes are ignored
> +     * - AXI writes generate an external AXI slave error (SLVERR)
> +     */
> +    qemu_log_mask(LOG_GUEST_ERROR, "%s Unexpected %u-bit access to 0x%" PRIx64
> +                                   " (value: 0x%" PRIx64 "\n",
> +                  __func__, size << 3, offset, value);
> +
> +    return MEMTX_ERROR;
> +}
> +
>  static const MemoryRegionOps lqspi_ops = {
>      .read_with_attrs = lqspi_read,
> +    .write_with_attrs = lqspi_write,
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 1,
> -- 
> 2.20.1
>