[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] Re: [PATCH v6 4/5] Add admin_queue_index register to PCI common configuration structure
On 8/1/2022 9:13 AM, Michael S. Tsirkin wrote:
On Mon, Aug 01, 2022 at 03:11:58AM +0300, Max Gurtovoy wrote:On 8/1/2022 12:03 AM, Michael S. Tsirkin wrote:On Sun, Jul 31, 2022 at 06:43:53PM +0300, Max Gurtovoy wrote:This new register will be used for querying the index of the admin virtqueue of a virtio device. To configure, reset or enable the admin virtqueue, the driver should follow existing queue configuration/setup sequence. Reviewed-by: Parav Pandit <parav@nvidia.com> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>Can you please at least add text to MMIO and CCW that drivers and devices must not negotiate the new feature bit? Will help avoid confusion.why this is needed ? who will create a device and expose bits that it doesn't know how to implement.Any multi-transport implementation actually. For example, on a Linux guest the default is to add a feature bit to all transports. Extra care is needed to actually only add it to the PCI transport. With qemu, same applies to features in include/hw/virtio/virtio.h.
I don't know how a reasonable driver will want to negotiate feature bits that a device didn't expose.
Please suggest whatever you want me to add and where.I don't mind adding this but I don't want to waste cycle of review on that so I need exact guideline and not hints please.
And if I'll add it, we might forget to remove it later on.When we add the implementation I'm reasonably sure we won't forget to remove text saying it's not implemented.
Ok.
--- content.tex | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/content.tex b/content.tex index c15423e..5fda1a0 100644 --- a/content.tex +++ b/content.tex @@ -904,6 +904,9 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport le64 queue_device; /* read-write */ le16 queue_notify_data; /* read-only for driver */ le16 queue_reset; /* read-write */ + + /* About admin virtqueue. */ + le16 admin_queue_index; /* read-only for driver */ }; \end{lstlisting} @@ -989,6 +992,10 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport This field exists only if VIRTIO_F_RING_RESET has been negotiated. (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}). +\item[\field{admin_queue_index}] + The device uses this to report the index of the admin virtqueue. + This field always exists. Its value is valid only if VIRTIO_F_ADMIN_VQ has been negotiated. + \end{description} \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}we have a mess with this exists versus valid. I think exists is the samethe bigest mess that we have is for things like ring_reset that are optionally exist according to bit negotiation.I think this optionally exist does not mean much, just that it should not be accessed
If I understand the word "exist" translation correctly, it doesn't mean what you said about "shouldn't be accessed".
about valid - the driver shouldn't even look on registers that it didn't negotiated it's feature bits. There is no reason to do so. So yes, there is a mess but not in our proposal.Right. I propose removing "This field always exists." just say that it's valid with VIRTIO_F_ADMIN_VQ.
Why removing ? does it exist always or not ?
is valid personally. Do others object if we say same as for reset here? Not a big deal either way, we need to clean this up later.@@ -1075,6 +1082,9 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport were used before the queue reset. (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}). +For configuring the admin virtqueue, the driver MUST use the value of \field{admin_queue_index}. +For more details on virtqueue configuration see section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Device Initialization / Virtqueue Configuration}. + \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability} The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG -- 2.21.0This 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.oasis-open.org%2Farchives%2Fvirtio-comment%2F&data=05%7C01%7Cmgurtovoy%40nvidia.com%7C46caf90d77cb4e2e7d1b08da7384fbe5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637949312238253496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=72o5SFbI1Ai5twIQXF0Onn2TTorpuIlWoqp46tpUgrQ%3D&reserved=0 Feedback License: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.oasis-open.org%2Fwho%2Fipr%2Ffeedback_license.pdf&data=05%7C01%7Cmgurtovoy%40nvidia.com%7C46caf90d77cb4e2e7d1b08da7384fbe5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637949312238253496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=7%2FdpXHFFpA7j97vIqLFXfaVQMj1FmDX2Ldwwzytpuuw%3D&reserved=0 List Guidelines: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.oasis-open.org%2Fpolicies-guidelines%2Fmailing-lists&data=05%7C01%7Cmgurtovoy%40nvidia.com%7C46caf90d77cb4e2e7d1b08da7384fbe5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637949312238253496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=lN2X%2BC%2BoHvd90YL%2BSf3FSleiNJYZnfTqRWSdDpK2wAc%3D&reserved=0 Committee: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.oasis-open.org%2Fcommittees%2Fvirtio%2F&data=05%7C01%7Cmgurtovoy%40nvidia.com%7C46caf90d77cb4e2e7d1b08da7384fbe5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637949312238253496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=5FFookod06loAfQ8uGp3HmeuEUdP2lurXWtPNOnCtNY%3D&reserved=0 Join OASIS: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.oasis-open.org%2Fjoin%2F&data=05%7C01%7Cmgurtovoy%40nvidia.com%7C46caf90d77cb4e2e7d1b08da7384fbe5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637949312238253496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=1OJe%2BI1%2FsWYMIaM0PIZjMKmcC%2FpGAoOkHr%2Bi4E4AJOE%3D&reserved=0
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]