Re: [Xen-devel] [PATCH 04/10] xen/arm: vgic-v3: Don't check the size when we ignore the write/read as zero

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2015-01-21 at 12:45 +0000, Julien Grall wrote:
> On 21/01/15 12:36, Ian Campbell wrote:
> > On Wed, 2015-01-21 at 12:28 +0000, Julien Grall wrote:
> > 
> >>> 3) Accesses which are valid but which do not correctly emulate according
> >>> to the features of the virtual gic which we are exposing can log if we
> >>> think it is useful to do so.
> >>
> >> I gave a look to the code. We have few registers we don't correctly
> >> emulate. The current behavior seems to be inconsistent, we either inject
> >> a data abort (such as ICPENDR) or ignore the error (such as ICACTIVER).
> >>
> >> Shall we take a domain_crash approach (as bad_width) or inject a data 
> >> abort?
> > 
> > I think for these cases since we do update and/or return
> > rank->iactive/ipend a debug log and continue is appropriate.
> >
> > Assuming we do need to do something more than track the status of
> > i{active,pend}, which I expect we do -- e.g. to cancel or inject as
> > appropriate.
> 
> This code is actually buggy as we never use those fields. I have a patch
> to drop iactive/ipend fields as we will unlikely handle ACTIVE/PENDING
> status via those bit. We already track in different way as we already
> have tracking per vIRQ.
> 
> My plan was to replace them with an error log and maybe a data abort.
> If you think a debug message is enough, I could go this way. Though, it
> may be more difficult for a developer to find the actual error if there
> is may logs.

The best thing to do would be to implement them properly. I'm not sure
that switching to an abort of any sort would be a good idea now that
we've pretended to implement them a bit, logging then RAZ/WI might be an
ok change.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel