[virtio-dev] [PATCH 0/2] transport-pci: msix register desc improve

[virtio-dev] [PATCH 0/2] transport-pci: msix register desc improve

[Parav Pandit]

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(-)

-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[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 <mgurtovoy@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
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(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index 1507c69..cf2c54d 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -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.
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Michael S. Tsirkin]

On Thu, Apr 20, 2023 at 10:51:57PM +0300, Parav Pandit wrote:
> 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 <mgurtovoy@nvidia.com>
> Signed-off-by: Parav Pandit <parav@nvidia.com>

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

> ---
> 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(-)
> 
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 1507c69..cf2c54d 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -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.




>  \item[\field{num_queues}]
>          The device specifies the maximum number of virtqueues supported here.
> -- 
> 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: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> 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/


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, April 20, 2023 4:45 PM
> >  \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.
It is high-level description. 0xffff is already covered elsewhere in the doc.

> 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".

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Michael S. Tsirkin]

On Thu, Apr 20, 2023 at 08:51:57PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, April 20, 2023 4:45 PM
> > >  \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.
> It is high-level description. 0xffff is already covered elsewhere in the doc.
> 
> > 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".

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.

?


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Michael S. Tsirkin]

On Thu, Apr 20, 2023 at 08:51:57PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, April 20, 2023 4:45 PM
> > >  \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.
> It is high-level description. 0xffff is already covered elsewhere in the doc.

So make it as brief as possible then.

> > 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".
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[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 <parav@nvidia.com>
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
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(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index cf2c54d..afa082f 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -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.
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Michael S. Tsirkin]

On Thu, Apr 20, 2023 at 10:51:58PM +0300, Parav Pandit wrote:
> 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 <parav@nvidia.com>
> Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
> 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(-)
> 
> diff --git a/transport-pci.tex b/transport-pci.tex
> index cf2c54d..afa082f 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH 0/2] transport-pci: msix register desc improve

[Parav Pandit]

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:
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 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Parav Pandit]

queue_msix_vector register is for receiving 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 <parav@nvidia.com>
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
changelog:
v0->v1:
- added 'here'
- dropped 'receiving'
---
 transport-pci.tex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index cb9fadd..00669b5 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -367,7 +367,7 @@ \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 an MSI-X vector number here for virtqueue interrupts.
 
 \item[\field{queue_enable}]
         The driver uses this to selectively prevent the device from executing requests from this virtqueue.
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org