On Fri, Sep 16, 2016 at 11:57:54AM +0100, Peter Maydell wrote:
> On 15 September 2016 at 21:38, Michael S. Tsirkin <m...@redhat.com> wrote:
> > The following changes since commit d1eb8f2acba579830cf3798c3c15ce51be852c56:
> >
> > fpu: add mechanism to check for invalid long double formats (2016-09-15
> > 12:43:18 +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 a5a43875b352810d29dc27e7b0fb602eb7ef2d31:
> >
> > MAINTAINERS: add virtio-* tests (2016-09-15 23:37:16 +0300)
> >
> > ----------------------------------------------------------------
> > virtio,pci: fixes and updates
> >
> > AMD IOMMU emulation
> > virtio feature negotiation rework
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> >
> > ----------------------------------------------------------------
>
> Fails to build on ppc64be, still:
> /home/pm215/qemu/hw/i386/amd_iommu.c:245:5: error: expected ‘,’, ‘;’
> or ‘}’ before ‘uint32_t’
> uint32_t reserved_3:29;
> ^
> /home/pm215/qemu/hw/i386/amd_iommu.c: In function ‘amdvi_complete_ppr’:
> /home/pm215/qemu/hw/i386/amd_iommu.c:588:62: error: ‘CMDCompletePPR’
> has no member named ‘reserved_3’
> if (pprcomp->reserved_1 || pprcomp->reserved_2 || pprcomp->reserved_3 ||
> ^
> /home/pm215/qemu/hw/i386/amd_iommu.c:589:16: error: ‘CMDCompletePPR’
> has no member named ‘reserved_4’
> pprcomp->reserved_4 || pprcomp->reserved_5) {
> ^
> /home/pm215/qemu/hw/i386/amd_iommu.c:589:39: error: ‘CMDCompletePPR’
> has no member named ‘reserved_5’
> pprcomp->reserved_4 || pprcomp->reserved_5) {
> ^
>
> Missing semicolon, again, line 237.
>
> In fact looking at this code it just looks broken. Structures
> like this:
>
> typedef struct QEMU_PACKED {
> #ifdef HOST_WORDS_BIGENDIAN
> uint64_t type:4; /* command type */
> uint64_t reserved_1:44;
> uint64_t devid:16; /* related devid */
> #else
> uint64_t devid:16;
> uint64_t reserved_1:44;
> uint64_t type:4;
> #endif /* __BIG_ENDIAN_BITFIELD */
> uint64_t reserved_2;
> } CMDInvalIntrTable;
>
> seem to be trying to represent bit layouts in memory using
> bitfields, but this is just not portable. It's not sufficient
> to have a "bigendian vs littleendian" set of ifdefs.
>
> The portable way to do this is to write the code to use
> bitwise logical operations (and functions like extract64
> and deposit64) to manipulate things. As a bonus you get rid
> of all these host-specific #ifdefs that are tripping you
> up now.
>
> It would be nice if C bitfields worked the way this code
> wants them to, but they don't, alas.
>
> thanks
> -- PMM
I agree, I wanted to rework this in tree but maybe best to fix it first.
I dropped all this code from tree for now.
--
MST
