On 5/13/24 15:44, Avihai Horon wrote:

On 06/05/2024 12:20, Cédric Le Goater wrote:
External email: Use caution opening links or attachments


Let the callers do the reporting. This will be useful in
vfio_iommu_map_dirty_notify().

Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: David Hildenbrand <da...@redhat.com>
Reviewed-by: Peter Xu <pet...@redhat.com>
Signed-off-by: Cédric Le Goater <c...@redhat.com>
---
  include/exec/memory.h  | 15 ++++++++++++++-
  hw/vfio/common.c       | 13 +++++++++----
  hw/virtio/vhost-vdpa.c |  5 ++++-
  system/memory.c        | 10 +++++-----
  4 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 
dadb5cd65ab58b4868fcae06b4e301f0ecb0c1d2..2c45051b7b419c48b4e14c25f4d16a99ccd23996
 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -774,9 +774,22 @@ void 
ram_discard_manager_register_listener(RamDiscardManager *rdm,
  void ram_discard_manager_unregister_listener(RamDiscardManager *rdm,
                                               RamDiscardListener *rdl);

+/**
+ * memory_get_xlat_addr: Extract addresses from a TLB entry
+ *
+ * @iotlb: pointer to an #IOMMUTLBEntry
+ * @vaddr: virtual addressf

Nit: s/addressf/address

Fixed.


Thanks,

C.



Thanks.

+ * @ram_addr: RAM address
+ * @read_only: indicates if writes are allowed
+ * @mr_has_discard_manager: indicates memory is controlled by a
+ *                          RamDiscardManager
+ * @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Return: true on success, else false setting @errp with error.
+ */
  bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
                            ram_addr_t *ram_addr, bool *read_only,
-                          bool *mr_has_discard_manager);
+                          bool *mr_has_discard_manager, Error **errp);

  typedef struct CoalescedMemoryRange CoalescedMemoryRange;
  typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
b929bb0b7ac60dcef34c0d5a098d5d91f75501dd..da748563eb33843e93631a5240759964f33162f2
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -253,12 +253,13 @@ static bool 
vfio_listener_skipped_section(MemoryRegionSection *section)

  /* Called with rcu_read_lock held.  */
  static bool vfio_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
-                               ram_addr_t *ram_addr, bool *read_only)
+                               ram_addr_t *ram_addr, bool *read_only,
+                               Error **errp)
  {
      bool ret, mr_has_discard_manager;

      ret = memory_get_xlat_addr(iotlb, vaddr, ram_addr, read_only,
-                               &mr_has_discard_manager);
+                               &mr_has_discard_manager, errp);
      if (ret && mr_has_discard_manager) {
          /*
           * Malicious VMs might trigger discarding of IOMMU-mapped memory. The
@@ -288,6 +289,7 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
      hwaddr iova = iotlb->iova + giommu->iommu_offset;
      void *vaddr;
      int ret;
+    Error *local_err = NULL;

      trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
                                  iova, iova + iotlb->addr_mask);
@@ -304,7 +306,8 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
      if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
          bool read_only;

-        if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
+        if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, &local_err)) {
+            error_report_err(local_err);
              goto out;
          }
          /*
@@ -1213,6 +1216,7 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
      VFIOContainerBase *bcontainer = giommu->bcontainer;
      hwaddr iova = iotlb->iova + giommu->iommu_offset;
      ram_addr_t translated_addr;
+    Error *local_err = NULL;
      int ret = -EINVAL;

      trace_vfio_iommu_map_dirty_notify(iova, iova + iotlb->addr_mask);
@@ -1224,7 +1228,8 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
      }

      rcu_read_lock();
-    if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
+    if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL, &local_err)) {
+        error_report_err(local_err);
          goto out_lock;
      }

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 
e827b9175fc61f1ef419e48d90a440b00449312a..ed99ab87457d8f31b98ace960713f48d47b27102
 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -208,6 +208,7 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
      void *vaddr;
      int ret;
      Int128 llend;
+    Error *local_err = NULL;

      if (iotlb->target_as != &address_space_memory) {
          error_report("Wrong target AS \"%s\", only system memory is allowed",
@@ -227,7 +228,9 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
      if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
          bool read_only;

-        if (!memory_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, NULL)) {
+        if (!memory_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, NULL,
+                                  &local_err)) {
+            error_report_err(local_err);
              return;
          }
          ret = vhost_vdpa_dma_map(s, VHOST_VDPA_GUEST_PA_ASID, iova,
diff --git a/system/memory.c b/system/memory.c
index 
49f1cb2c3835f78f5f948531cbbf13adc706f1ad..e2b9f0aa9ea7326a5bedde28b8c777cf86bc253e
 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -2179,7 +2179,7 @@ void 
ram_discard_manager_unregister_listener(RamDiscardManager *rdm,
  /* Called with rcu_read_lock held.  */
  bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
                            ram_addr_t *ram_addr, bool *read_only,
-                          bool *mr_has_discard_manager)
+                          bool *mr_has_discard_manager, Error **errp)
  {
      MemoryRegion *mr;
      hwaddr xlat;
@@ -2197,7 +2197,7 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void 
**vaddr,
      mr = address_space_translate(&address_space_memory, 
iotlb->translated_addr,
                                   &xlat, &len, writable, 
MEMTXATTRS_UNSPECIFIED);
      if (!memory_region_is_ram(mr)) {
-        error_report("iommu map to non memory area %" HWADDR_PRIx "", xlat);
+        error_setg(errp, "iommu map to non memory area %" HWADDR_PRIx "", 
xlat);
          return false;
      } else if (memory_region_has_ram_discard_manager(mr)) {
          RamDiscardManager *rdm = memory_region_get_ram_discard_manager(mr);
@@ -2216,8 +2216,8 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void 
**vaddr,
           * were already restored before IOMMUs are restored.
           */
          if (!ram_discard_manager_is_populated(rdm, &tmp)) {
-            error_report("iommu map to discarded memory (e.g., unplugged via"
-                         " virtio-mem): %" HWADDR_PRIx "",
+            error_setg(errp, "iommu map to discarded memory (e.g., unplugged"
+                         " via virtio-mem): %" HWADDR_PRIx "",
                           iotlb->translated_addr);
              return false;
          }
@@ -2228,7 +2228,7 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void 
**vaddr,
       * check that it did not truncate too much.
       */
      if (len & iotlb->addr_mask) {
-        error_report("iommu has granularity incompatible with target AS");
+        error_setg(errp, "iommu has granularity incompatible with target AS");
          return false;
      }

--
2.45.0




Reply via email to