Re: [PATCH v2 3/5] hw/nvme: add NVMe Admin Security SPDM support

[Wilfred Mallawa <wilfred.mallawa@wdc.com>]

On Fri, 2025-08-29 at 22:17 -0600, Keith Busch wrote:
> On Thu, Aug 28, 2025 at 05:25:31PM +1000, Wilfred Mallawa wrote:
> > +static uint16_t nvme_sec_prot_spdm_send(NvmeCtrl *n, NvmeRequest
> > *req)
> > +{
> > +    StorageSpdmTransportHeader hdr = {0};
> > +    g_autofree uint8_t *sec_buf = NULL;
> > +    uint32_t transfer_len = le32_to_cpu(req->cmd.cdw11);
> > +    uint32_t transport_transfer_len = transfer_len;
> > +    uint32_t dw10 = le32_to_cpu(req->cmd.cdw10);
> > +    uint32_t recvd;
> > +    uint16_t nvme_cmd_status, ret;
> > +    uint8_t secp = extract32(dw10, 24, 8);
> > +    uint8_t spsp1 = extract32(dw10, 16, 8);
> > +    uint8_t spsp0 = extract32(dw10, 8, 8);
> > +    bool spdm_res;
> > +
> > +    transport_transfer_len += sizeof(hdr);
> > +    if (transport_transfer_len >
> > SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE) {
> > +        return NVME_NO_COMPLETE | NVME_DNR;
> > +    }
> > +
> > +    /* Generate the NVMe transport header */
> > +    hdr.security_protocol = secp;
> > +    hdr.security_protocol_specific = cpu_to_le16((spsp1 << 8) |
> > spsp0);
> > +    hdr.length = cpu_to_le32(transfer_len);
> > +
> > +    sec_buf = g_malloc0(transport_transfer_len);
> > +    if (!sec_buf) {
> > +        return NVME_NO_COMPLETE | NVME_DNR;
> > +    }
> > +
> > +    /* Attach the transport header */
> > +    memcpy(sec_buf, &hdr, sizeof(hdr));
> > +    ret = nvme_h2c(n, sec_buf + sizeof(hdr), transfer_len, req);
> > +    if (ret) {
> > +        return NVME_NO_COMPLETE | NVME_DNR;
> 
> The "NO_COMPLETE" is a special -1 value so or'ing it with anything
> doesn't make sense.
> 
> But more importantly, what's supposed to complete it? This is
> supposed
> to be used as a return code when you have an async context to
> complete
> it later, and there doesn't appear to be one here.
Okay I totally misinterpreted it's use-case, and yeah there's no async
context here. I will update the error return values for V3. Thanks for
the feedback!

Regards,
Wilfred