[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [RFC v2 1/1] memory: Delete assertion in memory_region_unregister_iommu_notifier




On 2020/8/5 上午4:30, Peter Xu wrote:
On Mon, Aug 03, 2020 at 06:00:34PM +0200, Eugenio Pérez wrote:
On Fri, 2020-07-03 at 15:24 +0800, Jason Wang wrote:
On 2020/7/2 下午11:45, Peter Xu wrote:
On Thu, Jul 02, 2020 at 11:01:54AM +0800, Jason Wang wrote:
So I think we agree that a new notifier is needed?
Good to me, or a new flag should be easier (IOMMU_NOTIFIER_DEV_IOTLB)?
That should work but I wonder something as following is better.

Instead of introducing new flags, how about carry the type of event in
the notifier then the device (vhost) can choose the message it want to
process like:

static vhost_iommu_event(IOMMUNotifier *n, IOMMUTLBEvent *event)

{

switch (event->type) {

case IOMMU_MAP:
case IOMMU_UNMAP:
case IOMMU_DEV_IOTLB_UNMAP:
...

}

Thanks


Hi!

Sorry, I thought I had this clear but now it seems not so clear to me. Do you mean to add that switch to the current
vhost_iommu_unmap_notify, and then the "type" field to the IOMMUTLBEntry? Is that the scope of the changes, or there is
something I'm missing?

If that is correct, what is the advantage for vhost or other notifiers? I understand that move the IOMMUTLBEntry (addr,
len) -> (iova, mask) split/transformation to the different notifiers implementation could pollute them, but this is even a deeper change and vhost is not insterested in other events but IOMMU_UNMAP, isn't?

On the other hand, who decide what type of event is? If I follow the backtrace of the assert in
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg01015.html, it seems to me that it should be
vtd_process_device_iotlb_desc. How do I know if it should be IOMMU_UNMAP or IOMMU_DEV_IOTLB_UNMAP? If I set it in some
function of memory.c, I should decide the type looking the actual notifier, isn't?
(Since Jason didn't reply yesterday, I'll try to; Jason, feel free to correct
  me...)

IMHO whether to put the type into the IOMMUTLBEntry is not important.  The
important change should be that we introduce IOMMU_DEV_IOTLB_UNMAP (or I'd
rather call it IOMMU_DEV_IOTLB directly which is shorter and cleaner).  With
that information we can make the failing assertion conditional for MAP/UNMAP
only.


Or having another dedicated device IOTLB notifier.


   We can also allow dev-iotlb messages to take arbitrary addr_mask (so it
becomes a length of address range; imho we can keep using addr_mask for
simplicity, but we can comment for addr_mask that for dev-iotlb it can be not a
real mask).


Yes.

Thanks



Thanks,



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]