On Mon, Mar 23, 2015 at 5:58 PM, Andre Przywara <andre.przyw...@arm.com> wrote: > > Currently we have struct kvm_exit_mmio for encapsulating MMIO abort > data to be passed on from syndrome decoding all the way down to the > VGIC register handlers. Now as we switch the MMIO handling to be > routed through the KVM MMIO bus, it does not make sense anymore to > use that structure already from the beginning. So we put the data into > kvm_run very early and use that encapsulation till the MMIO bus call. > Then we fill kvm_exit_mmio in the VGIC only, making it a VGIC private > structure. On that way we replace the data buffer in that structure > with a pointer pointing to a single location in kvm_run, so we get > rid of some copying on the way. > I didn't bother to rename kvm_exit_mmio (to vgic_mmio or something), I would vote for the renaming.
Otherwise the patch looks much cleaner and straightforward than what it was before. Nikolay Nikolaev > because that touches a lot of code lines without any good reason. > > This is based on an original patch by Nikolay. > > Signed-off-by: Andre Przywara <andre.przyw...@arm.com> > Cc: Nikolay Nikolaev <n.nikol...@virtualopensystems.com> > --- > arch/arm/include/asm/kvm_mmio.h | 22 -------------- > arch/arm/kvm/mmio.c | 60 > ++++++++++++++++++++++++++----------- > arch/arm64/include/asm/kvm_mmio.h | 22 -------------- > include/kvm/arm_vgic.h | 3 -- > virt/kvm/arm/vgic.c | 18 +++-------- > virt/kvm/arm/vgic.h | 8 +++++ > 6 files changed, 55 insertions(+), 78 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_mmio.h b/arch/arm/include/asm/kvm_mmio.h > index 3f83db2..d8e90c8 100644 > --- a/arch/arm/include/asm/kvm_mmio.h > +++ b/arch/arm/include/asm/kvm_mmio.h > @@ -28,28 +28,6 @@ struct kvm_decode { > bool sign_extend; > }; > > -/* > - * The in-kernel MMIO emulation code wants to use a copy of run->mmio, > - * which is an anonymous type. Use our own type instead. > - */ > -struct kvm_exit_mmio { > - phys_addr_t phys_addr; > - u8 data[8]; > - u32 len; > - bool is_write; > - void *private; > -}; > - > -static inline void kvm_prepare_mmio(struct kvm_run *run, > - struct kvm_exit_mmio *mmio) > -{ > - run->mmio.phys_addr = mmio->phys_addr; > - run->mmio.len = mmio->len; > - run->mmio.is_write = mmio->is_write; > - memcpy(run->mmio.data, mmio->data, mmio->len); > - run->exit_reason = KVM_EXIT_MMIO; > -} > - > int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run); > int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run, > phys_addr_t fault_ipa); > diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c > index 5d3bfc0..bb2ab44 100644 > --- a/arch/arm/kvm/mmio.c > +++ b/arch/arm/kvm/mmio.c > @@ -122,7 +122,7 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct > kvm_run *run) > } > > static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > - struct kvm_exit_mmio *mmio) > + struct kvm_run *run) > { > unsigned long rt; > int len; > @@ -148,9 +148,9 @@ static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t > fault_ipa, > sign_extend = kvm_vcpu_dabt_issext(vcpu); > rt = kvm_vcpu_dabt_get_rd(vcpu); > > - mmio->is_write = is_write; > - mmio->phys_addr = fault_ipa; > - mmio->len = len; > + run->mmio.is_write = is_write; > + run->mmio.phys_addr = fault_ipa; > + run->mmio.len = len; > vcpu->arch.mmio_decode.sign_extend = sign_extend; > vcpu->arch.mmio_decode.rt = rt; > > @@ -162,23 +162,49 @@ static int decode_hsr(struct kvm_vcpu *vcpu, > phys_addr_t fault_ipa, > return 0; > } > > +/** > + * handle_kernel_mmio - handle an in-kernel MMIO access > + * @vcpu: pointer to the vcpu performing the access > + * @run: pointer to the kvm_run structure > + * > + * returns true if the MMIO access has been performed in kernel space, > + * and false if it needs to be emulated in user space. > + */ > +static bool handle_kernel_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run) > +{ > + int ret; > + > + if (run->mmio.is_write) { > + ret = kvm_io_bus_write(vcpu, KVM_MMIO_BUS, > run->mmio.phys_addr, > + run->mmio.len, run->mmio.data); > + > + } else { > + ret = kvm_io_bus_read(vcpu, KVM_MMIO_BUS, run->mmio.phys_addr, > + run->mmio.len, run->mmio.data); > + } > + if (!ret) { > + kvm_handle_mmio_return(vcpu, run); > + return true; > + } > + > + return false; > +} > + > int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run, > phys_addr_t fault_ipa) > { > - struct kvm_exit_mmio mmio; > unsigned long data; > unsigned long rt; > int ret; > > /* > - * Prepare MMIO operation. First stash it in a private > - * structure that we can use for in-kernel emulation. If the > - * kernel can't handle it, copy it into run->mmio and let user > - * space do its magic. > + * Prepare MMIO operation. First put the MMIO data into run->mmio. > + * Then try if some in-kernel emulation feels responsible, otherwise > + * let user space do its magic. > */ > > if (kvm_vcpu_dabt_isvalid(vcpu)) { > - ret = decode_hsr(vcpu, fault_ipa, &mmio); > + ret = decode_hsr(vcpu, fault_ipa, run); > if (ret) > return ret; > } else { > @@ -188,21 +214,21 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run > *run, > > rt = vcpu->arch.mmio_decode.rt; > > - if (mmio.is_write) { > + if (run->mmio.is_write) { > data = vcpu_data_guest_to_host(vcpu, *vcpu_reg(vcpu, rt), > - mmio.len); > + run->mmio.len); > > - trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, mmio.len, > + trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, run->mmio.len, > fault_ipa, data); > - mmio_write_buf(mmio.data, mmio.len, data); > + mmio_write_buf(run->mmio.data, run->mmio.len, data); > } else { > - trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, mmio.len, > + trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, run->mmio.len, > fault_ipa, 0); > } > > - if (vgic_handle_mmio(vcpu, run, &mmio)) > + if (handle_kernel_mmio(vcpu, run)) > return 1; > > - kvm_prepare_mmio(run, &mmio); > + run->exit_reason = KVM_EXIT_MMIO; > return 0; > } > diff --git a/arch/arm64/include/asm/kvm_mmio.h > b/arch/arm64/include/asm/kvm_mmio.h > index 9f52beb..889c908 100644 > --- a/arch/arm64/include/asm/kvm_mmio.h > +++ b/arch/arm64/include/asm/kvm_mmio.h > @@ -31,28 +31,6 @@ struct kvm_decode { > bool sign_extend; > }; > > -/* > - * The in-kernel MMIO emulation code wants to use a copy of run->mmio, > - * which is an anonymous type. Use our own type instead. > - */ > -struct kvm_exit_mmio { > - phys_addr_t phys_addr; > - u8 data[8]; > - u32 len; > - bool is_write; > - void *private; > -}; > - > -static inline void kvm_prepare_mmio(struct kvm_run *run, > - struct kvm_exit_mmio *mmio) > -{ > - run->mmio.phys_addr = mmio->phys_addr; > - run->mmio.len = mmio->len; > - run->mmio.is_write = mmio->is_write; > - memcpy(run->mmio.data, mmio->data, mmio->len); > - run->exit_reason = KVM_EXIT_MMIO; > -} > - > int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run); > int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run, > phys_addr_t fault_ipa); > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > index d6705f4..14853d8 100644 > --- a/include/kvm/arm_vgic.h > +++ b/include/kvm/arm_vgic.h > @@ -314,7 +314,6 @@ struct vgic_cpu { > struct kvm; > struct kvm_vcpu; > struct kvm_run; > -struct kvm_exit_mmio; > > int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool > write); > int kvm_vgic_hyp_init(void); > @@ -330,8 +329,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, > unsigned int irq_num, > void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg); > int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu); > int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu); > -bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run, > - struct kvm_exit_mmio *mmio); > > #define irqchip_in_kernel(k) (!!((k)->arch.vgic.in_kernel)) > #define vgic_initialized(k) (!!((k)->arch.vgic.nr_cpus)) > diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c > index 9cbb55f4..2598fe8 100644 > --- a/virt/kvm/arm/vgic.c > +++ b/virt/kvm/arm/vgic.c > @@ -758,7 +758,6 @@ static bool call_range_handler(struct kvm_vcpu *vcpu, > unsigned long offset, > const struct vgic_io_range *range) > { > - u32 *data32 = (void *)mmio->data; > struct kvm_exit_mmio mmio32; > bool ret; > > @@ -775,18 +774,12 @@ static bool call_range_handler(struct kvm_vcpu *vcpu, > mmio32.private = mmio->private; > > mmio32.phys_addr = mmio->phys_addr + 4; > - if (mmio->is_write) > - *(u32 *)mmio32.data = data32[1]; > + mmio32.data = &((u32 *)mmio->data)[1]; > ret = range->handle_mmio(vcpu, &mmio32, offset + 4); > - if (!mmio->is_write) > - data32[1] = *(u32 *)mmio32.data; > > mmio32.phys_addr = mmio->phys_addr; > - if (mmio->is_write) > - *(u32 *)mmio32.data = data32[0]; > + mmio32.data = &((u32 *)mmio->data)[0]; > ret |= range->handle_mmio(vcpu, &mmio32, offset); > - if (!mmio->is_write) > - data32[0] = *(u32 *)mmio32.data; > > return ret; > } > @@ -873,23 +866,20 @@ static int vgic_handle_mmio_access(struct kvm_vcpu > *vcpu, > mmio.phys_addr = addr; > mmio.len = len; > mmio.is_write = is_write; > - if (is_write) > - memcpy(mmio.data, val, len); > + mmio.data = val; > mmio.private = iodev->redist_vcpu; > > spin_lock(&dist->lock); > offset -= range->base; > if (vgic_validate_access(dist, range, offset)) { > updated_state = call_range_handler(vcpu, &mmio, offset, > range); > - if (!is_write) > - memcpy(val, mmio.data, len); > } else { > if (!is_write) > memset(val, 0, len); > updated_state = false; > } > spin_unlock(&dist->lock); > - kvm_prepare_mmio(run, &mmio); > + run->exit_reason = KVM_EXIT_MMIO; > kvm_handle_mmio_return(vcpu, run); > > if (updated_state) > diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h > index 28fa3aa..fc73103 100644 > --- a/virt/kvm/arm/vgic.h > +++ b/virt/kvm/arm/vgic.h > @@ -59,6 +59,14 @@ void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr > *vmcr); > bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq); > void vgic_unqueue_irqs(struct kvm_vcpu *vcpu); > > +struct kvm_exit_mmio { > + phys_addr_t phys_addr; > + void *data; > + u32 len; > + bool is_write; > + void *private; > +}; > + > void vgic_reg_access(struct kvm_exit_mmio *mmio, u32 *reg, > phys_addr_t offset, int mode); > bool handle_mmio_raz_wi(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio, > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html