Re: [PATCH v2 5/9] nvram: add nrf51_soc flash read method

[Li Qiang]

P J P <ppandit@redhat.com> 于2020年6月25日周四 上午3:01写道：
>
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> Add nrf51_soc mmio read method to avoid NULL pointer dereference
> issue.
>
> Reported-by: Lei Sun <slei.casper@gmail.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/nvram/nrf51_nvm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> Update v2: return ldl_le_p()
>   -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg04972.html
>
> diff --git a/hw/nvram/nrf51_nvm.c b/hw/nvram/nrf51_nvm.c
> index f2283c1a8d..8000ed530a 100644
> --- a/hw/nvram/nrf51_nvm.c
> +++ b/hw/nvram/nrf51_nvm.c
> @@ -273,6 +273,13 @@ static const MemoryRegionOps io_ops = {
>          .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>
> +static uint64_t flash_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    NRF51NVMState *s = NRF51_NVM(opaque);
> +
> +    assert(offset + size <= s->flash_size);
> +    return ldl_le_p(s->storage + offset);
> +}

The 'flash_ops' is for ROM, though I don't see where it calls
'memory_region_rom_device_set_romd'
to ROMD, so this MR is in MMIO mode and it needs a read callback.

However as the origin code doesn't provide a read callback. So why
here we return something?

I prefer here just 'qemu_log_mask(LOG_UNIMP, "%s not implemented\n",
__func__);' as other
patches does.

Thanks,
Li Qiang


>
>  static void flash_write(void *opaque, hwaddr offset, uint64_t value,
>          unsigned int size)
> @@ -300,6 +307,7 @@ static void flash_write(void *opaque, hwaddr offset, 
> uint64_t value,
>
>
>  static const MemoryRegionOps flash_ops = {
> +    .read = flash_read,
>      .write = flash_write,
>      .valid.min_access_size = 4,
>      .valid.max_access_size = 4,
> --
> 2.26.2
>