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

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

Hi Peter,

On Thu, Jul 24, 2014 at 09:05:39PM +0100, Peter Maydell wrote:
> On 24 July 2014 20:55, Will Deacon <will.deacon@arm.com> wrote:
> > 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.
> 
> Agreed that we don't want to put Marc's patchset in -stable
> (and that without it systems with GICV in their host devicetree
> at pagebase+60K are unusable, so we're not actually regressing
> anything if we put this into stable). But...
> 
> >> 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.
> 
> ...I don't see how your patch prevents instantiating a VGIC
> and hosing the machine on a system where the 64K
> with the GICV registers in it goes
>  [GICV registers] [machine blows up if you read this]
>  0K                      8K                                             64K

True, if such a machine existed, then this patch wouldn't detect it. I don't
think we support anything like that in mainline at the moment, but the
following additional diff should solve the problem, no?

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index fa9a95b3ed19..476d3bf540a8 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1539,6 +1539,14 @@ int kvm_vgic_hyp_init(void)
                goto out_unmap;
        }
 
+       if (!PAGE_ALIGNED(resource_size(&vcpu_res))) {
+               kvm_err("GICV size 0x%llx not a multiple of page size 0x%lx\n",
+                       (unsigned long long)resource_size(&vcpu_res),
+                       PAGE_SIZE);
+               ret = -ENXIO;
+               goto out_unmap;
+       }
+
        vgic_vcpu_base = vcpu_res.start;
 
        kvm_info("%s@%llx IRQ%d\n", vgic_node->name,

> Whether the 64K page contains Bad Stuff is completely
> orthogonal to whether the device tree offset the host has
> for the GICV is 0K, 60K or anything in between. What you
> should be checking for is "is this system design broken?",
> which is probably a device tree attribute.

Actually, I think we can just claim that the GICV occupies the full 64k
region and allow the offset within that region to be acquired via the new
ioctl.

Will