[v5,1/3] memory: drop guest writes to read-only ram device regions

for ram device regions, drop guest writes if the regions is read-only.

Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
Signed-off-by: Xin Zeng <xin.zeng@intel.com>
---
 memory.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

On Thu, Apr 30, 2020 at 03:07:21PM +0800, Philippe Mathieu-Daudé wrote:
> On 4/30/20 7:19 AM, Yan Zhao wrote:
> > for ram device regions, drop guest writes if the regions is read-only.
> > 
> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> > ---
> >   memory.c | 15 ++++++++++++---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/memory.c b/memory.c
> > index 601b749906..90a748912f 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -34,6 +34,7 @@
> >   #include "sysemu/accel.h"
> >   #include "hw/boards.h"
> >   #include "migration/vmstate.h"
> > +#include "qemu/log.h"
> >   
> >   //#define DEBUG_UNASSIGNED
> >   
> > @@ -1307,12 +1308,19 @@ static uint64_t memory_region_ram_device_read(void *opaque,
> >       return data;
> >   }
> >   
> > -static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> > -                                           uint64_t data, unsigned size)
> > +static MemTxResult memory_region_ram_device_write(void *opaque, hwaddr addr,
> > +                                           uint64_t data, unsigned size,
> > +                                           MemTxAttrs attrs)
> 
> Style alignment is now of and can be adjusted easily.
> 
> >   {
> >       MemoryRegion *mr = opaque;
> >   
> >       trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> > +    if (mr->readonly) {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                      "Invalid write to read only ram device region addr 0x%"
> > +                       HWADDR_PRIx" size %u\n", addr, size);
> 
> Style alignment of here too.
> 
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks! I'll update it right now!

> 
> > +        return MEMTX_ERROR;
> > +    }
> >   
> >       switch (size) {
> >       case 1:
> > @@ -1328,11 +1336,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> >           *(uint64_t *)(mr->ram_block->host + addr) = data;
> >           break;
> >       }
> > +    return MEMTX_OK;
> >   }
> >   
> >   static const MemoryRegionOps ram_device_mem_ops = {
> >       .read = memory_region_ram_device_read,
> > -    .write = memory_region_ram_device_write,
> > +    .write_with_attrs = memory_region_ram_device_write,
> >       .endianness = DEVICE_HOST_ENDIAN,
> >       .valid = {
> >           .min_access_size = 1,
> > 
>

On 4/30/20 7:19 AM, Yan Zhao wrote:
> for ram device regions, drop guest writes if the regions is read-only.
> 
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> ---
>   memory.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 601b749906..90a748912f 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -34,6 +34,7 @@
>   #include "sysemu/accel.h"
>   #include "hw/boards.h"
>   #include "migration/vmstate.h"
> +#include "qemu/log.h"
>   
>   //#define DEBUG_UNASSIGNED
>   
> @@ -1307,12 +1308,19 @@ static uint64_t memory_region_ram_device_read(void *opaque,
>       return data;
>   }
>   
> -static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> -                                           uint64_t data, unsigned size)
> +static MemTxResult memory_region_ram_device_write(void *opaque, hwaddr addr,
> +                                           uint64_t data, unsigned size,
> +                                           MemTxAttrs attrs)

Style alignment is now of and can be adjusted easily.

>   {
>       MemoryRegion *mr = opaque;
>   
>       trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> +    if (mr->readonly) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "Invalid write to read only ram device region addr 0x%"
> +                       HWADDR_PRIx" size %u\n", addr, size);

Style alignment of here too.

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +        return MEMTX_ERROR;
> +    }
>   
>       switch (size) {
>       case 1:
> @@ -1328,11 +1336,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
>           *(uint64_t *)(mr->ram_block->host + addr) = data;
>           break;
>       }
> +    return MEMTX_OK;
>   }
>   
>   static const MemoryRegionOps ram_device_mem_ops = {
>       .read = memory_region_ram_device_read,
> -    .write = memory_region_ram_device_write,
> +    .write_with_attrs = memory_region_ram_device_write,
>       .endianness = DEVICE_HOST_ENDIAN,
>       .valid = {
>           .min_access_size = 1,
>