transport-pci: msix register desc improve

PATCH Superseded Parav Pandit 2 participants 8 messages Apr 20, 2023 – Apr 21, 2023

lore

Share & tools

lore

Parav Pandit

author

2023-04-20 19:52 UTC #0 lore raw

cover transport-pci: msix register desc improve

Problem:

Current wording queue_msix_vector and config_msix_vector says "for MSI-X", which is bit confusing.

config_msix_vector and queue_msix_vector are the msix vector number for configuration change and queue related interrupts.

Hence, reword it.

---
changelog:
v1->v2:
- rewrote to indicate MSI-X table entry index
v0->v1:
- added 'here'
- dropped 'receiving'
- dropped already merged patch for empty line

Parav Pandit (2):
transport-pci: Improve config msix vector description
transport-pci: Improve queue msix vector register desc

 transport-pci.tex | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Signature · 18 lines

2.26.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Parav Pandit

author

2023-04-20 19:52 UTC #1 lore raw

PATCH 1/2 transport-pci: Improve config msix vector description

config_msix_vector is the register that holds the MSI-X vector number for receiving configuration change related interrupts.

It is not "for MSI-X".

Hence, replace the confusing text with appropriate one.

Reviewed-by Max Gurtovoy <[email protected]>

Signed-off-by Parav Pandit <[email protected]>

---
changelog:
v1->v2:
- rewrote to indicate MSI-X table entry index
v0->v1:
- added 'here'
- dropped 'receiving'

---
 transport-pci.tex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

transport-pci.tex +2 -1

index 1507c69..cf2c54d 100644
@@ -343,7 +343,8 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         Driver Feature Bits selected by \field{driver_feature_select}.
 
 \item[\field{config_msix_vector}]
-        The driver sets the Configuration Vector for MSI-X.
+        The driver writes the MSI-X table entry index in this field for
+        Configuration change notification interrupts.
 
 \item[\field{num_queues}]
         The device specifies the maximum number of virtqueues supported here.

Signature · 18 lines

2.26.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Michael S. Tsirkin

2023-04-20 20:44 UTC #2 lore raw

Re: [virtio-comment] [PATCH 1/2] transport-pci: Improve config msix vector description

Quoted 9 lines · Parav Pandit

config_msix_vector is the register that holds the MSI-X vector number for receiving configuration change related interrupts.
It is not "for MSI-X".
Hence, replace the confusing text with appropriate one.
Reviewed-by Max Gurtovoy <[email protected]>
Signed-off-by Parav Pandit <[email protected]>

it's unfortunate that the pci spec does not have a standard term for msi-x table entry.

Quoted 16 lines

---
changelog:
v1->v2:
- rewrote to indicate MSI-X table entry index
v0->v1:
- added 'here'
- dropped 'receiving'
---
 transport-pci.tex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
transport-pci.tex +0 -0
index 1507c69..cf2c54d 100644

transport-pci.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 347

Quoted 6 lines

transport-pci.tex +2 -1

@@ -343,7 +343,8 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         Driver Feature Bits selected by \field{driver_feature_select}.
 
 \item[\field{config_msix_vector}]
-        The driver sets the Configuration Vector for MSI-X.
+        The driver writes the MSI-X table entry index in this field for
+        Configuration change notification interrupts.

Urm not exactly. there is also a special value of 0xffff.
I feel this here is just a high level description.
how about
Used by the driver to set the MSI-X vector for Configuration change notification interrupts.

pci spec also calls the entry "the vector" so I feel it's ok.

Quoted 21 lines

transport-pci.tex +0 -0

 \item[\field{num_queues}]
         The device specifies the maximum number of virtqueues supported here.

Signature · 18 lines

2.26.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting.

Subscribe: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Parav Pandit

author

2023-04-20 20:52 UTC #3 lore raw

RE: [virtio-comment] [PATCH 1/2] transport-pci: Improve config msix vector description

Quoted 4 lines

From: Michael S. Tsirkin <[email protected]>
Sent: Thursday, April 20, 2023 4:45 PM

Quoted 8 lines

transport-pci.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 347
Quoted 4 lines
\item[\field{config_msix_vector}]
transport-pci.tex +2 -1
-        The driver sets the Configuration Vector for MSI-X.
+        The driver writes the MSI-X table entry index in this field for
+        Configuration change notification interrupts.
Urm not exactly. there is also a special value of 0xffff. I feel this here is just a high level description.

It is high-level description. 0xffff is already covered elsewhere in the doc.

Quoted 6 lines

how about
Used by the driver to set the MSI-X vector for Configuration change
notification interrupts.
pci spec also calls the entry "the vector" so I feel it's ok.

To keep the text consistent with rest of the fields, how about

The driver writes the MSI-X table vector in this field for Configuration change notification interrupts. It uses a single verb instead two = "used" and "set".

This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting.

Michael S. Tsirkin

2023-04-21 05:37 UTC #4 lore raw

Re: [virtio-comment] [PATCH 1/2] transport-pci: Improve config msix vector description

Quoted 18 lines · Parav Pandit

