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

[Suravee Suthikulpanit]

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