-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new manufacturer to allow DIO mode to work with the emulated flash (QEMU-45) #18
Conversation
Hi @a159x36, appreciate your PR. Would you mind adding a description, explaining the change? Does Gigadevice flash actually have this quirk, or you are adding it to let the emulation behave correctly? Also it looks like the GD behavior is now the same as Windbond. Would it be a simpler change to switch the flash type from "gd25q32" to "w25q32" in esp32.c? Line 620 in 2ec9f04
|
OK, I've added a description. I reckon the best way to fix this is to fix the emulated flash rather than pretend it's a winbond device but like you say that would work too. |
Thanks for the explanation, it makes sense to me now! Regarding the interrupt matrix, I'll have a look. The thing you mention might fix another odd behavior I was seeing, so thank you for that! I've also taken a look at the commits in your fork. The idea to add HSPI emulation along with the LCD model is great! Do you think the performance will be much worse if we split the emulation into a QEMU SSI master (HSPI) and an SSI slave device (LCD)? (It's really awesome to hear that this project is helping you in a university course!) |
Signed-off-by: Martin Johnson M.J.Johnson@massey.ac.nz |
I did try to split into a master and a slave device but the spi transaction was too slow. It should be possible to make it work well but I don't know enough about qemu internals to do that so putting everything in the SPI controller was the easiest thing to do. https://github.com/a159x36/TTGODemo It should work on Windows, Linux and OSX but doesn't include the DIO flash fix yet. Thanks again. |
Include the qtest reproducer provided by Alexander Bulekov in https://gitlab.com/qemu-project/qemu/-/issues/542. Without the previous commit, we get: $ make check-qtest-i386 ... Running test tests/qtest/intel-hda-test AddressSanitizer:DEADLYSIGNAL ================================================================= ==1580408==ERROR: AddressSanitizer: stack-overflow on address 0x7ffc3d566fe0 #0 0x63d297cf in address_space_translate_internal softmmu/physmem.c:356 #1 0x63d27260 in flatview_do_translate softmmu/physmem.c:499:15 #2 0x63d27af5 in flatview_translate softmmu/physmem.c:565:15 #3 0x63d4ce84 in flatview_write softmmu/physmem.c:2850:10 #4 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18 #5 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16 #6 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12 #7 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12 #8 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12 #9 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1 #10 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1 #11 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12 #12 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5 #13 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5 #14 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5 #15 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9 #16 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5 #17 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9 #18 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5 #19 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5 #20 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18 #21 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16 #22 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23 #23 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12 #24 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18 #25 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16 #26 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12 #27 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12 #28 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12 #29 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1 #30 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1 #31 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12 #32 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5 #33 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5 #34 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5 #35 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9 #36 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5 #37 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9 #38 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5 #39 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5 #40 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18 #41 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16 #42 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23 #43 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12 #44 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18 #45 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16 #46 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12 qemu#47 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12 #48 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12 ... SUMMARY: AddressSanitizer: stack-overflow softmmu/physmem.c:356 in address_space_translate_internal ==1580408==ABORTING Broken pipe Aborted (core dumped) Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Thomas Huth <thuth@redhat.com> Message-Id: <20211218160912.1591633-4-philmd@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
The issue reported by OSS-Fuzz produces the following backtrace: ==447470==ERROR: AddressSanitizer: heap-buffer-overflow READ of size 1 at 0x61500002a080 thread T0 #0 0x71766d47 in sdhci_read_dataport hw/sd/sdhci.c:474:18 #1 0x7175f139 in sdhci_read hw/sd/sdhci.c:1022:19 #2 0x721b937b in memory_region_read_accessor softmmu/memory.c:440:11 #3 0x72171e51 in access_with_adjusted_size softmmu/memory.c:554:18 #4 0x7216f47c in memory_region_dispatch_read1 softmmu/memory.c:1424:16 #5 0x7216ebb9 in memory_region_dispatch_read softmmu/memory.c:1452:9 #6 0x7212db5d in flatview_read_continue softmmu/physmem.c:2879:23 #7 0x7212f958 in flatview_read softmmu/physmem.c:2921:12 #8 0x7212f418 in address_space_read_full softmmu/physmem.c:2934:18 #9 0x721305a9 in address_space_rw softmmu/physmem.c:2962:16 #10 0x7175a392 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12 #11 0x7175a0ea in dma_memory_rw include/sysemu/dma.h:132:12 #12 0x71759684 in dma_memory_read include/sysemu/dma.h:152:12 #13 0x7175518c in sdhci_do_adma hw/sd/sdhci.c:823:27 #14 0x7174bf69 in sdhci_data_transfer hw/sd/sdhci.c:935:13 #15 0x7176aaa7 in sdhci_send_command hw/sd/sdhci.c:376:9 #16 0x717629ee in sdhci_write hw/sd/sdhci.c:1212:9 #17 0x72172513 in memory_region_write_accessor softmmu/memory.c:492:5 #18 0x72171e51 in access_with_adjusted_size softmmu/memory.c:554:18 #19 0x72170766 in memory_region_dispatch_write softmmu/memory.c:1504:16 #20 0x721419ee in flatview_write_continue softmmu/physmem.c:2812:23 #21 0x721301eb in flatview_write softmmu/physmem.c:2854:12 #22 0x7212fca8 in address_space_write softmmu/physmem.c:2950:18 #23 0x721d9a53 in qtest_process_command softmmu/qtest.c:727:9 A DMA descriptor is previously filled in RAM. An I/O access to the device (frames #22 to #16) start the DMA engine (frame #13). The engine fetch the descriptor and execute the request, which itself accesses the SDHCI I/O registers (frame #1 and #0), triggering a re-entrancy issue. Fix by prohibit transactions from the DMA to devices. The DMA engine is thus restricted to memories. Reported-by: OSS-Fuzz (Issue 36391) Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/451 Message-Id: <20211215205656.488940-3-philmd@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
Include the qtest reproducer provided by Alexander Bulekov in https://gitlab.com/qemu-project/qemu/-/issues/451. Without the previous commit, we get: $ make check-qtest-i386 ... Running test qtest-i386/fuzz-sdcard-test ==447470==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61500002a080 at pc 0x564c71766d48 bp 0x7ffc126c62b0 sp 0x7ffc126c62a8 READ of size 1 at 0x61500002a080 thread T0 #0 0x564c71766d47 in sdhci_read_dataport hw/sd/sdhci.c:474:18 #1 0x564c7175f139 in sdhci_read hw/sd/sdhci.c:1022:19 #2 0x564c721b937b in memory_region_read_accessor softmmu/memory.c:440:11 #3 0x564c72171e51 in access_with_adjusted_size softmmu/memory.c:554:18 #4 0x564c7216f47c in memory_region_dispatch_read1 softmmu/memory.c:1424:16 #5 0x564c7216ebb9 in memory_region_dispatch_read softmmu/memory.c:1452:9 #6 0x564c7212db5d in flatview_read_continue softmmu/physmem.c:2879:23 #7 0x564c7212f958 in flatview_read softmmu/physmem.c:2921:12 #8 0x564c7212f418 in address_space_read_full softmmu/physmem.c:2934:18 #9 0x564c721305a9 in address_space_rw softmmu/physmem.c:2962:16 #10 0x564c7175a392 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12 #11 0x564c7175a0ea in dma_memory_rw include/sysemu/dma.h:132:12 #12 0x564c71759684 in dma_memory_read include/sysemu/dma.h:152:12 #13 0x564c7175518c in sdhci_do_adma hw/sd/sdhci.c:823:27 #14 0x564c7174bf69 in sdhci_data_transfer hw/sd/sdhci.c:935:13 #15 0x564c7176aaa7 in sdhci_send_command hw/sd/sdhci.c:376:9 #16 0x564c717629ee in sdhci_write hw/sd/sdhci.c:1212:9 #17 0x564c72172513 in memory_region_write_accessor softmmu/memory.c:492:5 #18 0x564c72171e51 in access_with_adjusted_size softmmu/memory.c:554:18 #19 0x564c72170766 in memory_region_dispatch_write softmmu/memory.c:1504:16 #20 0x564c721419ee in flatview_write_continue softmmu/physmem.c:2812:23 #21 0x564c721301eb in flatview_write softmmu/physmem.c:2854:12 #22 0x564c7212fca8 in address_space_write softmmu/physmem.c:2950:18 #23 0x564c721d9a53 in qtest_process_command softmmu/qtest.c:727:9 0x61500002a080 is located 0 bytes to the right of 512-byte region [0x615000029e80,0x61500002a080) allocated by thread T0 here: #0 0x564c708e1737 in __interceptor_calloc (qemu-system-i386+0x1e6a737) #1 0x7ff05567b5e0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x5a5e0) #2 0x564c71774adb in sdhci_pci_realize hw/sd/sdhci-pci.c:36:5 SUMMARY: AddressSanitizer: heap-buffer-overflow hw/sd/sdhci.c:474:18 in sdhci_read_dataport Shadow bytes around the buggy address: 0x0c2a7fffd3c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2a7fffd3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2a7fffd3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2a7fffd3f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2a7fffd400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c2a7fffd410:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2a7fffd420: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2a7fffd430: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2a7fffd440: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2a7fffd450: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2a7fffd460: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Heap left redzone: fa Freed heap region: fd ==447470==ABORTING Broken pipe ERROR qtest-i386/fuzz-sdcard-test - too few tests run (expected 3, got 2) Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Thomas Huth <thuth@redhat.com> Message-Id: <20211215205656.488940-4-philmd@redhat.com> [thuth: Replaced "-m 4G" with "-m 512M"] Signed-off-by: Thomas Huth <thuth@redhat.com>
Have submitted the patch upstream, will follow up on the qemu-devel mailing list. |
This is below memleak detected when to quit the qemu-system-x86_64 (with vhost-scsi-pci). (qemu) quit ================================================================= ==15568==ERROR: LeakSanitizer: detected memory leaks Direct leak of 40 byte(s) in 1 object(s) allocated from: #0 0x7f00aec57917 in __interceptor_calloc (/lib64/libasan.so.6+0xb4917) #1 0x7f00ada0d7b5 in g_malloc0 (/lib64/libglib-2.0.so.0+0x517b5) espressif#2 0x5648ffd38bac in vhost_scsi_start ../hw/scsi/vhost-scsi.c:92 espressif#3 0x5648ffd38d52 in vhost_scsi_set_status ../hw/scsi/vhost-scsi.c:131 espressif#4 0x5648ffda340e in virtio_set_status ../hw/virtio/virtio.c:2036 espressif#5 0x5648ff8de281 in virtio_ioport_write ../hw/virtio/virtio-pci.c:431 espressif#6 0x5648ff8deb29 in virtio_pci_config_write ../hw/virtio/virtio-pci.c:576 espressif#7 0x5648ffe5c0c2 in memory_region_write_accessor ../softmmu/memory.c:493 espressif#8 0x5648ffe5c424 in access_with_adjusted_size ../softmmu/memory.c:555 espressif#9 0x5648ffe6428f in memory_region_dispatch_write ../softmmu/memory.c:1515 espressif#10 0x5648ffe8613d in flatview_write_continue ../softmmu/physmem.c:2825 espressif#11 0x5648ffe86490 in flatview_write ../softmmu/physmem.c:2867 espressif#12 0x5648ffe86d9f in address_space_write ../softmmu/physmem.c:2963 espressif#13 0x5648ffe86e57 in address_space_rw ../softmmu/physmem.c:2973 espressif#14 0x5648fffbfb3d in kvm_handle_io ../accel/kvm/kvm-all.c:2639 espressif#15 0x5648fffc0e0d in kvm_cpu_exec ../accel/kvm/kvm-all.c:2890 espressif#16 0x5648fffc90a7 in kvm_vcpu_thread_fn ../accel/kvm/kvm-accel-ops.c:51 espressif#17 0x56490042400a in qemu_thread_start ../util/qemu-thread-posix.c:505 espressif#18 0x7f00ac3b6ea4 in start_thread (/lib64/libpthread.so.0+0x7ea4) Free the vsc->inflight at the 'stop' path. Fixes: b82526c ("vhost-scsi: support inflight io track") Cc: Joe Jin <joe.jin@oracle.com> Cc: Li Feng <fengli@smartx.com> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> Message-Id: <20230104160433.21353-1-dongli.zhang@oracle.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Fixes this tsan crash, easy to reproduce with any large enough program: $ tests/unit/test-qht 1..2 ThreadSanitizer: CHECK failed: sanitizer_deadlock_detector.h:67 "((n_all_locks_)) < (((sizeof(all_locks_with_contexts_)/sizeof((all_locks_with_contexts_)[0]))))" (0x40, 0x40) (tid=1821568) #0 __tsan::CheckUnwind() ../../../../src/libsanitizer/tsan/tsan_rtl.cpp:353 (libtsan.so.2+0x90034) #1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86 (libtsan.so.2+0xca555) espressif#2 __sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::addLock(unsigned long, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:67 (libtsan.so.2+0xb3616) espressif#3 __sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::addLock(unsigned long, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:59 (libtsan.so.2+0xb3616) espressif#4 __sanitizer::DeadlockDetector<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::onLockAfter(__sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >*, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:216 (libtsan.so.2+0xb3616) espressif#5 __sanitizer::DD::MutexAfterLock(__sanitizer::DDCallback*, __sanitizer::DDMutex*, bool, bool) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector1.cpp:169 (libtsan.so.2+0xb3616) espressif#6 __tsan::MutexPostLock(__tsan::ThreadState*, unsigned long, unsigned long, unsigned int, int) ../../../../src/libsanitizer/tsan/tsan_rtl_mutex.cpp:200 (libtsan.so.2+0xa3382) espressif#7 __tsan_mutex_post_lock ../../../../src/libsanitizer/tsan/tsan_interface_ann.cpp:384 (libtsan.so.2+0x76bc3) espressif#8 qemu_spin_lock /home/cota/src/qemu/include/qemu/thread.h:259 (test-qht+0x44a97) espressif#9 qht_map_lock_buckets ../util/qht.c:253 (test-qht+0x44a97) espressif#10 do_qht_iter ../util/qht.c:809 (test-qht+0x45f33) espressif#11 qht_iter ../util/qht.c:821 (test-qht+0x45f33) espressif#12 iter_check ../tests/unit/test-qht.c:121 (test-qht+0xe473) espressif#13 qht_do_test ../tests/unit/test-qht.c:202 (test-qht+0xe473) espressif#14 qht_test ../tests/unit/test-qht.c:240 (test-qht+0xe7c1) espressif#15 test_default ../tests/unit/test-qht.c:246 (test-qht+0xe828) espressif#16 <null> <null> (libglib-2.0.so.0+0x7daed) espressif#17 <null> <null> (libglib-2.0.so.0+0x7d80a) espressif#18 <null> <null> (libglib-2.0.so.0+0x7d80a) espressif#19 g_test_run_suite <null> (libglib-2.0.so.0+0x7dfe9) espressif#20 g_test_run <null> (libglib-2.0.so.0+0x7e055) espressif#21 main ../tests/unit/test-qht.c:259 (test-qht+0xd2c6) espressif#22 __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 (libc.so.6+0x29d8f) espressif#23 __libc_start_main_impl ../csu/libc-start.c:392 (libc.so.6+0x29e3f) espressif#24 _start <null> (test-qht+0xdb44) Signed-off-by: Emilio Cota <cota@braap.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20230111151628.320011-5-cota@braap.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20230124180127.1881110-30-alex.bennee@linaro.org>
According to the gd25q32 datasheet -
[http://www.elm-tech.com/en/products/spi-flash-memory/gd25q32/gd25q32.pdf](page 22)
The 0xdd DIO read command needs an extra byte (m0-m7) to allow continuous read mode. It's the same for winbond but I haven't checked any other devices. The idf does send this byte (or at least 4 bits of it - see SPI_FLASH_DIO_ADDR_BITLEN in components/spi_flash/private_include/spi_flash_defs.h) so DIO read won't work on the emulator without discarding it.
This fixes a problem I was having with the nvs_get flash read functions not working on the emulator but I don't know if it fixes all DIO mode problems.