Quoted 4 lines
From: Michael S. Tsirkin <[email protected]>
Sent: Thursday, April 20, 2023 4:45 PM
Quoted 8 lines
transport-pci.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 347
Quoted 4 lines
\item[\field{config_msix_vector}]
transport-pci.tex +2 -1
-        The driver sets the Configuration Vector for MSI-X.
+        The driver writes the MSI-X table entry index in this field for
+        Configuration change notification interrupts.
Urm not exactly. there is also a special value of 0xffff. I feel this here is just a high level description.
It is high-level description. 0xffff is already covered elsewhere in the doc.
Quoted 4 lines
how about
Used by the driver to set the MSI-X vector for Configuration change
notification interrupts.

Quoted 7 lines

Quoted 2 lines
pci spec also calls the entry "the vector" so I feel it's ok.
To keep the text consistent with rest of the fields, how about
The driver writes the MSI-X table vector in this field for Configuration change notification interrupts. It uses a single verb instead two = "used" and "set".

well it's just "the MSI-X vector". not "table vector".
and personally I'd like something less verbose. yes Configuration isn't really clear. and I dislike saying "this field" or "here" - it feels repetetive ...

Set by the driver to the MSI-X Vector for Configuration change notifications.

This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting.

Michael S. Tsirkin

2023-04-21 05:43 UTC #5 lore raw

Re: [virtio-comment] [PATCH 1/2] transport-pci: Improve config msix vector description

In reply to Parav Pandit

Quoted 10 lines · Parav Pandit

Quoted 10 lines
From: Michael S. Tsirkin <[email protected]>
Sent: Thursday, April 20, 2023 4:45 PM
Quoted 4 lines
\item[\field{config_msix_vector}]
transport-pci.tex +2 -1
-        The driver sets the Configuration Vector for MSI-X.
+        The driver writes the MSI-X table entry index in this field for
+        Configuration change notification interrupts.

Quoted 3 lines

Quoted 2 lines
Urm not exactly. there is also a special value of 0xffff. I feel this here is just a high level description.
It is high-level description. 0xffff is already covered elsewhere in the doc.

So make it as brief as possible then.

Quoted 4 lines

Quoted 4 lines
how about
Used by the driver to set the MSI-X vector for Configuration change
notification interrupts.

Quoted 8 lines

Quoted 2 lines
pci spec also calls the entry "the vector" so I feel it's ok.
To keep the text consistent with rest of the fields, how about
The driver writes the MSI-X table vector in this field for Configuration change notification interrupts. It uses a single verb instead two = "used" and "set".

This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting.

Parav Pandit

author

2023-04-20 19:52 UTC #6 lore raw

PATCH 2/2 transport-pci: Improve queue msix vector register desc

In reply to Parav Pandit

queue_msix_vector register is for receiving virtqueue notification interrupts from the device for the virtqueue.

"for MSI-X" is confusing term.

Also it is the register that driver "writes" to, similar to many other registers such as queue_desc, queue_driver etc.

Hence, replace the verb from use to write.

Signed-off-by Parav Pandit <[email protected]>

Reviewed-by Max Gurtovoy <[email protected]>

---
changelog:
v1->v2:
- rewrote to indicate MSI-X table entry index
v0->v1:
- added 'here'
- dropped 'receiving'

---
 transport-pci.tex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

transport-pci.tex +2 -1

index cf2c54d..afa082f 100644
@@ -367,7 +367,8 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         A 0 means the queue is unavailable.
 
 \item[\field{queue_msix_vector}]
-        The driver uses this to specify the queue vector for MSI-X.
+        The driver writes the MSI-X table entry index in this field for
+        virtqueue notification interrupts.
 
 \item[\field{queue_enable}]
         The driver uses this to selectively prevent the device from executing requests from this virtqueue.

Signature · 18 lines

2.26.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Michael S. Tsirkin

2023-04-20 20:45 UTC #7 lore raw

[virtio-comment] Re: [PATCH 2/2] transport-pci: Improve queue msix vector register desc

Quoted 28 lines · Parav Pandit

queue_msix_vector register is for receiving virtqueue notification interrupts from the device for the virtqueue.
"for MSI-X" is confusing term.
Also it is the register that driver "writes" to, similar to many other registers such as queue_desc, queue_driver etc.
Hence, replace the verb from use to write.
Signed-off-by Parav Pandit <[email protected]>
Reviewed-by Max Gurtovoy <[email protected]>
---
changelog:
v1->v2:
- rewrote to indicate MSI-X table entry index
v0->v1:
- added 'here'
- dropped 'receiving'
---
 transport-pci.tex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
transport-pci.tex +0 -0
index cf2c54d..afa082f 100644

transport-pci.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 371

Quoted 6 lines

transport-pci.tex +2 -1

@@ -367,7 +367,8 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         A 0 means the queue is unavailable.
 
 \item[\field{queue_msix_vector}]
-        The driver uses this to specify the queue vector for MSI-X.
+        The driver writes the MSI-X table entry index in this field for
+        virtqueue notification interrupts.

same comment here

Quoted 4 lines

transport-pci.tex +0 -0

 \item[\field{queue_enable}]
         The driver uses this to selectively prevent the device from executing requests from this virtqueue.

Signature · 1 line

2.26.2

This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting.