On Tue, 12 Nov 2019, 04:01 Stefano Stabellini, <sstabell...@kernel.org>
wrote:

> On Sat, 9 Nov 2019, Julien Grall wrote:
> > On Sat, 9 Nov 2019, 04:27 Stefano Stabellini, <sstabell...@kernel.org>
> wrote:
> >       On Thu, 7 Nov 2019, Peng Fan wrote:
> >       > The end should be GICD_ISACTIVERN not GICD_ISACTIVER.
> >       >
> >       > Signed-off-by: Peng Fan <peng....@nxp.com>
> >
> >       Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
> >
> >
> > To be honest, I am not sure the code is correct. A read to those
> registers should tell you the list of interrupts active. As we always
> > return 0, this will not return the correct state of the GIC.
> >
> > I know that returning the list of actives interrupts is complicated with
> the old vGIC, but I don't think silently ignoring it is a good
> > idea.
> > The question here is why the guest accessed those registers? What is it
> trying to figure out?
>
> We are not going to solve the general problem at this stage. At the
> moment the code:
>
> - ignore the first register only
> - print an error and return an IO_ABORT error for the other regs
>
> For the inconsistency alone the second option is undesirable. Also it
> doesn't match the write implementation, which does the same thing for
> all the GICD_ISACTIVER* regs instead of having a special treatment for
> the first one only. It looks like a typo in the original patch to me.
>
> The proposed patch switches the behavior to:
>
> - silently ignore all the GICD_ISACTIVER* regs (as proposed)


> is an improvement.
>

Peng mentioned that Linux is accessing it, so the worst thing we can do is
lying to the guest (as you suggest here). I would definitely not call that
an improvement.

In the current state, it is a Nack. If there were a warning, then I would
be more inclined to see this patch going through.

Cheers,
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to