Re: [PART2 PATCH v6 12/12] svm: Implements update_pi_irte hook to setup posted interrupt

[Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>]

Hi Radim,

On 08/22/2016 04:19 PM, Suravee Suthikulpanit wrote:
>> he problem with wrappers is that we don't know what list we should
>> remove the "struct amd_ir_data" from;  we would need to add another
>> tracking structure or go through all VCPUs.
>>
>> Having "struct list_head" in "struct amd_ir_data" would allow us to know
>> the current list and remove it from there:
>> One "struct amd_ir_data" should never be used by more than one caller of
>> amd_iommu_update_ga(), because they would have to be cooperating anyway,
>> which would mean a single mediator, so we can add a "struct list_head"
>> into "struct amd_ir_data".
>>
>>    Minor design note:
>>    To make the usage of "struct amd_ir_data" safer, we could pass "struct
>>    list_head" into irq_set_vcpu_affinity(), instead of returning "struct
>>    amd_ir_data *".
>>
>>    irq_set_vcpu_affinity() would add "struct amd_ir_data" to the list
>> only
>>    if ir_data was not already in some list and report whether the list
>>    was modified.
>>
>> I think that adding "struct list_head" into "struct amd_ir_data" is
>> nicer than having wrappers.
>>
>> Joerg, Paolo, what do you think?
>>
>
> I think modifying irq_set_vcpu_affinity() to also pass struct list_head
> seems a bit redundant since it is currently design to allow passing in
> void *, which leaves the other option where we might just need to pass
> in a wrapper (e.g. going back to the previous design where we pass in
> struct amd_iommu_pi_data) and also add a pointer to the ir_list in the
> wrapper as well. Then, IOMMU is responsible for adding/deleting ir_data
> to/from this list instead of SVM. This should be fine since we only need
> to coordinate b/w SVM and AMD-IOMMU.
>
> Thanks,
> Suravee

Actually, thinking about this again, going back to keeping the per-vcpu 
list of struct amd_iommu_pi_data is probably the simplest here.

* We avoid having to expose the amd_ir_data to SVM.
* We can match using amd_ir_data * when traversing the list.
* We can easily add the code to manage the list in the SVM. We can make 
sure that the struct amd_iommu_pi_data is not already mapped before 
adding it to a new per-vcpu list. If it is currently mapped, we can 
simply unmapped it. Doing this from IOMMU would be more complicate and 
require lots of parameter passing.

Thanks,
Suravee