2016-07-14 16:33+0700, Suravee Suthikulpanit: > On 7/14/16 16:13, Suravee Suthikulpanit wrote: >> > > unsigned long flags; >> > > + struct amd_iommu *iommu; >> > > + >> > > + if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir)) >> > > + return 0; >> > > + >> > > + for_each_iommu(iommu) { >> > > + struct amd_ir_data *ir_data; >> > > + >> > > + spin_lock_irqsave(&iommu->gatag_ir_hash_lock, flags); >> > > + >> > > + /* Note: >> > > + * We need to update all interrupt remapping table entries >> > > + * for targeting the specified vcpu. Here, we use gatag >> > > + * as a hash key and iterate through all entries in the bucket. >> > > + */ >> > > + hash_for_each_possible(iommu->gatag_ir_hash, ir_data, hnode, >> > > + AMD_IOMMU_GATAG(vm_id, vcpu_id)) { >> > > + struct irte_ga *irte = (struct irte_ga *) ir_data->entry; >> > >> > |>> (The ga_tag check is missing here too.) >> > |> >> > |> Here, the intention is to update all interrupt remapping entries in >> > the >> > |> bucket w/ the same GATAG (i.e. vm_id + vcpu_id), where GATAG = >> > |> AMD_IOMMU_GATAG(vm_id, vcpu_id). >> > >> > Which is why you need to check that >> > AMD_IOMMU_GATAG(vm_id, vcpu_id) == entry->fields_vapic.ga_tag >> > >> > The hashing function can map two different vm_id + vcpu_id to the same >> > bucket and hash_for_each_possible() would return both of them, but only >> > one belongs to the VCPU that we want to update. >> > >> > (And shouldn't there be only one match?) >> >> Actually, with your suggestion above, the hask key would be (vm_id & >> 0x3FFFFF << 8)| (vcpu_id & 0xFF). So, it should be unique for each vcpu >> of each vm, or am I still missing something? > > Ok, one scenario would be when SVM run out of the VM_ID and having to start > re-using them. Since we want SVM to generate ga_tag and just pass into IOMMU > driver for it to program the IRTE, we probably can make an assumption that > SVM would make sure that ga_tag would not conflict for each vm_id/vcpu_id. I agree, it could enable doorbell to an unscheduled VCPU and therefore lose the notification. The per-vcpu list of IRTEs would solve it as well, but making sure that no two VMs have the same id might be easier and 2^22 active VMs should be more than enough. :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html