Re: [Qemu-devel] [PATCH for 2.4 2/3] net/dp8393x: specify memory operations for PROM PROM

[Aurelien Jarno]

On 2015-07-24 20:42, Hervé Poussineau wrote:
> This fixes a guest-triggerable QEMU crash when guest tries to write to PROM.
> 
> Signed-off-by: Hervé Poussineau <address@hidden>
> ---
>  hw/net/dp8393x.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 8fafdb0..55168b5 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -601,6 +601,16 @@ static const MemoryRegionOps dp8393x_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> +static bool dp8393x_rom_accepts(void *opaque, hwaddr addr, unsigned int size,
> +                                bool is_write)
> +{
> +    return !is_write;
> +}
> +
> +static const MemoryRegionOps dp8393x_rom_ops = {
> +    .valid.accepts = dp8393x_rom_accepts,
> +};
> +
>  static void dp8393x_watchdog(void *opaque)
>  {
>      dp8393xState *s = opaque;
> @@ -840,7 +850,7 @@ static void dp8393x_realize(DeviceState *dev, Error 
> **errp)
>      s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
>      s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
>  
> -    memory_region_init_rom_device(&s->prom, OBJECT(dev), NULL, NULL,
> +    memory_region_init_rom_device(&s->prom, OBJECT(dev), &dp8393x_rom_ops, 
> NULL,
>                                    "dp8393x-prom", SONIC_PROM_SIZE, NULL);
>      prom = memory_region_get_ram_ptr(&s->prom);
>      checksum = 0;

How does it crashes in that case? I would have guess that write access
to ROM are ignored by default. Looking at other code, it seems they call
memory_region_set_readonly() instead of providing an accepts function.
Maybe readonly should be the default for a rom device?

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net