Re: [PATCH v2] arm64: fix VTTBR_BADDR_MASK

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jul 12, 2014 at 2:14 AM, Joel Schopp <joel.schopp@xxxxxxx> wrote:
> This patch sets TCR_EL2.PS, VTCR_EL2.T0SZ and vttbr_baddr_mask in runtime,
> not compile time.
>
> In ARMv8, EL2 physical address size (TCR_EL2.PS) and stage2 input address
> size (VTCR_EL2.T0SZE) cannot be determined in compile time since they
> depends on hardware capability.
>
> According to Table D4-23 and Table D4-25 in ARM DDI 0487A.b document,
> vttbr_x is calculated using different hard-coded values with consideration
> of T0SZ, granule size and the level of translation tables. Therefore,
> vttbr_baddr_mask should be determined dynamically.
>
> Changes since v1:
> Rebased fix on Jungseok Lee's patch https://lkml.org/lkml/2014/5/12/189 to
> provide better long term fix.  Updated that patch to log error instead of
> silently fail on unaligned vttbr.

Thanks for picking it up and revising it!

> Cc: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> Cc: Sungjinn Chung <sungjinn.chung@xxxxxxxxxxx>

He does not follow up this patch with @samsung.com
Please put barami97@xxxxxxxxx from the next version.

> Signed-off-by: Jungseok Lee <jays.lee@xxxxxxxxxxx>

I do not work this patch any more with @samsung.com either.
Please put my gmail from the next version.

> Signed-off-by: Joel Schopp <joel.schopp@xxxxxxx>
> ---
>  arch/arm/kvm/arm.c               |   91 +++++++++++++++++++++++++++++++++++++-
>  arch/arm64/include/asm/kvm_arm.h |   17 +------
>  arch/arm64/kvm/hyp-init.S        |   20 ++++++--
>  3 files changed, 106 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 3c82b37..6d51a53 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -37,6 +37,7 @@
>  #include <asm/mman.h>
>  #include <asm/tlbflush.h>
>  #include <asm/cacheflush.h>
> +#include <asm/cputype.h>
>  #include <asm/virt.h>
>  #include <asm/kvm_arm.h>
>  #include <asm/kvm_asm.h>
> @@ -61,6 +62,9 @@ static atomic64_t kvm_vmid_gen = ATOMIC64_INIT(1);
>  static u8 kvm_next_vmid;
>  static DEFINE_SPINLOCK(kvm_vmid_lock);
>
> +/* VTTBR mask cannot be determined in complie time under ARMv8 */
> +static u64 vttbr_baddr_mask;
> +
>  static bool vgic_present;
>
>  static void kvm_arm_set_running_vcpu(struct kvm_vcpu *vcpu)
> @@ -413,6 +417,75 @@ static bool need_new_vmid_gen(struct kvm *kvm)
>  }
>
>  /**
> + * set_vttbr_baddr_mask - set mask value for vttbr base address
> + *
> + * In ARMv8, vttbr_baddr_mask cannot be determined in compile time since stage2
> + * input address size depends on hardware capability. Thus, it is needed to read
> + * ID_AA64MMFR0_EL1.PARange first and then set vttbr_baddr_mask with
> + * consideration of both granule size and the level of translation tables.
> + */
> +static int set_vttbr_baddr_mask(void)
> +{
> +#ifndef CONFIG_ARM64
> +       vttbr_baddr_mask = VTTBR_BADDR_MASK;
> +#else

This is one of main topics in the previous discussion, [1].
Please refer to [1].

> +       int pa_range, t0sz, vttbr_x;
> +
> +       pa_range = read_cpuid(ID_AA64MMFR0_EL1) & 0xf;
> +
> +       switch (pa_range) {
> +       case 0:
> +               t0sz = VTCR_EL2_T0SZ(32);
> +               break;
> +       case 1:
> +               t0sz = VTCR_EL2_T0SZ(36);
> +               break;
> +       case 2:
> +               t0sz = VTCR_EL2_T0SZ(40);
> +               break;
> +       case 3:
> +               t0sz = VTCR_EL2_T0SZ(42);
> +               break;
> +       case 4:
> +               t0sz = VTCR_EL2_T0SZ(44);
> +               break;
> +       default:
> +               t0sz = VTCR_EL2_T0SZ(48);
> +       }

According to [1], it is a better idea to use default to handle error case,
not supported value, "5".

> +
> +       /*
> +        * See Table D4-23 and Table D4-25 in ARM DDI 0487A.b to figure out
> +        * the origin of the hardcoded values, 38 and 37.
> +        */
> +#ifdef CONFIG_ARM64_64K_PAGES
> +       /*
> +        * 16 <= T0SZ <= 21 is valid under 3 level of translation tables
> +        * 18 <= T0SZ <= 34 is valid under 2 level of translation tables
> +        * 31 <= T0SZ <= 39 is valid under 1 level of transltaion tables
> +        */
> +       if (t0sz <= 17) {
> +               kvm_err("Cannot support %d-bit address space\n", 64 - t0sz);
> +               return -EINVAL;
> +       }
> +       vttbr_x = 38 - t0sz;
> +#else
> +       /*
> +        * 16 <= T0SZ <= 24 is valid under 4 level of translation tables
> +        * 21 <= T0SZ <= 30 is valid under 3 level of translation tables
> +        * 30 <= T0SZ <= 39 is valid under 2 level of translation tables
> +        */
> +       if (t0sz <= 20) {
> +               kvm_err("Cannot support %d-bit address space\n", 64 - t0sz);
> +               return -EINVAL;
> +       }
> +       vttbr_x = 37 - t0sz;
> +#endif
> +       vttbr_baddr_mask = (((1LLU << (48 - vttbr_x)) - 1) << (vttbr_x - 1));
> +#endif
> +       return 0;
> +}
> +
> +/**
>   * update_vttbr - Update the VTTBR with a valid VMID before the guest runs
>   * @kvm        The guest that we are about to run
>   *
> @@ -466,8 +539,16 @@ static void update_vttbr(struct kvm *kvm)
>         /* update vttbr to be used with the new vmid */
>         pgd_phys = virt_to_phys(kvm->arch.pgd);
>         vmid = ((u64)(kvm->arch.vmid) << VTTBR_VMID_SHIFT) & VTTBR_VMID_MASK;
> -       kvm->arch.vttbr = pgd_phys & VTTBR_BADDR_MASK;
> -       kvm->arch.vttbr |= vmid;
> +
> +       /*
> +        * If the VTTBR isn't aligned there is something wrong with the system
> +        * or kernel.  It is better to just fail and not mask it. But no need
> +        * to panic the host kernel with a BUG_ON(), instead just log the error.
> +        */
> +       if (pgd_phys & ~vttbr_baddr_mask)
> +               kvm_err("VTTBR not aligned, expect guest to fail");
> +
> +       kvm->arch.vttbr = pgd_phys | vmid;

It is a good idea to check alignment.

Reference:
----------
[1]: http://www.spinics.net/lists/kvm-arm/msg09712.html

- Jungseok Lee--
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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux