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

[Joel Schopp <joel.schopp@amd.com>]

On 07/25/2014 09:23 AM, Will Deacon wrote:
> On Fri, Jul 25, 2014 at 03:16:15PM +0100, Joel Schopp wrote:
>> On 07/25/2014 09:08 AM, Will Deacon wrote:
>>>> This would break with my SOC device tree which looks like this.  Note
>>>> this device tree works just fine without checks.
>>>>
>>>>     gic: interrupt-controller@e1101000 {
>>>>         compatible = "arm,gic-400-v2m";
>>>>         #interrupt-cells = <3>;
>>>>         #address-cells = <0>;
>>>>         interrupt-controller;
>>>>         msi-controller;
>>>>         reg = <0x0 0xe1110000 0 0x1000>, /* gic dist */
>>>>               <0x0 0xe112f000 0 0x2000>, /* gic cpu */
>>>>               <0x0 0xe114f000 0 0x2000>, /* gic virtual ic*/
>>>>               <0x0 0xe116f000 0 0x2000>, /* gic virtual cpu*/
>>>>               <0x0 0xe1180000 0 0x1000>; /* gic msi */
>>>>         interrupts = <1 8 0xf04>;
>>>>     };
>>> I appreciate it may work, but that's only because the kernel is actually
>>> using an alias of GICV at 0xe1160000 by accident. I would say that you're
>>> getting away with passing an incorrect description.
>> The problem is that by the spec the size is 0x2000 and was never
>> properly rearchitected from 4K to variable page size support. The
>> workaround is that each of the 4K in the 64K page (16 of them) are
>> really mapped to the same location in hardware. So the contents of
>> 0xe1160000 is the same as the contents of 0xe116f000. See “Appendix F:
>> GIC-400 and 64KB Translation Granule” in v2.1 of the ARM _Server Base
>> System Architecture_ specification.
> I've read that document, but it's not mandated and I don't think I have a
> single piece of hardware that actually follows it. Even the CPUs don't seem
> to perform the aliasing suggesting there (take a look at the A57 and A53
> TRMs -- there are reserved regions in there).
Not mandated, but it is obviously highly encouraged, and at a minimum
valid.  The SOC sitting under my desk follows it.

>
>> Now if we were to change the entry to <0x0 0xe1160000 0 0x2000> it would
>> be obviously broken because the second half would map to a duplicate of
>> the first half.
> I think you'd need something like <0x0 0xe1160000 0 0x20000> and we'd have
> to change the GIC driver to do the right thing. What we currently have is
> unsafe on most hardware, yet happens to work on your system.
If you change the GIC driver,kvm, kvmtool, and qemu to do the right
thing I'd be happy to change the device tree entry to <0x0 0xe1160000 0
0x20000> or any other value of your choosing.  I really have no opinion
here on how it should be done other than what is there now currently
works and I'd like to have whatever we do in the future continue to work.