[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] arch: arm: vgic-v3: fix GICD_ISACTIVER range





On Tue, 12 Nov 2019, 04:01 Stefano Stabellini, <sstabellini@xxxxxxxxxx> wrote:
On Sat, 9 Nov 2019, Julien Grall wrote:
> On Sat, 9 Nov 2019, 04:27 Stefano Stabellini, <sstabellini@xxxxxxxxxx> wrote:
>       On Thu, 7 Nov 2019, Peng Fan wrote:
>       > The end should be GICD_ISACTIVERN not GICD_ISACTIVER.
>       >
>       > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
>
>       Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>
>
> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.