Hi,
On 14/07/14 04:21, wanghaibin wrote:
> On 2014/6/19 17:45, Andre Przywara wrote:
>
>> With everything separated and prepared, we implement a model of a
>> GICv3 distributor and redistributors by using the existing framework
>> to provide handler functions for each register group.
>> Currently we limit the emulation to a model enforcing a single
>> security state, with SRE==1 (forcing system register access) and
>> ARE==1 (allowing more than 8 VCPUs).
>> We share some of functions provided for GICv2 emulation, but take
>> the different ways of addressing (v)CPUs into account.
>> Save and restore is currently not implemented.
>>
>> Similar to the split-off GICv2 specific code, the new emulation code
>> goes into a new file (vgic-v3-emul.c).
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>
>
>> +
>> +/*
>> + * triggered by a system register access trap, called from the sysregs
>> + * handling code there.
>> + * The register contains the upper three affinity levels of the target
>> + * processors as well as a bitmask of 16 Aff0 CPUs.
>> + * Iterate over all VCPUs to check for matching ones or signal on
>> + * all-but-self if the mode bit is set.
>> + */
>> +void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg)
>> +{
>> + struct kvm *kvm = vcpu->kvm;
>> + struct kvm_vcpu *c_vcpu;
>> + struct vgic_dist *dist = &kvm->arch.vgic;
>> + u16 target_cpus;
>> + u64 mpidr, mpidr_h, mpidr_l;
>> + int sgi, mode, c, vcpu_id;
>> + int updated = 0;
>> +
>> + vcpu_id = vcpu->vcpu_id;
>> +
>> + sgi = (reg >> 24) & 0xf;
>> + mode = (reg >> 40) & 0x1;
>> + target_cpus = reg & 0xffff;
>> + mpidr = ((reg >> 48) & 0xff) << MPIDR_LEVEL_SHIFT(3);
>> + mpidr |= ((reg >> 32) & 0xff) << MPIDR_LEVEL_SHIFT(2);
>> + mpidr |= ((reg >> 16) & 0xff) << MPIDR_LEVEL_SHIFT(1);
>> + mpidr &= ~MPIDR_LEVEL_MASK;
>> +
>> + /*
>> + * We take the dist lock here, because we come from the sysregs
>> + * code path and not from MMIO (where this is already done)
>> + */
>> + spin_lock(&dist->lock);
>> + kvm_for_each_vcpu(c, c_vcpu, kvm) {
>> + if (target_cpus == 0)
>> + break;
>
>
> I can't understand here !
> First, when target_cpus == 0, if you want to break and return, it should take
> if (target_cpus == 0)
> break;
> code out of the loop, is it?
That is what it does, right? TBH, I don't understand your comment here.
> Second, I think this case (target_cpus == 0) should be not break, when mode == 1, it should
> send SGI to all processors in the system, excluding itself.(spec 18)
That is right, good catch. I added this line later as an optimization to
avoid iterating over all VCPUs after the single one requested has been
served. But mode needs to be taken into account.
I have the fix in my tree already.
>
>> + if (mode && c == vcpu_id) /* not to myself */
>> + continue;
>
> Is there a mistake? for example: vcpu3 want to send sgi to all *other* vcpu, but , it will send SGI
> to all vcpu, *INCLUDE* vcpu3.
Why that? If this is vcpu3 for instance then we will continue, meaning
skipping the rest of the loop which actually injects the SGI. So
all-but-self. Or am I missing something here?
>
>> + if (!mode) {
>> + mpidr_h = kvm_vcpu_get_mpidr(c_vcpu);
>> + mpidr_l = MPIDR_AFFINITY_LEVEL(mpidr_h, 0);
>> + mpidr_h &= ~MPIDR_LEVEL_MASK;
>> + if (mpidr != mpidr_h)
>> + continue;
>> + if (!(target_cpus & BIT(mpidr_l)))
>> + continue;
>> + target_cpus &= ~BIT(mpidr_l);
>> + }
>> + /* Flag the SGI as pending */
>> + vgic_dist_irq_set(c_vcpu, sgi);
>> + updated = 1;
>> + kvm_debug("SGI%d from CPU%d to CPU%d\n", sgi, vcpu_id, c);
>> + }
>> + if (updated)
>> + vgic_update_state(vcpu->kvm);
>> + spin_unlock(&dist->lock);
>> + if (updated)
>> + vgic_kick_vcpus(vcpu->kvm);
>> +}
>
>
>> diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
>> index 9078e2a..f24cdd0 100644
>> --- a/virt/kvm/arm/vgic.h
>> +++ b/virt/kvm/arm/vgic.h
>> @@ -34,6 +34,8 @@
>>
>> #define VCPU_NOT_ALLOCATED ((u8)-1)
>>
>> +#define VCPU_NOT_ALLOCATED ((u8)-1)
>
>
> This are two same defines...
Again well spotted. Will fix it.
Thanks for looking at the patches!
Regards,
Andre.
>
>> +
>> unsigned long *vgic_bitmap_get_shared_map(struct vgic_bitmap *x);
>>
>> void vgic_update_state(struct kvm *kvm);
>> @@ -112,3 +114,4 @@ int vgic_set_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
>> int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
>>
>> bool vgic_v2_init_emulation_ops(struct kvm *kvm, int type);
>> +bool vgic_v3_init_emulation_ops(struct kvm *kvm, int type);
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 4b6c01b..c2fac5d 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -2286,6 +2286,9 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>> case KVM_DEV_TYPE_ARM_VGIC_V2:
>> ops = &kvm_arm_vgic_v2_ops;
>> break;
>> + case KVM_DEV_TYPE_ARM_VGIC_V3:
>> + ops = &kvm_arm_vgic_v3_ops;
>> + break;
>> #endif
>> #ifdef CONFIG_S390
>> case KVM_DEV_TYPE_FLIC:
>
>
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@xxxxxxxxxxxxxxxxxxxxx
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
