On Thu, Aug 29, 2024 at 1:11 AM Michael S. Tsirkin <m...@redhat.com> wrote:
>
> On Thu, Aug 29, 2024 at 01:04:05AM +0600, Dorjoy Chowdhury wrote:
> > On Thu, Aug 29, 2024 at 12:28 AM Michael S. Tsirkin <m...@redhat.com> wrote:
> > >
> > > On Thu, Aug 22, 2024 at 09:08:46PM +0600, Dorjoy Chowdhury wrote:
> > > > Nitro Secure Module (NSM)[1] device is used in AWS Nitro Enclaves[2]
> > > > for stripped down TPM functionality like cryptographic attestation.
> > > > The requests to and responses from NSM device are CBOR[3] encoded.
> > > >
> > > > This commit adds support for NSM device in QEMU. Although related to
> > > > AWS Nitro Enclaves, the virito-nsm device is independent and can be
> > > > used in other machine types as well. The libcbor[4] library has been
> > > > used for the CBOR encoding and decoding functionalities.
> > > >
> > > > [1] 
> > > > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00387.html
> > > > [2] https://docs.aws.amazon.com/enclaves/latest/user/nitro-enclave.html
> > > > [3] http://cbor.io/
> > > > [4] https://libcbor.readthedocs.io/en/latest/
> > > >
> > > > Signed-off-by: Dorjoy Chowdhury <dorjoychy...@gmail.com>
> > > > ---
> > > >  MAINTAINERS                      |   10 +
> > > >  hw/virtio/Kconfig                |    5 +
> > > >  hw/virtio/cbor-helpers.c         |  326 ++++++
> > > >  hw/virtio/meson.build            |    6 +
> > > >  hw/virtio/virtio-nsm-pci.c       |   73 ++
> > > >  hw/virtio/virtio-nsm.c           | 1638 ++++++++++++++++++++++++++++++
> > > >  include/hw/virtio/cbor-helpers.h |   46 +
> > > >  include/hw/virtio/virtio-nsm.h   |   59 ++
> > > >  meson.build                      |    2 +
> > > >  9 files changed, 2165 insertions(+)
> >
> > [...]
> >
> > > > +static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
> > > > +{
> > > > +    g_autofree VirtQueueElement *out_elem = NULL;
> > > > +    g_autofree VirtQueueElement *in_elem = NULL;
> > > > +    VirtIONSM *vnsm = VIRTIO_NSM(vdev);
> > > > +    Error *err = NULL;
> > > > +
> > > > +    out_elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> > > > +    if (!out_elem) {
> > > > +        /* nothing in virtqueue */
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    if (out_elem->out_num != 1) {
> > > > +        virtio_error(vdev, "Expected one request buffer first in 
> > > > virtqueue");
> > > > +        goto cleanup;
> > > > +    }
> > >
> > > Seems to assume request in a single s/g element?
> > > We generally avoid this kind of thing.
> > >
> > > Applies equally elsewheree.
> > >
> >
> > Thank you for reviewing. I think I did it this way (first virqueue_pop
> > gives out_elem with out_num == 1 and the next virtqueue_pop gives
> > in_elem with in_num == 1) after seeing what the virqueue contains
> > (using printfs) when running in a VM and sending some NSM requests and
> > I noticed the above. Can you give me a bit more details about what
> > this should be like? Is there any existing virtio device code I can
> > look at for example?
> > Thanks!
>
>
> Use iov_to_buf / iov_from_buf
>
> there are many examples in the tree, I'd look for some recent ones.
>

I am a bit stuck at this and I would appreciate some help. I looked at
other "iov_to_buf" and "iov_from_buf" examples in QEMU and in those I
see there are known request and response "structs" associated with it.
But in the case of NSM, the request and responses can be arbitrary
CBOR objects i.e., no specific structs or lengths associated. So I am
not sure using "iov_to_buf" / "iov_from_buf" makes sense here.
And about the request response being in a single s/g element, I think
it's because of how the NSM driver is in drivers/misc/nsm.c (see
nsm_sendrecv_msg_locked function)in the linux kernel tree.
I am not sure what changes are needed in the current code if any. Do
you have any suggestions on this?

Regards,
Dorjoy

Reply via email to