From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751190AbcGNNpX (ORCPT ); Thu, 14 Jul 2016 09:45:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55906 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750897AbcGNNpW (ORCPT ); Thu, 14 Jul 2016 09:45:22 -0400 Date: Thu, 14 Jul 2016 15:45:18 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Suravee Suthikulpanit Cc: joro@8bytes.org, pbonzini@redhat.com, alex.williamson@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, sherry.hurwitz@amd.com Subject: Re: [PART2 PATCH v4 07/11] iommu/amd: Introduce amd_iommu_update_ga() Message-ID: <20160714134518.GA20689@potion> References: <1468416032-7692-1-git-send-email-suravee.suthikulpanit@amd.com> <1468416032-7692-8-git-send-email-suravee.suthikulpanit@amd.com> <20160713141457.GF21976@potion> <578757A8.3000200@amd.com> <57875C4E.20203@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57875C4E.20203@amd.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 14 Jul 2016 13:45:22 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. :)