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é <[email protected]> > ... > >>> 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
