diff mbox series

[1/1] Skip flatview_simplify() for cpu vendor zhaoxin

Message ID 20201016112933.14856-2-FelixCui-oc@zhaoxin.com
State New
Headers show
Series Skip flatview_simplify() for cpu vendor zhaoxin | expand

Commit Message

FelixCuioc Oct. 16, 2020, 11:29 a.m. UTC 
The issue here is that an assinged EHCI device accesses
an adjacent mapping between the delete and add phases
of the VFIO MemoryListener.
We want to skip flatview_simplify() is to prevent EHCI
device IOVA mappings from being unmapped.

Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com>
---
 softmmu/memory.c  | 20 +++++++++++++++++++-
 target/i386/cpu.c |  7 +++++++
 target/i386/cpu.h |  3 +++
 3 files changed, 29 insertions(+), 1 deletion(-)

Comments

Paolo Bonzini Oct. 16, 2020, 11:42 a.m. UTC | #1 
On 16/10/20 13:29, FelixCuioc wrote:
> The issue here is that an assinged EHCI device accesses

> an adjacent mapping between the delete and add phases

> of the VFIO MemoryListener.

> We want to skip flatview_simplify() is to prevent EHCI

> device IOVA mappings from being unmapped.


Hi,

there is indeed a bug, but I have already explained last month
(https://mail.gnu.org/archive/html/qemu-devel/2020-09/msg01279.html)
that this patch is conceptually wrong:

1) you're adding host_get_vendor conditioned on compiling the x86
emulator, so you are breaking compilation on non-x86 machines.

2) you're adding a check for the host, but the bug applies to all hosts.
 If there is a bug on x86 hardware emulation, it should be fixed even
when emulating x86 from ARM.  It should also apply to all CPU vendors.

Alex, the issue here is that the delete+add passes are racing against an
assigned device's DMA. For KVM we were thinking of changing the whole
memory map with a single ioctl, but that's much easier because KVM
builds its page tables lazily. It would be possible for the IOMMU too
but it would require a relatively complicated comparison of the old and
new memory maps in the kernel.

Paolo
diff mbox series

Patch

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 403ff3abc9..a998018d87 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -700,6 +700,22 @@  static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
     return NULL;
 }
 
+static bool skip_simplify(void)
+{
+#ifdef I386_CPU_H
+    char vendor[CPUID_VENDOR_SZ + 1] = { 0 };
+    host_get_vendor(vendor);
+    if (!strncmp(vendor, CPUID_VENDOR_VIA, strlen(CPUID_VENDOR_VIA))
+        || !strncmp(vendor, CPUID_VENDOR_ZHAOXIN,
+                  strlen(CPUID_VENDOR_ZHAOXIN))) {
+        return true;
+    }
+    return false;
+#else
+    return false;
+#endif
+}
+
 /* Render a memory topology into a list of disjoint absolute ranges. */
 static FlatView *generate_memory_topology(MemoryRegion *mr)
 {
@@ -713,7 +729,9 @@  static FlatView *generate_memory_topology(MemoryRegion *mr)
                              addrrange_make(int128_zero(), int128_2_64()),
                              false, false);
     }
-    flatview_simplify(view);
+    if (!skip_simplify()) {
+        flatview_simplify(view);
+    }
 
     view->dispatch = address_space_dispatch_new(view);
     for (i = 0; i < view->nr; i++) {
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5d713c8528..6bda35a03e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1559,6 +1559,13 @@  void host_cpuid(uint32_t function, uint32_t count,
     if (edx)
         *edx = vec[3];
 }
+void host_get_vendor(char *vendor)
+{
+  uint32_t eax, ebx, ecx, edx;
+
+  host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
+  x86_cpu_vendor_words2str(vendor, ebx, edx, ecx);
+}
 
 void host_vendor_fms(char *vendor, int *family, int *model, int *stepping)
 {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 51c1d5f60a..e8e26e6a53 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -832,6 +832,8 @@  typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 
 #define CPUID_VENDOR_VIA   "CentaurHauls"
 
+#define CPUID_VENDOR_ZHAOXIN   "  Shanghai  "
+
 #define CPUID_VENDOR_HYGON    "HygonGenuine"
 
 #define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
@@ -1918,6 +1920,7 @@  void cpu_clear_apic_feature(CPUX86State *env);
 void host_cpuid(uint32_t function, uint32_t count,
                 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
 void host_vendor_fms(char *vendor, int *family, int *model, int *stepping);
+void host_get_vendor(char *vendor);
 
 /* helper.c */
 bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,