| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170222130616.GO26976@cbox>
Date: Wed, 22 Feb 2017 14:06:16 +0100
From: Christoffer Dall <cdall@...aro.org>
To: Jintack Lim <jintack@...columbia.edu>
Cc: christoffer.dall@...aro.org, marc.zyngier@....com,
pbonzini@...hat.com, rkrcmar@...hat.com, linux@...linux.org.uk,
catalin.marinas@....com, will.deacon@....com,
vladimir.murzin@....com, suzuki.poulose@....com,
mark.rutland@....com, james.morse@....com,
lorenzo.pieralisi@....com, kevin.brodsky@....com,
wcohen@...hat.com, shankerd@...eaurora.org, geoff@...radead.org,
andre.przywara@....com, eric.auger@...hat.com,
anna-maria@...utronix.de, shihwei@...columbia.edu,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC 27/55] KVM: arm/arm64: Emulate GICH interface on GICv2
On Mon, Jan 09, 2017 at 01:24:23AM -0500, Jintack Lim wrote:
> Emulate GICH interface accesses from the guest hypervisor.
>
> Signed-off-by: Jintack Lim <jintack@...columbia.edu>
> Signed-off-by: Shih-Wei Li <shihwei@...columbia.edu>
> Signed-off-by: Christoffer Dall <christoffer.dall@...aro.org>
> ---
> arch/arm64/kvm/Makefile | 1 +
> virt/kvm/arm/vgic/vgic-v2-nested.c | 207 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 208 insertions(+)
> create mode 100644 virt/kvm/arm/vgic/vgic-v2-nested.c
>
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 9c35e9a..8573faf 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -37,3 +37,4 @@ kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
>
> kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += handle_exit_nested.o
> kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += emulate-nested.o
> +kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += $(KVM)/arm/vgic/vgic-v2-nested.o
> diff --git a/virt/kvm/arm/vgic/vgic-v2-nested.c b/virt/kvm/arm/vgic/vgic-v2-nested.c
> new file mode 100644
> index 0000000..b13128e
> --- /dev/null
> +++ b/virt/kvm/arm/vgic/vgic-v2-nested.c
> @@ -0,0 +1,207 @@
> +#include <linux/cpu.h>
> +#include <linux/kvm.h>
> +#include <linux/kvm_host.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +
> +#include <linux/irqchip/arm-gic.h>
> +
> +#include <asm/kvm_emulate.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_mmu.h>
> +#include <kvm/arm_vgic.h>
> +
> +#include "vgic.h"
> +#include "vgic-mmio.h"
> +
> +static inline struct vgic_v2_cpu_if *vcpu_nested_if(struct kvm_vcpu *vcpu)
> +{
> + return &vcpu->arch.vgic_cpu.nested_vgic_v2;
> +}
> +
> +static inline struct vgic_v2_cpu_if *vcpu_shadow_if(struct kvm_vcpu *vcpu)
> +{
> + return &vcpu->arch.vgic_cpu.shadow_vgic_v2;
> +}
> +
> +static unsigned long vgic_mmio_read_v2_vtr(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len)
> +{
> + u32 reg;
> +
> + reg = kvm_vgic_global_state.nr_lr - 1;
> + reg |= 0b100 << 26;
> + reg |= 0b100 << 29;
Pure magic? Can we have some defines? Have you checked the existing
header file if we have some defines for this?
> +
> + return reg;
> +}
> +
> +static inline bool lr_triggers_eoi(u32 lr)
> +{
> + return !(lr & (GICH_LR_STATE | GICH_LR_HW)) && (lr & GICH_LR_EOI);
> +}
> +
> +static unsigned long get_eisr(struct kvm_vcpu *vcpu, bool upper_reg)
> +{
> + struct vgic_v2_cpu_if *cpu_if = vcpu_nested_if(vcpu);
> + int max_lr = upper_reg ? 64 : 32;
> + int min_lr = upper_reg ? 32 : 0;
> + int nr_lr = min(kvm_vgic_global_state.nr_lr, max_lr);
> + int i;
> + u32 reg = 0;
So the assumption here is that we can only emualte a virtual GICH
interface with the same number of LRs that the hardware has, yes ? Can
you document this assumption in the commit message and explain how we
deal with nr_lr for all this logic based on that.
Seems like this could go in the commit message.
> +
> + for (i = min_lr; i < nr_lr; i++) {
> + if (lr_triggers_eoi(cpu_if->vgic_lr[i]))
> + reg |= BIT(i - min_lr);
> + }
> +
> + return reg;
> +}
> +
> +static unsigned long vgic_mmio_read_v2_eisr0(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len)
> +{
> + return get_eisr(vcpu, false);
> +}
> +
> +static unsigned long vgic_mmio_read_v2_eisr1(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len)
> +{
> + return get_eisr(vcpu, true);
> +}
> +
> +static u32 get_elrsr(struct kvm_vcpu *vcpu, bool upper_reg)
> +{
> + struct vgic_v2_cpu_if *cpu_if = vcpu_nested_if(vcpu);
> + int max_lr = upper_reg ? 64 : 32;
> + int min_lr = upper_reg ? 32 : 0;
> + int nr_lr = min(kvm_vgic_global_state.nr_lr, max_lr);
> + u32 reg = 0;
> + int i;
> +
> + for (i = min_lr; i < nr_lr; i++) {
> + if (!(cpu_if->vgic_lr[i] & GICH_LR_STATE))
> + reg |= BIT(i - min_lr);
> + }
> +
> + return reg;
> +}
> +
> +static unsigned long vgic_mmio_read_v2_elrsr0(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len)
> +{
> + return get_elrsr(vcpu, false);
> +}
> +
> +static unsigned long vgic_mmio_read_v2_elrsr1(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len)
> +{
> + return get_elrsr(vcpu, true);
> +}
> +
> +static unsigned long vgic_mmio_read_v2_misr(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len)
> +{
> + struct vgic_v2_cpu_if *cpu_if = vcpu_nested_if(vcpu);
> + int nr_lr = kvm_vgic_global_state.nr_lr;
> + u32 reg = 0;
> +
> + if (vgic_mmio_read_v2_eisr0(vcpu, addr, len) ||
> + vgic_mmio_read_v2_eisr1(vcpu, addr, len))
> + reg |= GICH_MISR_EOI;
> +
> + if (cpu_if->vgic_hcr & GICH_HCR_UIE) {
> + u32 elrsr0 = vgic_mmio_read_v2_elrsr0(vcpu, addr, len);
> + u32 elrsr1 = vgic_mmio_read_v2_elrsr1(vcpu, addr, len);
> + int used_lrs;
> +
> + used_lrs = nr_lr - (hweight32(elrsr0) + hweight32(elrsr1));
> + if (used_lrs <= 1)
> + reg |= GICH_MISR_U;
> + }
> +
> + /* TODO: Support remaining bits in this register */
Is this going to happen in this series? Why don't we just do it here?
> + return reg;
> +}
> +
> +static unsigned long vgic_mmio_read_v2_gich(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len)
> +{
> + struct vgic_v2_cpu_if *cpu_if = vcpu_nested_if(vcpu);
> + u32 value;
> +
> + switch (addr & 0xfff) {
> + case GICH_HCR:
> + value = cpu_if->vgic_hcr;
> + break;
> + case GICH_VMCR:
> + value = cpu_if->vgic_vmcr;
> + break;
> + case GICH_APR:
> + value = cpu_if->vgic_apr;
> + break;
> + case GICH_LR0 ... (GICH_LR0 + 4 * (VGIC_V2_MAX_LRS - 1)):
> + value = cpu_if->vgic_lr[(addr & 0xff) >> 2];
> + break;
> + default:
> + return 0;
> + }
> +
> + return value;
> +}
> +
> +static void vgic_mmio_write_v2_gich(struct kvm_vcpu *vcpu,
> + gpa_t addr, unsigned int len,
> + unsigned long val)
> +{
> + struct vgic_v2_cpu_if *cpu_if = vcpu_nested_if(vcpu);
> +
> + switch (addr & 0xfff) {
> + case GICH_HCR:
> + cpu_if->vgic_hcr = val;
> + break;
> + case GICH_VMCR:
> + cpu_if->vgic_vmcr = val;
> + break;
> + case GICH_APR:
> + cpu_if->vgic_apr = val;
> + break;
> + case GICH_LR0 ... (GICH_LR0 + 4 * (VGIC_V2_MAX_LRS - 1)):
> + cpu_if->vgic_lr[(addr & 0xff) >> 2] = val;
Don't you need to check if we actually support this particular LR ?
> + break;
> + }
> +}
> +
> +static const struct vgic_register_region vgic_v2_gich_registers[] = {
> + REGISTER_DESC_WITH_LENGTH(GICH_HCR,
> + vgic_mmio_read_v2_gich, vgic_mmio_write_v2_gich, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_VTR,
> + vgic_mmio_read_v2_vtr, vgic_mmio_write_wi, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_VMCR,
> + vgic_mmio_read_v2_gich, vgic_mmio_write_v2_gich, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_MISR,
> + vgic_mmio_read_v2_misr, vgic_mmio_write_wi, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_EISR0,
> + vgic_mmio_read_v2_eisr0, vgic_mmio_write_wi, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_EISR1,
> + vgic_mmio_read_v2_eisr1, vgic_mmio_write_wi, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_ELRSR0,
> + vgic_mmio_read_v2_elrsr0, vgic_mmio_write_wi, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_ELRSR1,
> + vgic_mmio_read_v2_elrsr1, vgic_mmio_write_wi, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_APR,
> + vgic_mmio_read_v2_gich, vgic_mmio_write_v2_gich, 4,
> + VGIC_ACCESS_32bit),
> + REGISTER_DESC_WITH_LENGTH(GICH_LR0,
> + vgic_mmio_read_v2_gich, vgic_mmio_write_v2_gich,
> + 4 * VGIC_V2_MAX_LRS, VGIC_ACCESS_32bit),
> +};
> --
> 1.9.1
>
>
Powered by blists - more mailing lists