> > On Mon, Jul 09, 2018 at 04:48:46AM -0400, Frediano Ziglio wrote: > > > > > > On Tue, Jul 03, 2018 at 11:38:56AM -0400, Frediano Ziglio wrote: > > > > > > > > > > On Thu, Jun 28, 2018 at 09:43:47PM +0100, Frediano Ziglio wrote: > > > > > > This avoids compilers detect misaligned access. > > > > > > > > > > Maybe "This helps some compilers to realize QXLRam is correctly > > > > > aligned"? > > > > > > > > > > > > > Well, the current code tells the compiler that this structure > > > > is NOT aligned. > > > > But we actually know the opposite > > > > > > Well, we know that in QEMU and Xspice, this is correctly aligned (but > > > one need to look at how ram offset is computed in order to see that), so > > > we add the annotation to spice-protocol. Since this is quite odd, the > > > commit log would indeed need to be quite descriptive. > > > > > > Christophe > > > > > > > Suggestions? > > I would rephrase like this: > > > This avoids compilers detect misaligned access. > > Without such a hint, some compilers (clang) will generate warnings for trying > to > do atomic operations on misaligned memory ? > Actually for a bug code does not link as is referring to a undefined function. In Qemu an atomic operation is performed on int_pending which causes the problem. Atomic operator and unaligned access does not play well together although x86/x64 make an exceptions (beside potentially taking literally hundreds of CPU ticks). Note that QXL (in this case I'm talking about virtual QXL card, not SPICE interface) is provided only in x86/x64 so this would be a real issue only if you are emulating the CPU on a no x86/x64 system. OT: about endianness and QXL doing some minor portability attempts I realized that commands to QXL interface can arrive either in host encoding or little endian so for instance if you need to run a x64 machine on a big endian host machine you could receive for the same VM command in 2 different endianness! Nothing that we support at the moment, mainly a curiosity. > > The structure is allocated at the beginning of a page so surely > > will be 4 bytes aligned. > > In QEMU and Xspice, the offset of the structure will be rounded to a 4kB > boundary, so the structure will end up 4 bytes aligned as well with > these users. We are not aware of other users of QXLRam, but if there > was, it's likely the struct would be at least naturally aligned, so this > change should not impact anyone. > Not sure if Xspice aligns this structure to 4kB. But in QXLRam there are some fields that potentially are accessed by 2 thread which could be in theory an issue if not aligned. For instance incrementing 255 from another thread you could get a 0. > Aligning to 4 bytes maintains the size of the structure unchanged > avoiding possible ABI changes. > I assume I should copy these last 2 sentences in the commit message. > Christophe > Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel