Re: [PATCH v2 2/5] hw/nvme: use symbolic names for registers

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 7/13/21 9:24 PM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Add the NvmeBarRegs enum and use these instead of explicit register
> offsets.
> 
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  include/block/nvme.h | 27 +++++++++++++++++++++++++++
>  hw/nvme/ctrl.c       | 44 ++++++++++++++++++++++----------------------
>  2 files changed, 49 insertions(+), 22 deletions(-)
> 
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 84053b68b987..082d4bddbf9f 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -31,6 +31,33 @@ typedef struct QEMU_PACKED NvmeBar {
>      uint8_t     css[484];
>  } NvmeBar;
>  
> +enum NvmeBarRegs {
> +    NVME_REG_CAP     = 0x0,
> +    NVME_REG_VS      = 0x8,
> +    NVME_REG_INTMS   = 0xc,
> +    NVME_REG_INTMC   = 0x10,
> +    NVME_REG_CC      = 0x14,
> +    NVME_REG_CSTS    = 0x1c,
> +    NVME_REG_NSSR    = 0x20,
> +    NVME_REG_AQA     = 0x24,
> +    NVME_REG_ASQ     = 0x28,
> +    NVME_REG_ACQ     = 0x30,
> +    NVME_REG_CMBLOC  = 0x38,
> +    NVME_REG_CMBSZ   = 0x3c,
> +    NVME_REG_BPINFO  = 0x40,
> +    NVME_REG_BPRSEL  = 0x44,
> +    NVME_REG_BPMBL   = 0x48,
> +    NVME_REG_CMBMSC  = 0x50,
> +    NVME_REG_CMBSTS  = 0x58,
> +    NVME_REG_PMRCAP  = 0xe00,
> +    NVME_REG_PMRCTL  = 0xe04,
> +    NVME_REG_PMRSTS  = 0xe08,
> +    NVME_REG_PMREBS  = 0xe0c,
> +    NVME_REG_PMRSWTP = 0xe10,
> +    NVME_REG_PMRMSCL = 0xe14,
> +    NVME_REG_PMRMSCU = 0xe18,
> +};
> +
>  enum NvmeCapShift {
>      CAP_MQES_SHIFT     = 0,
>      CAP_CQR_SHIFT      = 16,
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 28299c6f3764..8c305315f41c 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -5740,7 +5740,7 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>      }
>  
>      switch (offset) {
> -    case 0xc:   /* INTMS */
> +    case NVME_REG_INTMS:

What about using offsetof(NvmeBar, intms) instead?