| Message ID | 20130925203803.GF32311@cbox |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 25, 2013 at 01:38:03PM -0700, Christoffer Dall wrote: > 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 <christoffer.dall@linaro.org> > > > --- > > > 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: > > 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); ahem, that's &mmio > > 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); ahem, that's &mmio > > return 0; > } > > 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; }