On 25 July 2014 10:31, Will Deacon <will.deacon@xxxxxxx> wrote:
> On Thu, Jul 24, 2014 at 09:05:39PM +0100, Peter Maydell wrote:
>> On 24 July 2014 20:55, Will Deacon <will.deacon@xxxxxxx> 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?
Well, the apm-storm.dtsi specifies a 64K aligned GICV
base but a size of 8K, so presumably it qualifies.
(If the GICV size should really be 64K and that's an APM
device tree bug then this patch is going to make KVM
on 64K page hosts stop working on that board... somebody
with access to the h/w docs for the APM boards needs
to check that, I guess.)
> 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,
Yes, I think if we check both start and size are page aligned
then this will both:
(a) avoid using the VGIC on systems where it's going to
allow the guest to take down the machine
(b) avoid using the VGIC if it's not at a page boundary,
pending Marc's patchset landing and making that case
actually work rather than do weird things.
thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
