Re: [Qemu-devel] [PULL v2 00/28] pci, pc, virtio: fixes, features

[Singh, Brijesh]

On 10/25/2018 07:59 PM, Michael S. Tsirkin wrote:
> On Thu, Oct 25, 2018 at 08:16:44PM +0100, Peter Maydell wrote:
>> On 25 October 2018 at 01:52, Michael S. Tsirkin <address@hidden> wrote:
>>> The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
>>>
>>>    Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' 
>>> into staging (2018-10-23 17:20:23 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>    git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>>>
>>> for you to fetch changes up to 6a9fb4e1ba5594cde7739068617ad88e6117db93:
>>>
>>>    vhost-scsi: prevent using uninitialized vqs (2018-10-24 20:50:13 -0400)
>>>
>>> ----------------------------------------------------------------
>>> pci, pc, virtio: fixes, features
>>>
>>> AMD IOMMU VAPIC support + fixes all over the place.
>>>
>>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>>
>> Hi; I get some compile failures and a test assertion, I'm afraid:
>>
>> On 32-bit hosts (where uint64_t and size_t are not the same):
>>
>> /home/peter.maydell/qemu/include/qemu/compiler.h:80:35: error: invalid
>> operands to binary - (have 'uint64_t * {aka long long unsigned int *}'
>> and 'size_t * {aka unsigned int *}')
>>   #define type_check(t1,t2) ((t1*)0 - (t2*)0)
>>                                     ^
>> /home/peter.maydell/qemu/include/hw/qdev-properties.h:77:15: note: in
>> expansion of macro 'type_check'
>>               + type_check(_type, typeof_field(_state, _field)),          \
>>                 ^
>> /home/peter.maydell/qemu/include/hw/qdev-properties.h:168:5: note: in
>> expansion of macro 'DEFINE_PROP_UNSIGNED'
>>       DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
>>       ^
>> /home/peter.maydell/qemu/hw/misc/pci-testdev.c:322:5: note: in
>> expansion of macro 'DEFINE_PROP_SIZE'
>>       DEFINE_PROP_SIZE("membar", PCITestDevState, membar_size, 0),
>>       ^
>> /home/peter.maydell/qemu/rules.mak:69: recipe for target
>> 'hw/misc/pci-testdev.o' failed
>>
>> On the Windows w64 cross-compile:
>>
>> In file included from 
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:26:0:
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c: In function
>> 'amdvi_int_remap_msi':
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:247:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_NMI_PASS_MASK         (1UL << 58)
>>                                                ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1281:25: note: in
>> expansion of macro 'AMDVI_DEV_NMI_PASS_MASK'
>>           pass = dte[3] & AMDVI_DEV_NMI_PASS_MASK;
>>                           ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:245:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_INT_PASS_MASK         (1UL << 56)
>>                                                ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1285:25: note: in
>> expansion of macro 'AMDVI_DEV_INT_PASS_MASK'
>>           pass = dte[3] & AMDVI_DEV_INT_PASS_MASK;
>>                           ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.h:246:46: error: left
>> shift count >= width of type [-Werror=shift-count-overflow]
>>   #define AMDVI_DEV_EINT_PASS_MASK        (1UL << 57)
>>                                                ^
>> /home/petmay01/qemu-for-merges/hw/i386/amd_iommu.c:1289:25: note: in
>> expansion of macro 'AMDVI_DEV_EINT_PASS_MASK'
>>           pass = dte[3] & AMDVI_DEV_EINT_PASS_MASK;
>>                           ^
>>
>> These should presumably all be "ULL". (The "UL" suffix is
>> usually a bug, as it's either unnecessary or should be ULL.)
> 
> Yea.  Fixed. Brijesh could you start cleaning up that header generally?
> It has all kind of weird code like using bitfields for hardware
> accesses. That isn't portable - switch to full dword fields with shift
> and | to operate them and proper cpu_to_le APIs or similar please.
> 


Noted, I will look into cleaning up this and send patches for reviews.

thanks