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

[Peter Maydell]

On Mon, 29 Jun 2020 at 12:18, Li Qiang <liq3ea@gmail.com> wrote:
>
> 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.

I think that 'romd mode' (ie reads-go-directly-to-RAM) is
the default: memory_region_initfn() sets romd_mode to true.
So unless the device actively calls memory_region_rom_device_set_romd(mr, false)
then the read callback can't be reached.

thanks
-- PMM