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

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





On 28/11/2019 09:00, Jürgen Groß wrote:
On 28.11.19 09:48, Julien Grall wrote:
Hi,

On 28/11/2019 08:32, Jürgen Groß wrote:
On 28.11.19 09:14, Julien Grall wrote:
In short, I think the patch should go in now and there are no downsides
to it. That's it, I rest my case. Julien, I hope you'll reconsider.
I don't really see the point to try to allow Linux 5.4 booting on Xen 4.13 without knowing whether we are not going to uncovered more BUG around I*ACTIVER.

Sorry, but this is a rather weird statement.

IIUC you are saying that a typo blocking boot of Linux 5.4 should not be
fixed as you are not sure there are no other bugs?

The implementation of I*ACTIVER was incorrect but gone unnoticed because no-one used it until 5.4. It also happen that we didn't cover all the I*ACTIVER registers, so 5.4 crashes instead of using the wrong behavior.

This patch is basically replacing a guest crash by a behavior we don't fully understand.


If you really want this patch in Xen 4.13, then you should read my mail on the first version and trying to answer me why this 5.4 is going to be safe running on Xen 4.13.

Or do you think that with the typo fixed and running Linux 5.4 guests
will destabilize the host?

It is not going to destabilize the hosts. But this is not going to make 5.4 running correctly as Xen guest.

Have you verified it isn't running correctly or do you just think it
could hit problems?

I haven't tested myself, but any bug around vGIC is usually subtled. I wrote a long e-mail on v1 (see [1]) explaning what could happen.

To summarize briefly, Linux is reading the I*ACTIVER registers to check whether an interrupt is active at the hardware level. For instance, this is used to ensure all active interrupts have been handled before continuing.

By always returning 0, we tell Linux there are no interrupts. One of the risk is interrupts may be lost.

But that's Linux behavior, I can't tell how this is going to be used by others OSes.


In both cases I see no reason to keep wrong code.

Either the patch will let run Linux 5.4 fine - then the patch should
definitely be taken.
That's up to Stefano and Peng to provide me information why this is fine. FAOD, the current justification provided is not acceptable for me.


Or the patch will let Linux 5.4 boot further, but some problems will
occur. Then it will be possible to analyze those problems and try to
fix them, very possibly with the sane approach you are hoping for.


Juergen

[1] https://lore.kernel.org/xen-devel/7289f75f-1ab2-2d42-cd88-1be5306b3072@xxxxxxx/

--
Julien Grall

_______________________________________________
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®.