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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
