On 2014/6/19 17:45, Andre Przywara wrote:
> Some GICv3 registers can and will be accessed as 64 bit registers.
> Currently the register handling code can only deal with 32 bit
> accesses, so we do two consecutive calls to cover this.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
> virt/kvm/arm/vgic.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 4c6b212..b3cf4c7 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -906,6 +906,48 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
> }
>
> /*
> + * Call the respective handler function for the given range.
> + * We split up any 64 bit accesses into two consecutive 32 bit
> + * handler calls and merge the result afterwards.
> + */
> +static bool call_range_handler(struct kvm_vcpu *vcpu,
> + struct kvm_exit_mmio *mmio,
> + unsigned long offset,
> + const struct mmio_range *range)
> +{
> + u32 *data32 = (void *)mmio->data;
> + struct kvm_exit_mmio mmio32;
> + bool ret;
> +
> + if (likely(mmio->len <= 4))
> + return range->handle_mmio(vcpu, mmio, offset);
> +
> + /*
> + * We assume that any access greater than 4 bytes is actually
> + * 8 bytes long, caused by a 64-bit access
> + */
> +
> + mmio32.len = 4;
> + mmio32.is_write = mmio->is_write;
> +
> + mmio32.phys_addr = mmio->phys_addr + 4;
> + if (mmio->is_write)
> + *(u32 *)mmio32.data = data32[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];
> + ret |= range->handle_mmio(vcpu, &mmio32, offset);
> + if (!mmio->is_write)
> + data32[0] = *(u32 *)mmio32.data;
> +
> + return ret;
> +}
It seems that this func will treat (the mmio->len >=4) as same as (mmio->len == 8).
About VGIC_v2 module, it will be access successful that guest does (ldrb r0, [r1, #5], remember the GICD_CFG access BUG? you show this example to me),
there is no need the offset align and no need mmio->len == 4.
Obviously, the 64-bits reg access func has many limits(Though I still think that guest will guarantee the offset align) :) .
> +
> +/*
> * vgic_handle_mmio_range - handle an in-kernel MMIO access
> * @vcpu: pointer to the vcpu performing the access
> * @run: pointer to the kvm_run structure
> @@ -936,10 +978,10 @@ static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
> spin_lock(&vcpu->kvm->arch.vgic.lock);
> offset -= range->base;
> if (vgic_validate_access(dist, range, offset)) {
> - updated_state = range->handle_mmio(vcpu, mmio, offset);
> + updated_state = call_range_handler(vcpu, mmio, offset, range);
> } else {
> - vgic_reg_access(mmio, NULL, offset,
> - ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
Likely, and we hope that the VM access VGIC is validate, but there maybe something wrong bring on this branch (at present,only irq >= dist->nr_irqs ) for VGIC invalidate access,
So I think there only READ_RAZ and WRITE_IGNORED may be not enough, think that, VM may get the irq' attr and the irq is overlap, and the VGIC will return a 0 to VM, and without
any warning info.
Maybe there need some warning,or inject the abort to VM, and so on.
> + if (!mmio->is_write)
> + memset(mmio->data, 0, mmio->len);
> updated_state = false;
> }
> spin_unlock(&vcpu->kvm->arch.vgic.lock);
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
