On 7/14/21 12:12 AM, Philippe Mathieu-Daudé wrote:
> On 7/13/21 9:24 PM, Klaus Jensen wrote:
>> From: Klaus Jensen <[email protected]>
>>
>> Add the NvmeBarRegs enum and use these instead of explicit register
>> offsets.
>>
>> Signed-off-by: Klaus Jensen <[email protected]>
>> ---
>> 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?
BTW I'm not suggesting this is better, I just wonder how we can avoid
to duplicate the definitions. Alternative is declaring:
enum NvmeBarRegs {
NVME_REG_CAP = offsetof(NvmeBar, cap),
NVME_REG_VS = offsetof(NvmeBar, vs),
...
Or keeping your patch.