Re: [PATCH] kvm: arm64: vgic: fix hyp panic with 64k pages on juno platform

[Will Deacon <will.deacon@arm.com>]

On Thu, Jul 24, 2014 at 08:47:23PM +0100, Peter Maydell wrote:
> On 24 July 2014 20:27, Will Deacon <will.deacon@arm.com> wrote:
> > If the physical address of GICV isn't page-aligned, then we end up
> > creating a stage-2 mapping of the page containing it, which causes us to
> > map neighbouring memory locations directly into the guest.
> >
> > As an example, consider a platform with GICV at physical 0x2c02f000
> > running a 64k-page host kernel. If qemu maps this into the guest at
> > 0x80010000, then guest physical addresses 0x80010000 - 0x8001efff will
> > map host physical region 0x2c020000 - 0x2c02efff. Accesses to these
> > physical regions may cause UNPREDICTABLE behaviour, for example, on the
> > Juno platform this will cause an SError exception to EL3, which brings
> > down the entire physical CPU resulting in RCU stalls / HYP panics / host
> > crashing / wasted weeks of debugging.
> 
> This seems to me like a specific problem with Juno rather than an
> issue with having the GICV at a non-page-aligned start. The
> requirement to be able to expose host GICV as the guest GICC
> in a 64K pages system is just "nothing else in that 64K page
> (or pages, if the GICV runs across two pages) is allowed to be
> unsafe for the guest to touch", which remains true whether the
> GICV starts at 0K in the 64K page or 60K.

I agree, and for that we would need a new ioctl so we can query the
page-offset of the GICV on systems where it is safe. Given that such an
ioctl doesn't exist today, I would like to plug the hole in mainline kernels
with this patch, we can relax in the future if systems appear where it would
be safe to map the entire 64k region.

> > SBSA recommends that systems alias the 4k GICV across the bounding 64k
> > region, in which case GICV physical could be described as 0x2c020000 in
> > the above scenario.
> 
> The SBSA "make every 4K region in the 64K page be the same thing"
> recommendation is one way of satisfying the requirement that the
> whole 64K page is safe for the guest to touch. (Making the rest of
> the page RAZ/WI would be another option I guess.) If your system
> actually implements the SBSA recommendation then in fact
> describing the GICV-phys-base as the 64K-aligned address is wrong,
> because then the register at GICV-base + 4K would not be
> the first register in the 2nd page of the GICV, it would be another
> copy of the 1st page. This happens to work on Linux guests
> currently because they don't touch anything in the 2nd page,
> but for cases like device passthrough IIRC we might well like
> the guest to use some of the 2nd page registers. So the only
> correct choice on those systems is to specify the +60K address
> as the GICV physaddr in the device tree, and use Marc's patchset
> to allow QEMU/kvmtool to determine the page offset within the 64K
> page so it can reflect that in the guest's device tree.

Again, that can be solved by introduced Marc's attr for determining the
GICV offset within the 64k page. I don't think that's -stable material.

> I can't think of any way of determining whether a particular
> system gets this right or wrong automatically, which suggests
> perhaps we need to allow the device tree to specify that the
> GICV is 64k-page-safe...

When we support such systems, I also think we'll need a device-tree change.
My main concern right now is stopping the ability to hose the entire machine
by trying to instantiate a virtual GIC.

Will