Re: [PATCH v4 0/2] Add OCP extended log to nvme QEMU

[Joel Granados]

ping.

Is the solution to the guid constant ok?

Best

On Fri, Nov 25, 2022 at 10:48:06AM +0100, Joel Granados wrote:
> The motivation and description are contained in the last patch in this set.
> Will copy paste it here for convenience:
> 
>     In order to evaluate write amplification factor (WAF) within the storage
>     stack it is important to know the number of bytes written to the
>     controller. The existing SMART log value of Data Units Written is too
>     coarse (given in units of 500 Kb) and so we add the SMART health
>     information extended from the OCP specification (given in units of bytes).
> 
>     To accommodate different vendor specific specifications like OCP, we add a
>     multiplexing function (nvme_vendor_specific_log) which will route to the
>     different log functions based on arguments and log ids. We only return the
>     OCP extended smart log when the command is 0xC0 and ocp has been turned on
>     in the args.
> 
>     Though we add the whole nvme smart log extended structure, we only 
> populate
>     the physical_media_units_{read,written}, log_page_version and
>     log_page_uuid.
> 
> V4 changes:
> 1. Fixed cpu_to_le64 instead of cpu_to_le32
> 2. Variable naming : uuid -> guid
> 3. Changed how the guid value appears in the code:
>    Used to be:
>         smart_l.log_page_uuid[0] = 0xA4F2BFEA2810AFC5;
>         smart_l.log_page_uuid[1] = 0xAFD514C97C6F4F9C;
> 
>    Now is:
>         static const uint8_t guid[16] = {
>             0xC5, 0xAF, 0x10, 0x28, 0xEA, 0xBF, 0xF2, 0xA4,
>             0x9C, 0x4F, 0x6F, 0x7C, 0xC9, 0x14, 0xD5, 0xAF
>         };
> 
>    This is different from what @klaus suggested because I want to keep it
>    consistent to what nvme-cli currently implements. I think here we can
>    either change both nvme-cli and this patch or leave the order of the
>    bytes as they are here. This all depends on how you interpret the Spec
>    (which is ambiguous)
> 
> V3 changes:
> 1. Corrected a bunch of checkpatch issues. Since I changed the first patch
>    I did not include the reviewed-by.
> 2. Included some documentation in nvme.rst for the ocp argument
> 3. Squashed the ocp arg changes into the main patch.
> 4. Fixed several comments and an open parenthesis
> 5. Hex values are now in lower case.
> 6. Change the reserved format to rsvd<BYTEOFFSET>
> 7. Made sure that NvmeCtrl is the first arg in all the functions.
> 8. Fixed comment on commit of main patch
> 
> V2 changes:
> 1. I moved the ocp parameter from the namespace to the subsystem as it is
>    defined there in the OCP specification
> 2. I now accumulate statistics from all namespaces and report them back on
>    the extended log as per the spec.
> 3. I removed the default case in the switch in nvme_vendor_specific_log as
>    it does not have any special function.
> 
> Joel Granados (2):
>   nvme: Move adjustment of data_units{read,written}
>   nvme: Add physical writes/reads from OCP log
> 
>  docs/system/devices/nvme.rst |  7 ++++
>  hw/nvme/ctrl.c               | 73 +++++++++++++++++++++++++++++++++---
>  hw/nvme/nvme.h               |  1 +
>  include/block/nvme.h         | 36 ++++++++++++++++++
>  4 files changed, 111 insertions(+), 6 deletions(-)
> 
> -- 
> 2.30.2
>

signature.asc
Description: PGP signature