Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bring consistency to identify a virtuque by number #163

Closed
paravmellanox opened this issue Feb 23, 2023 · 1 comment
Closed

Bring consistency to identify a virtuque by number #163

paravmellanox opened this issue Feb 23, 2023 · 1 comment
Labels
virtio-v1.3-cs01

Comments

@paravmellanox
Copy link
Contributor

paravmellanox commented Feb 23, 2023 •
edited
Loading

Problem statement:

  1. Currently, a virtqueue is identified between the driver and device
    interchangeably using either number of index terminology.

  2. Between PCI and MMIO transport the queue size (depth) is
    defined as queue_size and QueueNum respectively.

To avoid confusion and to have consistency, unify them to use as Number.

Solution:
Use virtqueue index description and rename MMIO register as QueueSize.

Patch to vote: https://lists.oasis-open.org/archives/virtio-comment/202305/msg00101.html
Patch for maintainer to merge for automation: https://lore.kernel.org/virtio-comment/[email protected]/
@mstsirkin
Copy link
Contributor

BALLOT CREATED AT URL: https://www.oasis-open.org/committees/ballot.php?id=3777

mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
content.tex Replace virtqueue number with index
362f1ca 
Replace virtqueue number with index to align to rest of the
specification.

Fixes: #163
Reviewed-by: David Edmondson <[email protected]>
Reviewed-by: Halil Pasic <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
content: Rename confusing queue_notify_data and vqn names
cc4a560 
Currently queue_notify_data register indicates the device
internal queue notification content. This register is
meaningful only when feature bit VIRTIO_F_NOTIF_CONFIG_DATA is
negotiated.

However, above register name often get confusing association with
very similar feature bit VIRTIO_F_NOTIFICATION_DATA.

When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated,
notification really involves sending additional queue progress
related information (not queue identifier or index).

Hence
1. to avoid any misunderstanding and association of
queue_notify_data with similar name VIRTIO_F_NOTIFICATION_DATA,

and
2. to reflect that queue_notify_data is the actual device
internal virtqueue identifier/index/data/cookie,

a. rename queue_notify_data to queue_notif_config_data.

b. rename ambiguous vqn to a union of vq_index and vq_config_data

c. The driver notification section assumes that queue notification contains
vq index always. CONFIG_DATA feature bit introduction missed to
update the driver notification section. Hence, correct it.

Fixes: #163
Acked-by: Halil Pasic <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>

Reviewed-by: David Edmondson <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
transport-pci: Avoid first vq index reference
fbb119d 
Drop reference to first virtqueue as it is already
covered now by the generic section in first patch.

Fixes: #163
Reviewed-by: David Edmondson <[email protected]>
Acked-by: Halil Pasic <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
transport-mmio: Rename QueueNum register
a7a21e4 
These are further named differently between pci and mmio transport.
PCI transport indicates queue size as queue_size.

To bring consistency between pci and mmio transport,
rename the QueueNumMax and QueueNum
registers to QueueSizeMax and QueueSize respectively.

Fixes: #163
Reviewed-by: Cornelia Huck <[email protected]>
Reviewed-by: Jiri Pirko <[email protected]>
Reviewed-by: Halil Pasic <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
transport-mmio: Avoid referring to zero based index
9ddc595 
VQ range is already described in the first patch in basic virtqueue
section. Hence remove the duplicate reference to it.

Fixes: #163
Reviewed-by: David Edmondson <[email protected]>
Acked-by: Halil Pasic <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
transport-ccw: Rename queue depth/size to other transports
e7a764f 
max_num field reflects the maximum queue size/depth. Hence align name of
this field with similar field in PCI and MMIO transport to
max_queue_size.
Similarly rename 'num' to 'size'.

Fixes: #163
Reviewed-by: Halil Pasic <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
transport-ccw: Refer to the vq by its index
c309241 
Currently specification uses virtqueue index and
number interchangeably to refer to the virtqueue.

Instead refer to it by its index.

Fixes: #163
Reviewed-by: Halil Pasic <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
virtio-net: Avoid duplicate receive queue example
d6f310d 
Receive queue number/index example is duplicate which is already defined
in the Setting RSS parameters section.

Hence, avoid such duplicate example and prepare it for the subsequent
patch to describe using receive queue handle.

Fixes: #163
Reviewed-by: Cornelia Huck <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
virtio-net: Describe RSS using rss rq id
da0e169 
The content of the indirection table and unclassified_queue were
originally described based on mathematical operations. In order to
make it easier to understand and to avoid intermixing the array
index with the vq index, introduce a structure
rss_rq_id (RSS receive queue
ID) and use it to describe the unclassified_queue and
indirection_table fields.

As part of it, have the example that uses non-zero virtqueue
index which helps to have better mapping between receiveX
object with virtqueue index and the actual value in the
indirection table.

Fixes: #163
Reviewed-by: David Edmondson <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
mstsirkin pushed a commit that referenced this issue May 19, 2023
@paravmellanox @mstsirkin
virtio-net: Update vqn to vq_index for cvq cmds
f9ff777 
Replace field name vqn to vq_index for recent virtqueue level commands.

Fixes: #163
Reviewed-by: David Edmondson <[email protected]>
Signed-off-by: Parav Pandit <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
virtio-v1.3-cs01
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mstsirkin @paravmellanox