Message ID | 20190705104255.24027-1-philmd@redhat.com |
---|---|
State | New |
Headers | show |
Series | [PATCH-for-4.1,v2] hw/ssi/xilinx_spips: Avoid AXI writes to the LQSPI linear memory | expand |
+-- On Fri, 5 Jul 2019, Philippe Mathieu-Daudé wrote --+
| +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;
| +}
| +
| 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
| }
Looks okay.
To confirm,
When lqspi_accepts() returns false, guest will see an error/exception?
Reviewed-by: Prasad J Pandit <pjp@fedoraproject.org>
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
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 <slei.casper@gmail.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > 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 >
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; +} + 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 }
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 <slei.casper@gmail.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- v1: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01238.html --- hw/ssi/xilinx_spips.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)