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 15:08:51 +0100 Message-ID: <20140725140851.GJ5269@arm.com> References: <1406230067-926-1-git-send-email-will.deacon@arm.com> <20140724195528.GC9143@arm.com> <20140725093127.GB5269@arm.com> <53D26392.9080602@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Peter Maydell , Paolo Bonzini , "gleb@kernel.org" , "kvmarm@lists.cs.columbia.edu" , kvm-devel , Christoffer Dall , Marc Zyngier , Don Dutile , "stable@vger.kernel.org" To: Joel Schopp Return-path: Received: from cam-admin0.cambridge.arm.com ([217.140.96.50]:40779 "EHLO cam-admin0.cambridge.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760130AbaGYOJG (ORCPT ); Fri, 25 Jul 2014 10:09:06 -0400 Content-Disposition: inline In-Reply-To: <53D26392.9080602@amd.com> Sender: kvm-owner@vger.kernel.org List-ID: Hi Joel, On Fri, Jul 25, 2014 at 03:02:58PM +0100, Joel Schopp wrote: > >>>> 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, > 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. Will