Re: [Qemu-devel] [Qemu devel v9 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block

[Philippe Mathieu-Daudé]

On 09/18/2017 11:00 AM, Peter Maydell wrote:
> On 18 September 2017 at 14:58, Philippe Mathieu-Daudé <address@hidden> wrote:
> > Hi Peter,
> >
> > On 09/18/2017 10:36 AM, Peter Maydell wrote:
> > > On 18 September 2017 at 14:27, sundeep subbaraya <address@hidden>
> > > wrote:
> > > > > > Hi Sundeep, Peter,
> > > > > >
> > > > > >
> > > > > > On 09/15/2017 01:59 PM, Subbaraya Sundeep wrote:
> > > > > > > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
> > > > > > > +                          uint64_t val, unsigned size)
> > > > > > > +{
> > > > > > > +    MSF2SysregState *s = opaque;
> > > > > > > +    uint32_t newval = val;
> > > > > > > +
> > > > > > > +    offset >>= 2;
> > > > > > > +
> > > > > > > +    switch (offset) {
> > > > > > > +    case MSSDDR_PLL_STATUS:
> > > > > > > +        trace_msf2_sysreg_write_pll_status();
> > > > > > > +        break;
> > > > > > > +
> > > > > > > +    case ESRAM_CR:
> > > > > > > +    case DDR_CR:
> > > > > > > +    case ENVM_REMAP_BASE_CR:
> > > > > > > +        if (newval != s->regs[offset]) {
> > > > > > > +            qemu_log_mask(LOG_UNIMP,
> > > > > > > +                       TYPE_MSF2_SYSREG": remapping not
> > > > > > > supported\n");
> > > > > >
> > > > > >
> > > > > >
> > > > > > I'd rather exit here than continue with inconsistent cpu, Peter what do
> > > > > > you recommend?
> > > >
> > > >
> > > >
> > > > Could you please suggest.
> > >
> > >
> > > We shouldn't exit QEMU for something the guest can provoke. If
> > > the functionality isn't implemented then just LOG_UNIMP it.
> >
> >
> > This was commented here:
> > http://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg02971.html
> >
> > Guest remapping code is like non-return function. Hardware error here can
> > not happen, so there is no more guest code after a remap request.
> > Continuing running QEMU will lead to chaos, that's why I recommend aborting
> > instead of just LOG_UNIMP it.
>
> There's lots of stuff that can cause the guest to go badly
> and confusingly wrong if unimplemented. If we print a
> useful message with LOG_UNIMP then there's an easy clue
> for what's gone wrong. The guest shouldn't be able to
> provoke QEMU to exit.

I understand. I'm worried about being PITA for an user to figure out 
what's happening here, that's why I'm wondering how this should be 
handled. Another idea I thought about was raising a PREFETCH or BKPT 
exception, but this would also be an incorrect model behavior.

Anyway I'd be happy enough if instead of using LOG_UNIMP/LOG_GUEST_ERROR 
we use an unconditional qemu_log("bus remap unimplemented, unpredictable 
behaviour!") warning. Fair enough?

Regards,

Phil.