On Thu, Oct 29, 2020 at 10:02:37AM +0100, Philippe Mathieu-Daudé wrote: > On 10/28/20 7:24 PM, Philippe Mathieu-Daudé wrote: > > On 10/28/20 4:16 PM, Stefan Hajnoczi wrote: > >> On Tue, Oct 27, 2020 at 02:55:35PM +0100, Philippe Mathieu-Daudé wrote: > >>> Rename Submission Queue flags with 'Sq' and introduce > >>> Completion Queue flag definitions. > >>> > >>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> > >>> --- > >>> include/block/nvme.h | 17 +++++++++++------ > >>> 1 file changed, 11 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/include/block/nvme.h b/include/block/nvme.h > >>> index 65e68a82c89..079f884a2d3 100644 > >>> --- a/include/block/nvme.h > >>> +++ b/include/block/nvme.h > >>> @@ -491,6 +491,11 @@ typedef struct QEMU_PACKED NvmeCreateCq { > >>> #define NVME_CQ_FLAGS_PC(cq_flags) (cq_flags & 0x1) > >>> #define NVME_CQ_FLAGS_IEN(cq_flags) ((cq_flags >> 1) & 0x1) > >>> > >>> +enum NvmeFlagsCq { > >>> + NVME_CQ_PC = 1, > >>> + NVME_CQ_IEN = 2, > >>> +}; > >>> + > >>> typedef struct QEMU_PACKED NvmeCreateSq { > >>> uint8_t opcode; > >>> uint8_t flags; > >>> @@ -508,12 +513,12 @@ typedef struct QEMU_PACKED NvmeCreateSq { > >>> #define NVME_SQ_FLAGS_PC(sq_flags) (sq_flags & 0x1) > >>> #define NVME_SQ_FLAGS_QPRIO(sq_flags) ((sq_flags >> 1) & 0x3) > >>> > >>> -enum NvmeQueueFlags { > >>> - NVME_Q_PC = 1, > >>> - NVME_Q_PRIO_URGENT = 0, > >>> - NVME_Q_PRIO_HIGH = 1, > >>> - NVME_Q_PRIO_NORMAL = 2, > >>> - NVME_Q_PRIO_LOW = 3, > >>> +enum NvmeFlagsSq { > >>> + NVME_SQ_PC = 1, > >>> + NVME_SQ_PRIO_URGENT = 0, > >>> + NVME_SQ_PRIO_HIGH = 1, > >>> + NVME_SQ_PRIO_NORMAL = 2, > >>> + NVME_SQ_PRIO_LOW = 3, > >>> }; > >> > >> There is also: > >> > >> #define NVME_SQ_FLAGS_PC(sq_flags) (sq_flags & 0x1) > >> #define NVME_SQ_FLAGS_QPRIO(sq_flags) ((sq_flags >> 1) & 0x3) > >> > >> These macros should use the new constants. > > SQ_PC is bit#0, NVME_SQ_PC is "bit SQ_PC set (PC enabled)", > SQ_PRIO are bits #1-2 (shift by 1, mask 2 bits), > NVME_SQ_PRIO_xxx is the enum of these 2 bits. > > The NVME_SQ_FLAGS_X() macros extract the flags. > > So the macros can not use the new constants.
I'm not sure I understand. Does this mean the header only defines the flag values but not the bit shift constants? It seems like hw/block/nvme.c and block/nvme.c are expressing flags in slightly different approaches. Can they be unified instead of introducing hw/block/nvme.c- and block/nvme.c-only constants in the shared header file? Stefan
signature.asc
Description: PGP signature