Re: [Qemu-devel] [PATCH v6 01/17] pci: fix unaligned access in pci_xxx_quad()

[Dmitry Fleytman]

> On 30 May 2016, at 18:11 PM, Michael S. Tsirkin <address@hidden> wrote:
> 
> On Mon, May 30, 2016 at 06:05:57PM +0300, Dmitry Fleytman wrote:
>> 
>>> On 30 May 2016, at 17:47 PM, Michael S. Tsirkin <address@hidden> wrote:
>>> 
>>> On Mon, May 30, 2016 at 12:14:26PM +0300, Leonid Bloch wrote:
>>>> From: Dmitry Fleytman <address@hidden>
>>>> 
>>>> Replace legacy cpu_to_le64w()/le64_to_cpup()
>>>> calls with stq_le_p()/ldq_le_p().
>>>> 
>>>> Signed-off-by: Dmitry Fleytman <address@hidden>
>>>> Signed-off-by: Leonid Bloch <address@hidden>
>>> 
>> 
>> Hi Michael,
>> 
>>> Could you please add a code comment to clarify what's going on a bit more?
>>> Something to the point that capabilities are guaranteed to
>>> be dword-aligned only.
>>> 
>> 
>> Just to clarify, do you want to add these comments to 
>> pci_set/get_quad functions or to the new DSN-generation function?
> 
> pci_set/get_quad
> 
>>> Also, this isn't a dependency of this patchset I think -
>>> as far as I could say the only user of this is
>>>     pcie: Introduce function for DSN capability creation
>>> but that merely accesses a capability, and all callers pass in
>>> an aligned offset.
>> 
>> Right, this issue appeared after introduction of DSN generation function.
> 
> Does DSN generation function pass unaligned offsets?
> It does not look like it does…

It does according to clang sanitiser.

> 
>> All other callers pass aligned offsets so far.
>> 
>> Thanks,
>> Dmitry
>> 
>>> 
>>>> ---
>>>> include/hw/pci/pci.h | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>>> index ef6ba51..ee238ad 100644
>>>> --- 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);
>>>> }
>>>> 
>>>> static inline void
>>>> -- 
>>>> 2.5.5