On Nov 3 21:37, Philippe Mathieu-Daudé wrote: > On 11/3/20 8:48 PM, Dmitry Fomichev wrote: > >> -----Original Message----- > >> From: Philippe Mathieu-Daudé <phi...@redhat.com> > ... > >>> typedef struct QEMU_PACKED NvmeCqe { > >>> - uint32_t result; > >>> - uint32_t rsvd; > >>> + union { > >>> + uint64_t result64; > >>> + uint32_t result32; > >>> + }; > >> > >> When using packed structure you want to define all fields to > >> avoid alignment confusion (and I'm surprised the compiler doesn't > >> complain...). So this would be: > >> > >> union { > >> uint64_t result64; > >> struct { > >> uint32_t result32; > >> uint32_t rsvd32; > >> }; > >> }; > >>
I align (hehe...) towards this. The amount of bit-juggling we need for commands justify the need for separate NvmeCmd's, but in this case I think an NvmeCqeZA is just unnecessary clutter. If the result value is complex, the approach used by AERs is better I think: typedef struct QEMU_PACKED NvmeAerResult { uint8_t event_type; uint8_t event_info; uint8_t log_page; uint8_t resv; } NvmeAerResult; NvmeAerResult *result = (NvmeAerResult *)&req->cqe.result32; Since storing the Zone Append ALBA in the result64 isn't really a complex operation, let's just assign it into that member directly. (Addendum) That DW1 is command specific and no longer reserved is defined by TP 4056 (Namespace Types) - not v1.4 or any of its revisions.
signature.asc
Description: PGP signature