[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL V3 00/20] Net patches

From: Jason Wang
Subject: Re: [Qemu-devel] [PULL V3 00/20] Net patches
Date: Mon, 30 May 2016 09:51:39 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0

On 2016年05月30日 00:45, Peter Maydell wrote:
On 29 May 2016 at 16:22, Dmitry Fleytman <address@hidden> 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 then?
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.

-- PMM

git grep shows lots of places. Is it ok to send a new version of pull request with Dmitry's fix first?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]