[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/block/nvme: Align I/O BAR to 4 KiB
From: |
Klaus Jensen |
Subject: |
Re: [PATCH] hw/block/nvme: Align I/O BAR to 4 KiB |
Date: |
Tue, 30 Jun 2020 10:46:45 +0200 |
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? :)