[PATCH] KVM: arm64: Check GICV address is placed at a 4Kb boundary

[PATCH] KVM: arm64: Check GICV address is placed at a 4Kb boundary

[Itaru Kitayama]

Make sure GICV physical address is placed at a 4Kb boundary as spec 
says, not host kernel's page boundary. This prevents !4Kb kernels from 
returning an ENXIO prematurely during the KVM init phase at boot.
Tested on Overdrive 3000 with 64Kb page-size kernel with ACPI.

Signed-off-by: Itaru Kitayama <itaru.kitayama@riken.jp>
Cc: Marc Zyngier <marc.zyngier@arm.com>


diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index e31405e..0123e5d 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -309,17 +309,15 @@ int vgic_v2_probe(const struct gic_kvm_info *info)
                 return -ENXIO;
         }

-       if (!PAGE_ALIGNED(info->vcpu.start)) {
-               kvm_err("GICV physical address 0x%llx not page aligned\n",
+       if (!IS_ALIGNED(info->vcpu.start, SZ_4K)) {
+               kvm_err("GICV physical address 0x%llx not 4Kb aligned\n",
                         (unsigned long long)info->vcpu.start);
                 return -ENXIO;
         }

-       if (!PAGE_ALIGNED(resource_size(&info->vcpu))) {
-               kvm_err("GICV size 0x%llx not a multiple of page size 
0x%lx\n",
-                       (unsigned long long)resource_size(&info->vcpu),
+       if (!PAGE_ALIGNED(resource_size(&info->vcpu)))
+               kvm_warn("GICV size 0x%llx not a multiple of page size 
0x%lx, system may be unsafe\n", (unsigned long 
long)resource_size(&info->vcpu),
                         PAGE_SIZE);
-               return -ENXIO;
         }

         kvm_vgic_global_state.vctrl_base = ioremap(info->vctrl.start,

Re: [PATCH] KVM: arm64: Check GICV address is placed at a 4Kb boundary

[Christoffer Dall]

Hi Itaru,

On Wed, Jun 29, 2016 at 03:22:45PM +0900, Itaru Kitayama wrote:
> Make sure GICV physical address is placed at a 4Kb boundary as spec
> says, not host kernel's page boundary. This prevents !4Kb kernels
> from returning an ENXIO prematurely during the KVM init phase at
> boot.
> Tested on Overdrive 3000 with 64Kb page-size kernel with ACPI.
> 
> Signed-off-by: Itaru Kitayama <itaru.kitayama@riken.jp>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> 
> 
> diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
> index e31405e..0123e5d 100644
> --- a/virt/kvm/arm/vgic/vgic-v2.c
> +++ b/virt/kvm/arm/vgic/vgic-v2.c
> @@ -309,17 +309,15 @@ int vgic_v2_probe(const struct gic_kvm_info *info)
>                 return -ENXIO;
>         }
> 
> -       if (!PAGE_ALIGNED(info->vcpu.start)) {
> -               kvm_err("GICV physical address 0x%llx not page aligned\n",
> +       if (!IS_ALIGNED(info->vcpu.start, SZ_4K)) {
> +               kvm_err("GICV physical address 0x%llx not 4Kb aligned\n",
>                         (unsigned long long)info->vcpu.start);
>                 return -ENXIO;
>         }
> 
> -       if (!PAGE_ALIGNED(resource_size(&info->vcpu))) {
> -               kvm_err("GICV size 0x%llx not a multiple of page
> size 0x%lx\n",
> -                       (unsigned long long)resource_size(&info->vcpu),
> +       if (!PAGE_ALIGNED(resource_size(&info->vcpu)))
> +               kvm_warn("GICV size 0x%llx not a multiple of page
> size 0x%lx, system may be unsafe\n", (unsigned long
> long)resource_size(&info->vcpu),
>                         PAGE_SIZE);
> -               return -ENXIO;
>         }
> 
>         kvm_vgic_global_state.vctrl_base = ioremap(info->vctrl.start,

Doesn't this change carry the risk of KVM mapping a page (larger than
4K) containing other things than the GICV interface into the guest,
potentially breaking isolation of the VM from host hardware resources?

-Christoffer

Re: [PATCH] KVM: arm64: Check GICV address is placed at a 4Kb boundary

[Itaru Kitayama]

Christoffer,

On 6/29/16 9:43 PM, Christoffer Dall wrote:
> Doesn't this change carry the risk of KVM mapping a page (larger than
> 4K) containing other things than the GICV interface into the guest,
> potentially breaking isolation of the VM from host hardware resources?

I still suspect the 64Kb long region is filled with 16 copies of first 
4Kb of GICV, as Joel said in a thread two years ago when Marc introduced 
sub-page alignment patch:

https://patches.linaro.org/patch/32191/

. So with that patch applied, a VM always boots on a 64Kb page granular 
host, thanks to the multiple copies of the first part of GICV, but it'll 
have a problem accessing the GICV_DIR register that is
placed at the second 4Kb of region mapped for GICV.

For the ACPI enabled systems, the check of physical address of the 
virtual cpu interface is page aligned only works for 4Kb page granular 
kernels, so the patch is still required, though I need to come up with 
another patch that does map GICV to guest's physical address space 
taking care of the offset in the presence of the new VGIC code.