Re: [PATCH] hw/block/nvme: Align I/O BAR to 4 KiB

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

On 6/30/20 10:46 AM, Klaus Jensen wrote:
> On Jun 30 10:35, Philippe Mathieu-Daudé wrote:
>> Hi Klaus,
>>
>> On 6/25/20 8:23 PM, Klaus Jensen wrote:
>>> On Jun 25 17:48, Philippe Mathieu-Daudé wrote:
>>>> Simplify the NVMe emulated device by aligning the I/O BAR to 4 KiB.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>>  include/block/nvme.h | 3 +++
>>>>  hw/block/nvme.c      | 5 ++---
>>>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/block/nvme.h b/include/block/nvme.h
>>>> index 1720ee1d51..6d87c9c146 100644
>>>> --- a/include/block/nvme.h
>>>> +++ b/include/block/nvme.h
>>>> @@ -22,6 +22,8 @@ typedef struct NvmeBar {
>>>>      uint32_t    pmrebs;
>>>>      uint32_t    pmrswtp;
>>>>      uint32_t    pmrmsc;
>>>> +    uint32_t    reserved[58];
>>>> +    uint8_t     cmd_set_specfic[0x100];
>>>>  } NvmeBar;
>>>
>>> This ends up as a freak mix of v1.3 and v1.4 specs. Since we already
>>> have the PMR stuff in there, I think it makes more sense to align with
>>> v1.4 and remove the reserved bytes.
>>
>> I'm sorry but I don't understand what you'd prefer, removing the
>> cmd_set_specfic[] for v1.3 and instead use this?
>>
>>       uint32_t    pmrmsc;
>>  +    uint32_t    reserved[122];
>>   } NvmeBar;
>>
>> Or this?
>>
>>       uint32_t    pmrmsc;
>>  +    uint8_t     reserved[488];
>>   } NvmeBar;
>>
> 
> Yes, the second one.
> 
> But it should be 484 bytes reserved and the bug is in the pmrmsc field
> that should be uint64_t. Can you fix that as well?  :)
> 

Ah this is what you did in "hw/block/nvme: add NVMe 1.4 specific fields"
https://www.mail-archive.com/qemu-devel@nongnu.org/msg717891.html