[PATCH v10 13/28] whpx: change memory management logic

Mohamed Mediouni posted 28 patches 2 weeks, 1 day ago
Maintainers: Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Pedro Barbuda <pbarbuda@microsoft.com>, Mohamed Mediouni <mohamed@unpredictable.fr>, Peter Maydell <peter.maydell@linaro.org>, Shannon Zhao <shannon.zhaosl@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Alexander Graf <agraf@csgraf.de>
[PATCH v10 13/28] whpx: change memory management logic
Posted by Mohamed Mediouni 2 weeks, 1 day ago
This allows edk2 to work on Arm, although u-boot is still not functional.

Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
---
 accel/whpx/whpx-common.c | 101 +++++++++++++++------------------------
 1 file changed, 38 insertions(+), 63 deletions(-)

diff --git a/accel/whpx/whpx-common.c b/accel/whpx/whpx-common.c
index c69792e638..aefec4bc8f 100644
--- a/accel/whpx/whpx-common.c
+++ b/accel/whpx/whpx-common.c
@@ -258,89 +258,64 @@ void whpx_vcpu_kick(CPUState *cpu)
  * Memory support.
  */
 
-static void whpx_update_mapping(hwaddr start_pa, ram_addr_t size,
-                                void *host_va, int add, int rom,
-                                const char *name)
+static void whpx_set_phys_mem(MemoryRegionSection *section, bool add)
 {
     struct whpx_state *whpx = &whpx_global;
-    HRESULT hr;
-
-    /*
-    if (add) {
-        printf("WHPX: ADD PA:%p Size:%p, Host:%p, %s, '%s'\n",
-               (void*)start_pa, (void*)size, host_va,
-               (rom ? "ROM" : "RAM"), name);
-    } else {
-        printf("WHPX: DEL PA:%p Size:%p, Host:%p,      '%s'\n",
-               (void*)start_pa, (void*)size, host_va, name);
-    }
-    */
-
-    if (add) {
-        hr = whp_dispatch.WHvMapGpaRange(whpx->partition,
-                                         host_va,
-                                         start_pa,
-                                         size,
-                                         (WHvMapGpaRangeFlagRead |
-                                          WHvMapGpaRangeFlagExecute |
-                                          (rom ? 0 : WHvMapGpaRangeFlagWrite)));
-    } else {
-        hr = whp_dispatch.WHvUnmapGpaRange(whpx->partition,
-                                           start_pa,
-                                           size);
-    }
-
-    if (FAILED(hr)) {
-        error_report("WHPX: Failed to %s GPA range '%s' PA:%p, Size:%p bytes,"
-                     " Host:%p, hr=%08lx",
-                     (add ? "MAP" : "UNMAP"), name,
-                     (void *)(uintptr_t)start_pa, (void *)size, host_va, hr);
+    MemoryRegion *area = section->mr;
+    bool writable = !area->readonly && !area->rom_device;
+    WHV_MAP_GPA_RANGE_FLAGS flags;
+    uint64_t page_size = qemu_real_host_page_size();
+    uint64_t gva = section->offset_within_address_space;
+    uint64_t size = int128_get64(section->size);
+    HRESULT res;
+    void *mem;
+
+    if (!memory_region_is_ram(area)) {
+        if (writable) {
+            return;
+        } else if (!memory_region_is_romd(area)) {
+             add = false;
+        }
     }
-}
-
-static void whpx_process_section(MemoryRegionSection *section, int add)
-{
-    MemoryRegion *mr = section->mr;
-    hwaddr start_pa = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
-    unsigned int delta;
-    uint64_t host_va;
 
-    if (!memory_region_is_ram(mr)) {
-        return;
+    if (!QEMU_IS_ALIGNED(size, page_size) ||
+        !QEMU_IS_ALIGNED(gva, page_size)) {
+        /* Not page aligned, so we can not map as RAM */
+        add = false;
     }
 
-    delta = qemu_real_host_page_size() - (start_pa & ~qemu_real_host_page_mask());
-    delta &= ~qemu_real_host_page_mask();
-    if (delta > size) {
-        return;
-    }
-    start_pa += delta;
-    size -= delta;
-    size &= qemu_real_host_page_mask();
-    if (!size || (start_pa & ~qemu_real_host_page_mask())) {
+    if (!add) {
+        res = whp_dispatch.WHvUnmapGpaRange(whpx->partition,
+                gva, size);
+        if (!SUCCEEDED(res)) {
+            error_report("WHPX: failed to unmap GPA range");
+            abort();
+        }
         return;
     }
 
-    host_va = (uintptr_t)memory_region_get_ram_ptr(mr)
-            + section->offset_within_region + delta;
+    flags = WHvMapGpaRangeFlagRead | WHvMapGpaRangeFlagExecute
+     | (writable ? WHvMapGpaRangeFlagWrite : 0);
+    mem = memory_region_get_ram_ptr(area) + section->offset_within_region;
 
-    whpx_update_mapping(start_pa, size, (void *)(uintptr_t)host_va, add,
-                        memory_region_is_rom(mr), mr->name);
+    res = whp_dispatch.WHvMapGpaRange(whpx->partition,
+         mem, gva, size, flags);
+    if (!SUCCEEDED(res)) {
+        error_report("WHPX: failed to map GPA range");
+        abort();
+    }
 }
 
 static void whpx_region_add(MemoryListener *listener,
                            MemoryRegionSection *section)
 {
-    memory_region_ref(section->mr);
-    whpx_process_section(section, 1);
+    whpx_set_phys_mem(section, true);
 }
 
 static void whpx_region_del(MemoryListener *listener,
                            MemoryRegionSection *section)
 {
-    whpx_process_section(section, 0);
-    memory_region_unref(section->mr);
+    whpx_set_phys_mem(section, false);
 }
 
 static void whpx_transaction_begin(MemoryListener *listener)
-- 
2.50.1 (Apple Git-155)
Re: [PATCH v10 13/28] whpx: change memory management logic
Posted by Bernhard Beschow 3 days, 17 hours ago

Am 29. Oktober 2025 16:54:55 UTC schrieb Mohamed Mediouni <mohamed@unpredictable.fr>:
>This allows edk2 to work on Arm, although u-boot is still not functional.

This seems to fix https://gitlab.com/qemu-project/qemu/-/issues/513, at least it works for me with x86 SeaBIOS via pflash. Also, https://gitlab.com/qemu-project/qemu/-/issues/934 looks like a duplicate.

I wonder if this patch could be 10.2 material such that OVMF can finally work with WHPX.

Best regards,
Bernhard

>
>Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>---
> accel/whpx/whpx-common.c | 101 +++++++++++++++------------------------
> 1 file changed, 38 insertions(+), 63 deletions(-)
>
>diff --git a/accel/whpx/whpx-common.c b/accel/whpx/whpx-common.c
>index c69792e638..aefec4bc8f 100644
>--- a/accel/whpx/whpx-common.c
>+++ b/accel/whpx/whpx-common.c
>@@ -258,89 +258,64 @@ void whpx_vcpu_kick(CPUState *cpu)
>  * Memory support.
>  */
> 
>-static void whpx_update_mapping(hwaddr start_pa, ram_addr_t size,
>-                                void *host_va, int add, int rom,
>-                                const char *name)
>+static void whpx_set_phys_mem(MemoryRegionSection *section, bool add)
> {
>     struct whpx_state *whpx = &whpx_global;
>-    HRESULT hr;
>-
>-    /*
>-    if (add) {
>-        printf("WHPX: ADD PA:%p Size:%p, Host:%p, %s, '%s'\n",
>-               (void*)start_pa, (void*)size, host_va,
>-               (rom ? "ROM" : "RAM"), name);
>-    } else {
>-        printf("WHPX: DEL PA:%p Size:%p, Host:%p,      '%s'\n",
>-               (void*)start_pa, (void*)size, host_va, name);
>-    }
>-    */
>-
>-    if (add) {
>-        hr = whp_dispatch.WHvMapGpaRange(whpx->partition,
>-                                         host_va,
>-                                         start_pa,
>-                                         size,
>-                                         (WHvMapGpaRangeFlagRead |
>-                                          WHvMapGpaRangeFlagExecute |
>-                                          (rom ? 0 : WHvMapGpaRangeFlagWrite)));
>-    } else {
>-        hr = whp_dispatch.WHvUnmapGpaRange(whpx->partition,
>-                                           start_pa,
>-                                           size);
>-    }
>-
>-    if (FAILED(hr)) {
>-        error_report("WHPX: Failed to %s GPA range '%s' PA:%p, Size:%p bytes,"
>-                     " Host:%p, hr=%08lx",
>-                     (add ? "MAP" : "UNMAP"), name,
>-                     (void *)(uintptr_t)start_pa, (void *)size, host_va, hr);
>+    MemoryRegion *area = section->mr;
>+    bool writable = !area->readonly && !area->rom_device;
>+    WHV_MAP_GPA_RANGE_FLAGS flags;
>+    uint64_t page_size = qemu_real_host_page_size();
>+    uint64_t gva = section->offset_within_address_space;
>+    uint64_t size = int128_get64(section->size);
>+    HRESULT res;
>+    void *mem;
>+
>+    if (!memory_region_is_ram(area)) {
>+        if (writable) {
>+            return;
>+        } else if (!memory_region_is_romd(area)) {
>+             add = false;
>+        }
>     }
>-}
>-
>-static void whpx_process_section(MemoryRegionSection *section, int add)
>-{
>-    MemoryRegion *mr = section->mr;
>-    hwaddr start_pa = section->offset_within_address_space;
>-    ram_addr_t size = int128_get64(section->size);
>-    unsigned int delta;
>-    uint64_t host_va;
> 
>-    if (!memory_region_is_ram(mr)) {
>-        return;
>+    if (!QEMU_IS_ALIGNED(size, page_size) ||
>+        !QEMU_IS_ALIGNED(gva, page_size)) {
>+        /* Not page aligned, so we can not map as RAM */
>+        add = false;
>     }
> 
>-    delta = qemu_real_host_page_size() - (start_pa & ~qemu_real_host_page_mask());
>-    delta &= ~qemu_real_host_page_mask();
>-    if (delta > size) {
>-        return;
>-    }
>-    start_pa += delta;
>-    size -= delta;
>-    size &= qemu_real_host_page_mask();
>-    if (!size || (start_pa & ~qemu_real_host_page_mask())) {
>+    if (!add) {
>+        res = whp_dispatch.WHvUnmapGpaRange(whpx->partition,
>+                gva, size);
>+        if (!SUCCEEDED(res)) {
>+            error_report("WHPX: failed to unmap GPA range");
>+            abort();
>+        }
>         return;
>     }
> 
>-    host_va = (uintptr_t)memory_region_get_ram_ptr(mr)
>-            + section->offset_within_region + delta;
>+    flags = WHvMapGpaRangeFlagRead | WHvMapGpaRangeFlagExecute
>+     | (writable ? WHvMapGpaRangeFlagWrite : 0);
>+    mem = memory_region_get_ram_ptr(area) + section->offset_within_region;
> 
>-    whpx_update_mapping(start_pa, size, (void *)(uintptr_t)host_va, add,
>-                        memory_region_is_rom(mr), mr->name);
>+    res = whp_dispatch.WHvMapGpaRange(whpx->partition,
>+         mem, gva, size, flags);
>+    if (!SUCCEEDED(res)) {
>+        error_report("WHPX: failed to map GPA range");
>+        abort();
>+    }
> }
> 
> static void whpx_region_add(MemoryListener *listener,
>                            MemoryRegionSection *section)
> {
>-    memory_region_ref(section->mr);
>-    whpx_process_section(section, 1);
>+    whpx_set_phys_mem(section, true);
> }
> 
> static void whpx_region_del(MemoryListener *listener,
>                            MemoryRegionSection *section)
> {
>-    whpx_process_section(section, 0);
>-    memory_region_unref(section->mr);
>+    whpx_set_phys_mem(section, false);
> }
> 
> static void whpx_transaction_begin(MemoryListener *listener)