From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH] kvm: arm64: vgic: fix hyp panic with 64k pages on juno platform Date: Fri, 25 Jul 2014 10:31:27 +0100 Message-ID: <20140725093127.GB5269@arm.com> References: <1406230067-926-1-git-send-email-will.deacon@arm.com> <20140724195528.GC9143@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Paolo Bonzini , "gleb@kernel.org" , "kvmarm@lists.cs.columbia.edu" , kvm-devel , Christoffer Dall , Marc Zyngier , Joel Schopp , Don Dutile , "stable@vger.kernel.org" To: Peter Maydell Return-path: Content-Disposition: inline In-Reply-To: Sender: stable-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Hi Peter, On Thu, Jul 24, 2014 at 09:05:39PM +0100, Peter Maydell wrote: > On 24 July 2014 20:55, Will Deacon 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