Re: [PATCH v7 14/16] firmware: arm_scmi: Add atomic mode support to virtio transport

Peter Hilber
2022-01-18T14:32:26Z

On 20.12.21 22:30, Cristian Marussi wrote:
> On Fri, Dec 10, 2021 at 01:12:18PM +0100, Peter Hilber wrote:
>> On 29.11.21 20:11, Cristian Marussi wrote:
<snip>
>>> @@ -65,12 +69,22 @@ struct scmi_vio_channel {
>>> * @input: SDU used for (delayed) responses and notifications
>>> * @list: List which scmi_vio_msg may be part of
>>> * @rx_len: Input SDU size in bytes, once input has been received
>>> + * @poll_idx: Last used index registered for polling purposes if this message
>>> + * transaction reply was configured for polling.
>>> + * Note that virtqueue used index is an unsigned 16-bit.
>>> + * @poll_lock: Protect access to @poll_idx.
>>> */
>>> struct scmi_vio_msg {
>>> struct scmi_msg_payld *request;
>>> struct scmi_msg_payld *input;
>>> struct list_head list;
>>> unsigned int rx_len;
>>> +#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE
>>> +#define VIO_MSG_POLL_DONE 0xffffffffUL
>>
>> virtqueue_enable_cb_prepare() returns an "opaque unsigned value", so
>> this special value should not be used for .poll_idx.
>>
>
> Yes you are right, but looking at comments in virtqueue_enable_cb_prepare()
> and virtqueue_poll()
>
> /**
> * virtqueue_enable_cb_prepare - restart callbacks after disable_cb
> *
> * This re-enables callbacks; it returns current queue state
> * in an opaque unsigned value. This value should be later tested by
> * virtqueue_poll, to detect a possible race between the driver
> * checking for more work, and enabling callbacks.
>
>
> /**
> * virtqueue_poll - query pending used buffers
> * @_vq: the struct virtqueue we're talking about.
> * @last_used_idx: virtqueue state (from call to virtqueue_enable_cb_prepare).
>
> ... it seems to me that is exactly how I'm using it, and moreover I
> don't see any other way via the VirtIO API to grab that last_used_idx
> that I need for virtqueu_poll.
>

I meant to say that the VIO_MSG_POLL_DONE special value should best not be put
into the .poll_idx (since the special value might in theory overlap with an
opaque value). Another variable could hold the special states VIO_MSG_POLL_DONE
and VIO_MSG_NOT_POLLED.