|
From: | FelixCui-oc |
Subject: | 答复: [PATCH 1/1] Skip flatview_simplify() for specific cpu vendor |
Date: | Thu, 3 Sep 2020 11:24:15 +0000 |
>I think you're seeing issues when a guest accesses an adjacent mapping
>We're considering fixing that in the kernel, by adding a new ioctl that
>changes the whole memory map in a single step. I am CCing Peter Xu.
hi paolo, What you said is very similar to my issues. My problem is happened when a EHCI device accesses an adjacent mapping between the delete and add phases of the VFIO MemoryListener. VFIO MemoryListener is also included under address_space_memory. Does adding a new ioctl also apply to VFIO MemoryListener?
Best regards Felixcui-oc
发件人: Paolo Bonzini <pbonzini@redhat.com>
发送时间: 2020年9月3日 18:37:47 收件人: FelixCui-oc; Richard Henderson; Eduardo Habkost 抄送: qemu-devel@nongnu.org; RockCui-oc; Tony W Wang-oc; CobeChen-oc; Peter Xu 主题: Re: [PATCH 1/1] Skip flatview_simplify() for specific cpu vendor On 03/09/20 11:49, FelixCuioc wrote:
> Flatview_simplify() will merge many address ranges > into one range.When a part of the big range needs > to be changed,this will cause some innocent mappings > to be unmapped.So we want to skip flatview_simplify(). > > Signed-off-by: FelixCuioc <FelixCui-oc@zhaoxin.com> This has several issues. In no particular order: 1) you're adding host_get_vendor to target/i386/cpu.c so this does not even build for the default "../configure && make". 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. 3) you're not explaining what is the big range and how the bug is manifesting. I think you're seeing issues when a guest accesses an adjacent mapping between the delete and add phases of the KVM MemoryListener. We're considering fixing that in the kernel, by adding a new ioctl that changes the whole memory map in a single step. I am CCing Peter Xu. Paolo > --- > softmmu/memory.c | 16 +++++++++++++++- > target/i386/cpu.c | 8 ++++++++ > target/i386/cpu.h | 3 +++ > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/softmmu/memory.c b/softmmu/memory.c > index 70b93104e8..348e9db622 100644 > --- a/softmmu/memory.c > +++ b/softmmu/memory.c > @@ -699,6 +699,18 @@ static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr) > return NULL; > } > > +static bool skip_simplify(void) > +{ > + 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; > +} > + > /* Render a memory topology into a list of disjoint absolute ranges. */ > static FlatView *generate_memory_topology(MemoryRegion *mr) > { > @@ -712,7 +724,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 49d8958528..08508c8580 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -1664,6 +1664,14 @@ void host_cpuid(uint32_t function, uint32_t count, > *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) > { > uint32_t eax, ebx, ecx, edx; > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index d3097be6a5..14c8b4c09f 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 && \ > @@ -1917,6 +1919,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, > |
[Prev in Thread] | Current Thread | [Next in Thread] |