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

Reply via email to