[v3,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.

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
Signed-off-by: Xin Zeng <xin.zeng@intel.com>
---
 memory.c     | 6 +++++-
 trace-events | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

On 4/13/20 8:37 AM, Yan Zhao wrote:
> for ram device regions, drop guest writes if the regions is read-only.
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> ---
>   memory.c     | 6 +++++-
>   trace-events | 2 +-
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 601b749906..a380b59980 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1312,7 +1312,11 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
>   {
>       MemoryRegion *mr = opaque;
>   
> -    trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> +    trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data,
> +                                         size, mr->readonly);
> +    if (mr->readonly) {

            qemu_log_mask(LOG_GUEST_ERROR, ...)?

> +        return;
> +    }
>   
>       switch (size) {
>       case 1:
> diff --git a/trace-events b/trace-events
> index 42107ebc69..e1de662973 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -61,7 +61,7 @@ memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value,
>   memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
>   memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
>   memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
> -memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
> +memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size, bool readonly) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" " is_readonly_region=%d"
>   flatview_new(void *view, void *root) "%p (root %p)"
>   flatview_destroy(void *view, void *root) "%p (root %p)"
>   flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
>

On Tue, Apr 14, 2020 at 05:35:22PM +0800, Philippe Mathieu-Daudé wrote:
> On 4/13/20 8:37 AM, Yan Zhao wrote:
> > for ram device regions, drop guest writes if the regions is read-only.
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> > ---
> >   memory.c     | 6 +++++-
> >   trace-events | 2 +-
> >   2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/memory.c b/memory.c
> > index 601b749906..a380b59980 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1312,7 +1312,11 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> >   {
> >       MemoryRegion *mr = opaque;
> >   
> > -    trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> > +    trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data,
> > +                                         size, mr->readonly);
> > +    if (mr->readonly) {
> 
>             qemu_log_mask(LOG_GUEST_ERROR, ...)?
>
ok. will add it in next version.

Thanks
Yan

> > +        return;
> > +    }
> >   
> >       switch (size) {
> >       case 1:
> > diff --git a/trace-events b/trace-events
> > index 42107ebc69..e1de662973 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -61,7 +61,7 @@ memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value,
> >   memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
> >   memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
> >   memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
> > -memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
> > +memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size, bool readonly) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" " is_readonly_region=%d"
> >   flatview_new(void *view, void *root) "%p (root %p)"
> >   flatview_destroy(void *view, void *root) "%p (root %p)"
> >   flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
> > 
>