[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag |
Date: |
Mon, 5 Sep 2016 11:56:12 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
On 05/09/2016 10:38, Peter Xu wrote:
> However in this patch I was not meant to do that. I made it an
> exclusive flag to identify two different use cases. I don't know
> whether this is good, but at least for Intel IOMMU's current use case,
> these two types should be totally isolated from each other:
>
> - IOMMU_NONE notification is used by future DMAR-enabled vhost, it
> should only be notified with device-IOTLB invalidations, this will
> only require "Device IOTLB" capability for Intel IOMMUs, and be
> notified in Device IOTLB invalidation handlers.
>
> - IOMMU_RW notifications should only be used for vfio-pci, notified
> with IOTLB invalidations. This will only require Cache Mode (CM=1)
> capability, and will be notified in common IOTLB invalidations (no
> matter whether it's an cache invalidation or not, we will all use
> IOMMU_RW flag for this kind of notifies).
>
> Maybe here naming the flags as IOMMU_{RW_NONE} is a little bit
> confusing (just to leverage existing access flags),
Yeah, if you really want to have these semantics, you need to define an
enum like this:
IOMMU_NOTIFIER_NONE = -1,
IOMMU_NOTIFIER_FLUSH = 0,
IOMMU_NOTIFIER_CHANGED_ENTRY = 1,
But I'm still not convinced of the exclusivity between "flush" and
"entry changed" notifiers. If I saw the above, my first reaction would
be that you need a bit mask:
IOMMU_NOTIFIER_NONE = -1,
IOMMU_NOTIFIER_FLUSH = 1,
IOMMU_NOTIFIER_CHANGED_ENTRY = 2,
But perhaps what you're looking for is to change the "notifier" to a
"listener" like
struct IOMMUListener {
void (*flush)(IOMMUListener *);
void (*entry_changed)(IOMMUListener *, IOMMUTLBEntry *);
QLIST_ENTRY(IOMMUListener) node;
};
The patches can start with an IOMMUListener that only has the
entry_changed callback and that replaces the current use of Notifier.
Then notify_started and notify_stopped can be called on every notifier
that is added/removed (see attached prototype), and the Intel IOMMU can
simply reject registration of a listener that has a non-NULL
iotlb_changed member.
Thanks,
Paolo
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 6bc4d4d..d8cbd90 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -156,14 +156,20 @@ static uint64_t spapr_tce_get_min_page_size(MemoryRegion
*iommu)
return 1ULL << tcet->page_shift;
}
-static void spapr_tce_notify_started(MemoryRegion *iommu)
+static void spapr_tce_listener_add(MemoryRegion *iommu, IOMMUListener *l)
{
- spapr_tce_set_need_vfio(container_of(iommu, sPAPRTCETable, iommu), true);
+ sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
+ if (tcet->users++ == 0) {
+ spapr_tce_set_need_vfio(tcet, true);
+ }
}
-static void spapr_tce_notify_stopped(MemoryRegion *iommu)
+static void spapr_tce_listener_del(MemoryRegion *iommu, IOMMUListener *l)
{
- spapr_tce_set_need_vfio(container_of(iommu, sPAPRTCETable, iommu), false);
+ sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
+ if (--tcet->users == 0) {
+ spapr_tce_set_need_vfio(tcet, false);
+ }
}
static int spapr_tce_table_post_load(void *opaque, int version_id)
@@ -246,8 +252,8 @@ static const VMStateDescription vmstate_spapr_tce_table = {
static MemoryRegionIOMMUOps spapr_iommu_ops = {
.translate = spapr_tce_translate_iommu,
.get_min_page_size = spapr_tce_get_min_page_size,
- .notify_started = spapr_tce_notify_started,
- .notify_stopped = spapr_tce_notify_stopped,
+ .listener_add = spapr_tce_listener_add,
+ .listener_del = spapr_tce_listener_del,
};
static int spapr_tce_table_realize(DeviceState *dev)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index caf7be9..4761afd 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -558,6 +558,7 @@ struct sPAPRTCETable {
uint64_t *table;
uint32_t mig_nb_table;
uint64_t *mig_table;
+ int users;
bool bypass;
bool need_vfio;
int fd;
diff --git a/memory.c b/memory.c
index 0eb6895..e9f50af 100644
--- a/memory.c
+++ b/memory.c
@@ -1515,9 +1515,8 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t
client)
void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n)
{
- if (mr->iommu_ops->notify_started &&
- QLIST_EMPTY(&mr->iommu_notify.notifiers)) {
- mr->iommu_ops->notify_started(mr);
+ if (mr->iommu_ops->listener_add) {
+ mr->iommu_ops->listener_add(mr, ...);
}
notifier_list_add(&mr->iommu_notify, n);
}
@@ -1555,9 +1554,8 @@ void memory_region_iommu_replay(MemoryRegion *mr,
Notifier *n, bool is_write)
void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n)
{
notifier_remove(n);
- if (mr->iommu_ops->notify_stopped &&
- QLIST_EMPTY(&mr->iommu_notify.notifiers)) {
- mr->iommu_ops->notify_stopped(mr);
+ if (mr->iommu_ops->listener_del) {
+ mr->iommu_ops->listener_del(mr, ...);
}
}
- [Qemu-devel] [PATCH 0/3] memory: add IOMMU notifier type, Peter Xu, 2016/09/05
- [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/05
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Paolo Bonzini, 2016/09/05
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/05
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Paolo Bonzini, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Paolo Bonzini, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/07
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/07
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/07
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/08
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/11