On 22/02/21 12:56, David Hildenbrand wrote:
+ /**
+ * @get_min_granularity:
+ *
+ * Get the minimum granularity in which listeners will get notified
+ * about changes within the #MemoryRegion via the #RamDiscardMgr.
+ *
+ * @rdm: the #RamDiscardMgr
+ * @mr: the #MemoryRegion
+ *
+ * Returns the minimum granularity.
+ */
+ uint64_t (*get_min_granularity)(const RamDiscardMgr *rdm,
+ const MemoryRegion *mr);
+
+ /**
+ * @is_populated:
+ *
+ * Check whether the given range within the #MemoryRegion is completely
+ * populated (i.e., no parts are currently discarded). There are no
+ * alignment requirements for the range.
+ *
+ * @rdm: the #RamDiscardMgr
+ * @mr: the #MemoryRegion
+ * @offset: offset into the #MemoryRegion
+ * @size: size in the #MemoryRegion
+ *
+ * Returns whether the given range is completely populated.
+ */
+ bool (*is_populated)(const RamDiscardMgr *rdm, const MemoryRegion *mr,
+ ram_addr_t offset, ram_addr_t size);
+
+ /**
+ * @register_listener:
+ *
+ * Register a #RamDiscardListener for a #MemoryRegion via the
+ * #RamDiscardMgr and immediately notify the #RamDiscardListener about all
+ * populated parts within the #MemoryRegion via the #RamDiscardMgr.
+ *
+ * In case any notification fails, no further notifications are triggered
+ * and an error is logged.
+ *
+ * @rdm: the #RamDiscardMgr
+ * @mr: the #MemoryRegion
+ * @rdl: the #RamDiscardListener
+ */
+ void (*register_listener)(RamDiscardMgr *rdm, const MemoryRegion *mr,
+ RamDiscardListener *rdl);
+
+ /**
+ * @unregister_listener:
+ *
+ * Unregister a previously registered #RamDiscardListener for a
+ * #MemoryRegion via the #RamDiscardMgr after notifying the
+ * #RamDiscardListener about all populated parts becoming unpopulated
+ * within the #MemoryRegion via the #RamDiscardMgr.
+ *
+ * @rdm: the #RamDiscardMgr
+ * @mr: the #MemoryRegion
+ * @rdl: the #RamDiscardListener
+ */
+ void (*unregister_listener)(RamDiscardMgr *rdm, const MemoryRegion *mr,
+ RamDiscardListener *rdl);
+
+ /**
+ * @replay_populated:
+ *
+ * Notify the #RamDiscardListener about all populated parts within the
+ * #MemoryRegion via the #RamDiscardMgr.
+ *
+ * In case any notification fails, no further notifications are triggered.
+ * The listener is not required to be registered.
+ *
+ * @rdm: the #RamDiscardMgr
+ * @mr: the #MemoryRegion
+ * @rdl: the #RamDiscardListener
+ *
+ * Returns 0 on success, or a negative error if any notification failed.
+ */
+ int (*replay_populated)(const RamDiscardMgr *rdm, const MemoryRegion *mr,
+ RamDiscardListener *rdl);
If this function is only going to use a single callback, just pass it
(together with a void *opaque) as the argument.
+};
+
typedef struct CoalescedMemoryRange CoalescedMemoryRange;
typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
@@ -487,6 +683,7 @@ struct MemoryRegion {
const char *name;
unsigned ioeventfd_nb;
MemoryRegionIoeventfd *ioeventfds;
+ RamDiscardMgr *rdm; /* Only for RAM */
};
The idea of sending discard notifications is obviously good. I have a
couple of questions on the design that you used for the interface; I'm
not saying that it should be done differently, I would only like to
understand the trade offs that you chose:
1) can the RamDiscardManager (no abbreviations :)) be just the owner of
the memory region (obj->parent)? Alternatively, if you want to make it
separate from the owner, does it make sense for it to be a separate
reusable object, sitting between virtio-mem and the MemoryRegion, so
that the implementation can be reused?
2) why have the new RamDiscardListener instead of just extending
MemoryListener? Conveniently, vfio already has a MemoryListener that can
be extended, and you wouldn't need the list of RamDiscardListeners.
There is already a precedent of replaying the current state when a
listener is added (see listener_add_address_space), so this is not
something different between ML and RDL.
Also, if you add a new interface, you should have "method call" wrappers
similar to user_creatable_complete or user_creatable_can_be_deleted.
Thanks,
Paolo
