From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49218) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7CMw-0007El-4O for qemu-devel@nongnu.org; Sun, 29 May 2016 21:51:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b7CMs-0002yc-Vi for qemu-devel@nongnu.org; Sun, 29 May 2016 21:51:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52988) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7CMs-0002yI-QG for qemu-devel@nongnu.org; Sun, 29 May 2016 21:51:46 -0400 References: <1464228995-26657-1-git-send-email-jasowang@redhat.com> <5747C08C.9020104@redhat.com> <540CF146-518B-40BA-86A3-5A7B1D89E6B4@daynix.com> From: Jason Wang Message-ID: <574B9CAB.3020001@redhat.com> Date: Mon, 30 May 2016 09:51:39 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PULL V3 00/20] Net patches List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , Dmitry Fleytman Cc: Leonid Bloch , QEMU Developers On 2016=E5=B9=B405=E6=9C=8830=E6=97=A5 00:45, Peter Maydell wrote: > On 29 May 2016 at 16:22, Dmitry Fleytman wrote: >> It turned out that the issue is not in the new code but in >> pci.h helper functions used by the new code to fill DSN capability. > Thanks for tracking this down. > >> Following patch fixes the problem: >> >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index ef6ba51..ee238ad >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -468,13 +468,13 @@ pci_get_long(const uint8_t *config) >> static inline void >> pci_set_quad(uint8_t *config, uint64_t val) >> { >> - cpu_to_le64w((uint64_t *)config, val); >> + stq_le_p(config, val); >> } >> >> static inline uint64_t >> pci_get_quad(const uint8_t *config) >> { >> - return le64_to_cpup((const uint64_t *)config); >> + return ldq_le_p(config); >> } >> >> I see from git blame that some time ago you did similar change in all = other >> pci_set_* pci_get_* functions except these two. >> >> Is there any specific reason you did not change these two functions th= en? > The patches where I changed the other functions were cleanups > to convert away from some legacy *_to_cpupu() functions, which > were specifically intended to work with unaligned pointers > (which is what the "u" indicated). I was doing the cleanups > per-conversion-function, not per-callsite, and since these > two functions aren't using a _to_cpupu() function but just > _to_cpup() they wouldn't have shown up in my searches. > > In any case this is the correct fix, and we should probably > audit the other uses of *_to_cpup and cpu_to_* -- I suspect > many of them are working with possibly-unaligned > pointers in to guest memory and we should replace them with > ld*_*_p functions and get rid of the _to_cpup functions entirely. > > thanks > -- PMM git grep shows lots of places. Is it ok to send a new version of pull=20 request with Dmitry's fix first? Thanks