From mboxrd@z Thu Jan  1 00:00:00 1970
From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Subject: Re: [PART2 PATCH v4 07/11] iommu/amd: Introduce amd_iommu_update_ga()
Date: Thu, 14 Jul 2016 16:33:02 +0700
Message-ID: <57875C4E.20203@amd.com>
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>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Cc: <joro@8bytes.org>, <pbonzini@redhat.com>,
	<alex.williamson@redhat.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <sherry.hurwitz@amd.com>
To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-bn3nam01on0081.outbound.protection.outlook.com ([104.47.33.81]:13672
	"EHLO NAM01-BN3-obe.outbound.protection.outlook.com"
	rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
	id S1750739AbcGNJd0 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 14 Jul 2016 05:33:26 -0400
In-Reply-To: <578757A8.3000200@amd.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



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.

Thanks,
Suravee

Thanks,
Suravee