[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 13/25] block/nvme: Introduce Completion Queue definitions
|
From: |
Philippe Mathieu-Daudé |
|
Subject: |
Re: [PATCH 13/25] block/nvme: Introduce Completion Queue definitions |
|
Date: |
Fri, 30 Oct 2020 15:51:30 +0100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 |
On 10/30/20 12:46 PM, Stefan Hajnoczi wrote:
> 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é <philmd@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?
Yes.
>
> 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?
I suggested the hw/block/nvme.c to unify the style.
Klaus agreed (at least to have a look). Any change will
be for 6.0 anyway.
- [PATCH 12/25] block/nvme: Make nvme_init_queue() return boolean indicating error, (continued)
[PATCH 14/25] block/nvme: Use definitions instead of magic values in add_io_queue(), Philippe Mathieu-Daudé, 2020/10/27
[PATCH 16/25] block/nvme: Simplify ADMIN queue access, Philippe Mathieu-Daudé, 2020/10/27
[PATCH 17/25] block/nvme: Simplify nvme_cmd_sync(), Philippe Mathieu-Daudé, 2020/10/27