| Message ID | 1464228995-26657-1-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 26 May 2016 at 03:16, Jason Wang <jasowang@redhat.com> wrote: > The following changes since commit 287db79df8af8e31f18e262feb5e05103a09e4d4: > > Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into staging (2016-05-24 13:06:33 +0100) > > are available in the git repository at: > > https://github.com/jasowang/qemu.git tags/net-pull-request > > for you to fetch changes up to 136796b070ddd09dd14ef73e77ae20419ba6554a: > > net/net: Add SocketReadState for reuse codes (2016-05-26 09:58:22 +0800) > > ---------------------------------------------------------------- > > Main changes: > - e1000e emulation > - convet vmxnet3 to use DMA api > Changes from V2: > - fix clang build > Changes from V1: > - fix 32bit build Hi. I'm afraid this introduces new errors in the clang sanitizer output from make check: all the check-qtest-i386 and check-qtest-x86_64 runs produce output like: /home/petmay01/linaro/qemu-for-merges/hw/pci/pcie.c:641:25: runtime error: left shift of 4092 by 20 places cannot be represented in type 'int' /home/petmay01/linaro/qemu-for-merges/hw/pci/pcie.c:642:45: runtime error: left shift of 4092 by 20 places cannot be represented in type 'int' ==14902==WARNING: Trying to symbolize code, but external symbolizer is not initialized! /home/petmay01/linaro/qemu-for-merges/include/qemu/bswap.h:120:1: runtime error: store to misaligned address 0x2b23c01e6674 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment 0x2b23c01e6674: note: pointer points here 03 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ^ The stuff about left shifts is just the usual shift-into-sign-bit which we haven't yet sorted out what we're doing with (ie whether we can ignore them and shut up the sanitizer without silencing other interesting warnings), but we shouldn't be doing misaligned stores of 64-bit values. Apologies for the lack of any backtraces in the output, but this is almost certainly the result of trying to do le64_to_cpu() or cpu_to_le64() on a buffer which isn't necessarily aligned (usually some pointer into guest memory). Use the functions ldq_le_p() and stq_le_p() instead, which will handle a potentially misaligned pointer for you. (There are similar functions for other access widths too.) thanks -- PMM
On 05/26/2016 09:08 AM, Peter Maydell wrote: > > Apologies for the lack of any backtraces in the output, but > this is almost certainly the result of trying to do le64_to_cpu() > or cpu_to_le64() on a buffer which isn't necessarily aligned > (usually some pointer into guest memory). Use the functions > ldq_le_p() and stq_le_p() instead, which will handle a > potentially misaligned pointer for you. (There are similar > functions for other access widths too.) Since these functions are constructed by ## token pasting, it's very hard to grep .h files to see what variations-on-a-theme are actually available, nor is the documentation clear on what they all do (in-place vs. copy, pass a value vs. a pointer, ...). It might be a nice bite-sized task to beef up the documentation and at least call out ALL of the endian-conversion functions in a nice comment (to make it greppable), along with this tidbit of information on which forms are optimized but require alignment, vs. work anywhere but potentially slower.
On 26 May 2016 at 16:20, Eric Blake <eblake@redhat.com> wrote: > On 05/26/2016 09:08 AM, Peter Maydell wrote: >> >> Apologies for the lack of any backtraces in the output, but >> this is almost certainly the result of trying to do le64_to_cpu() >> or cpu_to_le64() on a buffer which isn't necessarily aligned >> (usually some pointer into guest memory). Use the functions >> ldq_le_p() and stq_le_p() instead, which will handle a >> potentially misaligned pointer for you. (There are similar >> functions for other access widths too.) > > Since these functions are constructed by ## token pasting, it's very > hard to grep .h files to see what variations-on-a-theme are actually > available, nor is the documentation clear on what they all do (in-place > vs. copy, pass a value vs. a pointer, ...). It might be a nice > bite-sized task to beef up the documentation and at least call out ALL > of the endian-conversion functions in a nice comment (to make it > greppable), along with this tidbit of information on which forms are > optimized but require alignment, vs. work anywhere but potentially slower. The documentation of the ld* functions is in bswap.h (the comment starting "the generic syntax is"), though I agree it is a bit awkward not having it greppable. There would probably be about 200 functions if you expanded out the names just in the ld*_p() family. We could probably also do a better job of marking which of these function families are recommended, and which are obsolete flavours which we have because a bunch of old code still uses them. (You can tell cpu_to_le64() and friends don't handle misalignment because they take a uint64_t, not a pointer to one.) thanks -- PMM
On 2016年05月26日 23:08, Peter Maydell wrote: > On 26 May 2016 at 03:16, Jason Wang <jasowang@redhat.com> wrote: >> The following changes since commit 287db79df8af8e31f18e262feb5e05103a09e4d4: >> >> Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into staging (2016-05-24 13:06:33 +0100) >> >> are available in the git repository at: >> >> https://github.com/jasowang/qemu.git tags/net-pull-request >> >> for you to fetch changes up to 136796b070ddd09dd14ef73e77ae20419ba6554a: >> >> net/net: Add SocketReadState for reuse codes (2016-05-26 09:58:22 +0800) >> >> ---------------------------------------------------------------- >> >> Main changes: >> - e1000e emulation >> - convet vmxnet3 to use DMA api >> Changes from V2: >> - fix clang build >> Changes from V1: >> - fix 32bit build > Hi. I'm afraid this introduces new errors in the clang sanitizer output > from make check: all the check-qtest-i386 and check-qtest-x86_64 > runs produce output like: > > /home/petmay01/linaro/qemu-for-merges/hw/pci/pcie.c:641:25: runtime > error: left shift of 4092 by 20 places cannot be > represented in type 'int' > /home/petmay01/linaro/qemu-for-merges/hw/pci/pcie.c:642:45: runtime > error: left shift of 4092 by 20 places cannot be > represented in type 'int' > ==14902==WARNING: Trying to symbolize code, but external symbolizer is > not initialized! > /home/petmay01/linaro/qemu-for-merges/include/qemu/bswap.h:120:1: > runtime error: store to misaligned address 0x2b23c01e6674 for type > 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment > 0x2b23c01e6674: note: pointer points here > 03 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 > ^ Sorry for the trouble again. Wonder the correct way to enable sanitizer, after I add "-fsanitizer=address", it produces tons of warnings and errors but don't find the above outputs. > The stuff about left shifts is just the usual shift-into-sign-bit > which we haven't yet sorted out what we're doing with (ie > whether we can ignore them and shut up the sanitizer without > silencing other interesting warnings), but we shouldn't be doing > misaligned stores of 64-bit values. I agree. > > Apologies for the lack of any backtraces in the output, but > this is almost certainly the result of trying to do le64_to_cpu() > or cpu_to_le64() on a buffer which isn't necessarily aligned > (usually some pointer into guest memory). Use the functions > ldq_le_p() and stq_le_p() instead, which will handle a > potentially misaligned pointer for you. (There are similar > functions for other access widths too.) > > thanks > -- PMM Leonid and Dmitry, please check the guest memory access as suggested above and respin the series. I will hold the pull until the new version. Thanks
On 27 May 2016 at 04:35, Jason Wang <jasowang@redhat.com> wrote: > Sorry for the trouble again. Wonder the correct way to enable sanitizer, > after I add "-fsanitizer=address", it produces tons of warnings and errors > but don't find the above outputs. I pass configure '--extra-cflags=-fsanitize=undefined' . thanks -- PMM
The following changes since commit 287db79df8af8e31f18e262feb5e05103a09e4d4: Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into staging (2016-05-24 13:06:33 +0100) are available in the git repository at: https://github.com/jasowang/qemu.git tags/net-pull-request for you to fetch changes up to 136796b070ddd09dd14ef73e77ae20419ba6554a: net/net: Add SocketReadState for reuse codes (2016-05-26 09:58:22 +0800) ---------------------------------------------------------------- Main changes: - e1000e emulation - convet vmxnet3 to use DMA api Changes from V2: - fix clang build Changes from V1: - fix 32bit build ---------------------------------------------------------------- Dmitry Fleytman (14): msix: make msix_clr_pending() visible for clients pci: Introduce define for PM capability version 1.1 pcie: Add support for PCIe CAP v1 pcie: Introduce function for DSN capability creation vmxnet3: Use generic function for DSN capability definition net: Introduce Toeplitz hash calculator net: Add macros for MAC address tracing vmxnet3: Use common MAC address tracing macros net_pkt: Name vmxnet3 packet abstractions more generic rtl8139: Move more TCP definitions to common header vmxnet3: Use pci_dma_* API instead of cpu_physical_memory_* e1000_regs: Add definitions for Intel 82574-specific bits e1000: Move out code that will be reused in e1000e e1000e: Introduce qtest for e1000e device Eduardo Habkost (1): net: vl: Move default_net to vl.c Jason Wang (2): net_pkt: Extend packet abstraction as required by e1000e functionality net: Introduce e1000e device emulation Prasad J Pandit (1): net: mipsnet: check packet length against buffer Zhang Chen (1): net/net: Add SocketReadState for reuse codes Zhou Jie (1): net/tap: Allocating Large sized arrays to heap MAINTAINERS | 18 + default-configs/pci.mak | 1 + hw/net/Makefile.objs | 5 +- hw/net/e1000.c | 411 +--- hw/net/e1000_regs.h | 349 ++- hw/net/e1000e.c | 739 +++++++ hw/net/e1000e_core.c | 3478 ++++++++++++++++++++++++++++++ hw/net/e1000e_core.h | 146 ++ hw/net/e1000x_common.c | 267 +++ hw/net/e1000x_common.h | 213 ++ hw/net/mipsnet.c | 3 + hw/net/net_rx_pkt.c | 600 ++++++ hw/net/net_rx_pkt.h | 363 ++++ hw/net/{vmxnet_tx_pkt.c => net_tx_pkt.c} | 358 +-- hw/net/net_tx_pkt.h | 191 ++ hw/net/rtl8139.c | 5 - hw/net/vmxnet3.c | 155 +- hw/net/vmxnet_debug.h | 3 - hw/net/vmxnet_rx_pkt.c | 187 -- hw/net/vmxnet_rx_pkt.h | 174 -- hw/net/vmxnet_tx_pkt.h | 146 -- hw/pci/msix.c | 2 +- hw/pci/pcie.c | 94 +- include/hw/pci/msix.h | 1 + include/hw/pci/pci_regs.h | 2 + include/hw/pci/pcie.h | 5 + include/hw/pci/pcie_regs.h | 5 +- include/net/checksum.h | 49 +- include/net/eth.h | 161 +- include/net/net.h | 19 +- net/checksum.c | 7 +- net/eth.c | 410 +++- net/filter-mirror.c | 66 +- net/net.c | 93 +- net/socket.c | 77 +- net/tap.c | 6 +- tests/Makefile | 7 +- tests/e1000e-test.c | 480 +++++ trace-events | 211 ++ vl.c | 24 +- 40 files changed, 8221 insertions(+), 1310 deletions(-) create mode 100644 hw/net/e1000e.c create mode 100644 hw/net/e1000e_core.c create mode 100644 hw/net/e1000e_core.h create mode 100644 hw/net/e1000x_common.c create mode 100644 hw/net/e1000x_common.h create mode 100644 hw/net/net_rx_pkt.c create mode 100644 hw/net/net_rx_pkt.h rename hw/net/{vmxnet_tx_pkt.c => net_tx_pkt.c} (53%) create mode 100644 hw/net/net_tx_pkt.h delete mode 100644 hw/net/vmxnet_rx_pkt.c delete mode 100644 hw/net/vmxnet_rx_pkt.h delete mode 100644 hw/net/vmxnet_tx_pkt.h create mode 100644 tests/e1000e-test.c