From patchwork Wed Sep 25 20:38:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 20570 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ve0-f200.google.com (mail-ve0-f200.google.com [209.85.128.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 32AEE25E71 for ; Wed, 25 Sep 2013 20:37:55 +0000 (UTC) Received: by mail-ve0-f200.google.com with SMTP id oy12sf153013veb.11 for ; Wed, 25 Sep 2013 13:37:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :references:mime-version:in-reply-to:user-agent:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type :content-disposition; bh=q4HVZus/vVagLEP4uxdL7Cj4pt6sXd4SiRUoVRObj9g=; b=coxunWlfqBDj/Kq2Uq+iuCO/PM0qo/ext+HOMqtxp2zdabcdGVCURnugbTXVeZCESl iLM19bYD+9j94OizsMR7EXOUXAugQ9rk5j7w0wAg8GzKeBiywHAswuwsf84LwWM5E5Kl qxIhQKChMc6DI3o1qMMhZNtJhG4JmH5wnc7JKnBmoRPGhmHIYoCeY9U+Ba2b8nijSQSG /m5uA2l+eUiGT6WgNJATbcG6/2qvG/J9yAVoeuXJS4/pb85DvBfksL3OmhtSTgZ9Af2W I4BaXtx12Qv3XpixbFlTqdbB8p02K2HmlcjL1jzPUVQoAZ504/mIWvaUga4c8+XEhGIm O0Dg== X-Gm-Message-State: ALoCoQlj7/jYSyzy39IXVFVmSC8ORhycM2NO+ab91eNsAddlqXcHTWGys4s0nrXYnmmSe9Dimvs9 X-Received: by 10.236.180.2 with SMTP id i2mr11067054yhm.4.1380141474500; Wed, 25 Sep 2013 13:37:54 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.109.226 with SMTP id hv2ls671349qeb.22.gmail; Wed, 25 Sep 2013 13:37:54 -0700 (PDT) X-Received: by 10.220.46.72 with SMTP id i8mr35326348vcf.10.1380141474359; Wed, 25 Sep 2013 13:37:54 -0700 (PDT) Received: from mail-vc0-f179.google.com (mail-vc0-f179.google.com [209.85.220.179]) by mx.google.com with ESMTPS id wp10si10235290vdb.19.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 25 Sep 2013 13:37:54 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.179; Received: by mail-vc0-f179.google.com with SMTP id ht10so182523vcb.38 for ; Wed, 25 Sep 2013 13:37:54 -0700 (PDT) X-Received: by 10.58.181.230 with SMTP id dz6mr1938105vec.35.1380141474218; Wed, 25 Sep 2013 13:37:54 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp326970vcz; Wed, 25 Sep 2013 13:37:52 -0700 (PDT) X-Received: by 10.68.197.3 with SMTP id iq3mr34678790pbc.113.1380141471783; Wed, 25 Sep 2013 13:37:51 -0700 (PDT) Received: from mail-pb0-f54.google.com (mail-pb0-f54.google.com [209.85.160.54]) by mx.google.com with ESMTPS id ia4si9079518pbb.192.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 25 Sep 2013 13:37:51 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.160.54 is neither permitted nor denied by best guess record for domain of christoffer.dall@linaro.org) client-ip=209.85.160.54; Received: by mail-pb0-f54.google.com with SMTP id ro12so165312pbb.41 for ; Wed, 25 Sep 2013 13:37:51 -0700 (PDT) X-Received: by 10.68.172.36 with SMTP id az4mr12082563pbc.48.1380141470970; Wed, 25 Sep 2013 13:37:50 -0700 (PDT) Received: from localhost (c-67-169-183-77.hsd1.ca.comcast.net. [67.169.183.77]) by mx.google.com with ESMTPSA id uw6sm49676426pbc.8.1969.12.31.16.00.00 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 25 Sep 2013 13:37:50 -0700 (PDT) Date: Wed, 25 Sep 2013 13:38:03 -0700 From: Christoffer Dall To: Alexander Graf Cc: kvmarm@lists.cs.columbia.edu, linaro-kernel@lists.linaro.org, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, patches@linaro.org Subject: Re: [PATCH 6/8] KVM: arm-vgic: Add vgic reg access from dev attr Message-ID: <20130925203803.GF32311@cbox> References: <1377285606-15692-1-git-send-email-christoffer.dall@linaro.org> <1377285606-15692-7-git-send-email-christoffer.dall@linaro.org> <2FCDD68C-E993-4E52-AB7F-A4E5B0FB6E6F@suse.de> MIME-Version: 1.0 In-Reply-To: <2FCDD68C-E993-4E52-AB7F-A4E5B0FB6E6F@suse.de> User-Agent: Mutt/1.5.21 (2010-09-15) X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: christoffer.dall@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Content-Disposition: inline On Sun, Aug 25, 2013 at 04:21:58PM +0100, Alexander Graf wrote: > > On 23.08.2013, at 20:20, Christoffer Dall wrote: > > > Add infrastructure to handle distributor and cpu interface register > > accesses through the KVM_{GET/SET}_DEVICE_ATTR interface by adding the > > KVM_DEV_ARM_VGIC_GRP_DIST_REGS and KVM_DEV_ARM_VGIC_GRP_CPU_REGS groups > > and defining the semantics of the attr field to be the MMIO offset as > > specified in the GICv2 specs. > > > > Missing register accesses or other changes in individual register access > > functions to support save/restore of the VGIC state is added in > > subsequent patches. > > > > Signed-off-by: Christoffer Dall > > --- > > Documentation/virtual/kvm/devices/arm-vgic.txt | 35 ++++++ > > virt/kvm/arm/vgic.c | 143 ++++++++++++++++++++++++ > > 2 files changed, 178 insertions(+) > > > > diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt > > index c9febb2..1b68475 100644 > > --- a/Documentation/virtual/kvm/devices/arm-vgic.txt > > +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt > > @@ -19,3 +19,38 @@ Groups: > > KVM_VGIC_V2_ADDR_TYPE_CPU (rw, 64-bit) > > Base address in the guest physical address space of the GIC virtual cpu > > interface register mappings. > > + > > + KVM_DEV_ARM_VGIC_GRP_DIST_REGS > > + Attributes: > > + The attr field of kvm_device_attr encodes two values: > > + bits: | 63 .... 40 | 39 .. 32 | 31 .... 0 | > > + values: | reserved | cpu id | offset | > > + > > + All distributor regs are (rw, 32-bit) > > + > > + The offset is relative to the "Distributor base address" as defined in the > > + GICv2 specs. Getting or setting such a register has the same effect as > > + reading or writing the register on the actual hardware from the cpu > > + specified with cpu id field. Note that most distributor fields are not > > + banked, but return the same value regardless of the cpu id used to access > > + the register. > > + Limitations: > > + - Priorities are not implemented, and registers are RAZ/WI > > + Errors: > > + - ENODEV: Getting or setting this register is not yet supported > > + > > + KVM_DEV_ARM_VGIC_GRP_CPU_REGS > > + Attributes: > > + The attr field of kvm_device_attr encodes two values: > > + bits: | 63 .... 40 | 39 .. 32 | 31 .... 0 | > > + values: | reserved | cpu id | offset | > > + > > + All CPU regs are (rw, 32-bit) > > + > > + The offsetspecifies the offset from the "CPU interface base address" as > > offset specifies > > > + defined in the GICv2 specs. Getting or setting such a register has the > > + same effect as reading or writing the register on the actual hardware. > > + Limitations: > > + - Priorities are not implemented, and registers are RAZ/WI > > + Errors: > > + - ENODEV: Getting or setting this register is not yet supported > > diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c > > index 629caeb..e31625c 100644 > > --- a/virt/kvm/arm/vgic.c > > +++ b/virt/kvm/arm/vgic.c > > @@ -591,11 +591,29 @@ static bool handle_mmio_sgi_reg(struct kvm_vcpu *vcpu, > > return false; > > } > > > > +static bool handle_mmio_sgi_clear(struct kvm_vcpu *vcpu, > > + struct kvm_exit_mmio *mmio, > > + phys_addr_t offset) > > +{ > > + return false; > > +} > > + > > +static bool handle_mmio_sgi_set(struct kvm_vcpu *vcpu, > > + struct kvm_exit_mmio *mmio, > > + phys_addr_t offset) > > +{ > > + return false; > > +} > > + > > /* > > * I would have liked to use the kvm_bus_io_*() API instead, but it > > * cannot cope with banked registers (only the VM pointer is passed > > * around, and we need the vcpu). One of these days, someone please > > * fix it! > > + * > > + * Note that the handle_mmio implementations should not use the phys_addr > > + * field from the kvm_exit_mmio struct as this will not have any sane values > > + * when used to save/restore state from user space. > > */ > > struct mmio_range { > > phys_addr_t base; > > @@ -665,6 +683,16 @@ static const struct mmio_range vgic_dist_ranges[] = { > > .len = 4, > > .handle_mmio = handle_mmio_sgi_reg, > > }, > > + { > > + .base = GIC_DIST_SGI_CLEAR, > > + .len = VGIC_NR_SGIS, > > + .handle_mmio = handle_mmio_sgi_clear, > > + }, > > + { > > + .base = GIC_DIST_SGI_SET, > > + .len = VGIC_NR_SGIS, > > + .handle_mmio = handle_mmio_sgi_set, > > + }, > > {} > > }; > > > > @@ -1543,6 +1571,80 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write) > > return r; > > } > > > > +static bool handle_cpu_mmio_misc(struct kvm_vcpu *vcpu, > > + struct kvm_exit_mmio *mmio, phys_addr_t offset) > > +{ > > + return true; > > +} > > + > > +static const struct mmio_range vgic_cpu_ranges[] = { > > + { > > + .base = GIC_CPU_CTRL, > > + .len = 12, > > + .handle_mmio = handle_cpu_mmio_misc, > > + }, > > + { > > + .base = GIC_CPU_ALIAS_BINPOINT, > > + .len = 4, > > + .handle_mmio = handle_cpu_mmio_misc, > > + }, > > + { > > + .base = GIC_CPU_ACTIVEPRIO, > > + .len = 16, > > + .handle_mmio = handle_cpu_mmio_misc, > > + }, > > + { > > + .base = GIC_CPU_IDENT, > > + .len = 4, > > + .handle_mmio = handle_cpu_mmio_misc, > > + }, > > +}; > > + > > +static struct kvm_exit_mmio dev_attr_mmio = { .len = 4 }; > > + > > +static int vgic_attr_regs_access(struct kvm_device *dev, > > + struct kvm_device_attr *attr, > > + u32 *reg, bool is_write) > > +{ > > + const struct mmio_range *r = NULL; > > + phys_addr_t offset; > > + int cpuid; > > + struct kvm_vcpu *vcpu; > > + struct kvm_exit_mmio mmio; > > + > > + offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK; > > + cpuid = (attr->attr & KVM_DEV_ARM_VGIC_CPUID_MASK) >> > > + KVM_DEV_ARM_VGIC_CPUID_SHIFT; > > + > > + if (cpuid >= atomic_read(&dev->kvm->online_vcpus)) > > + return -EINVAL; > > + > > + vcpu = kvm_get_vcpu(dev->kvm, cpuid); > > + > > + mmio.len = 4; > > + mmio.is_write = is_write; > > + if (is_write) > > + memcpy(mmio.data, reg, sizeof(*reg)); > > Is this endianness safe? > With a big-endian kernel, no. But I suspect that breaks KVM elsewhere too. However, this is actually nicer: Thanks, -Christoffer diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c index ea7fb5c..191ff9f 100644 --- a/virt/kvm/arm/vgic.c +++ b/virt/kvm/arm/vgic.c @@ -1622,7 +1622,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev, mmio.len = 4; mmio.is_write = is_write; if (is_write) - memcpy(mmio.data, reg, sizeof(*reg)); + mmio_data_write(mmio, ~0, *reg); if (attr->group == KVM_DEV_ARM_VGIC_GRP_DIST_REGS) r = find_matching_range(vgic_dist_ranges, &mmio, offset); @@ -1638,7 +1638,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev, spin_unlock(&vcpu->kvm->arch.vgic.lock); if (!is_write) - memcpy(reg, mmio.data, sizeof(*reg)); + *reg = mmio_data_read(mmio, ~0); return 0; }