[virtio-dev] RE: [PATCH v8 3/4] admin: Add group member legacy register access commands

2023-07-04 Thread Parav Pandit

> From: Michael S. Tsirkin 
> Sent: Tuesday, July 4, 2023 6:01 PM

[..]

> Got up to this point. Can you try changing the rest to match?
>
Changed and match for rest in v9.
Please continue review rest of in v9.


[virtio-dev] [PATCH v9 2/4] admin: Fix section numbering

2023-07-04 Thread Parav Pandit
Requirements are put one additional level down. Fix it.

Signed-off-by: Parav Pandit 
---
changelog:
v4->v5:
- new patch
---
 admin.tex | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/admin.tex b/admin.tex
index e51f9e6..fd3b97d 100644
--- a/admin.tex
+++ b/admin.tex
@@ -286,7 +286,7 @@ \subsection{Group administration commands}\label{sec:Basic 
Facilities of a Virti
 supporting multiple group types, the list of supported commands
 might differ between different group types.
 
-\devicenormative{\paragraph}{Group administration commands}{Basic Facilities 
of a Virtio Device / Device groups / Group administration commands}
+\devicenormative{\subsubsection}{Group administration commands}{Basic 
Facilities of a Virtio Device / Device groups / Group administration commands}
 
 The device MUST validate \field{opcode}, \field{group_type} and
 \field{group_member_id}, and if any of these has an invalid or
@@ -378,7 +378,7 @@ \subsection{Group administration commands}\label{sec:Basic 
Facilities of a Virti
 \field{VF Enable} refer to registers within the SR-IOV Extended
 Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
 
-\drivernormative{\paragraph}{Group administration commands}{Basic Facilities 
of a Virtio Device / Device groups / Group administration commands}
+\drivernormative{\subsubsection}{Group administration commands}{Basic 
Facilities of a Virtio Device / Device groups / Group administration commands}
 
 The driver MAY discover whether device supports a specific group type
 by issuing VIRTIO_ADMIN_CMD_LIST_QUERY with the matching
-- 
2.26.2


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



[virtio-dev] [PATCH v9 0/4] admin: Access legacy registers using admin commands

2023-07-04 Thread Parav Pandit
This short series introduces legacy registers access commands for the owner
group member access the legacy registers of the member VFs.
This short series introduces legacy region access commands by the group owner
device for its member devices.
Currently it is applicable to the PCI PF and VF devices. If in future any
SIOV devices to support legacy registers, they can be easily supported using
same commands by using the group member identifiers of the future SIOV devices.

More details as overview, motivation, use case are further described
below.

Patch summary:
--
patch-1 split rows of admin opcode tables by a line
patch-2 fix section numbering
patch-3 add generic legacy region access commands
patch-4 add pci specific definition

It uses the newly introduced administration command facility with 4 new
commands and a new optional command to query the legacy notification region.

Usecase:

1. A hypervisor/system needs to provide transitional
   virtio devices to the guest VM at scale of thousands,
   typically, one to eight devices per VM.

2. A hypervisor/system needs to provide such devices using a
   vendor agnostic driver in the hypervisor system.

3. A hypervisor system prefers to have single stack regardless of
   virtio device type (net/blk) and be future compatible with a
   single vfio stack using SR-IOV or other scalable device
   virtualization technology to map PCI devices to the guest VM.
   (as transitional or otherwise)

Motivation/Background:
--
The existing virtio transitional PCI device is missing support for
PCI SR-IOV based devices. Currently it does not work beyond
PCI PF, or as software emulated device in reality. Currently it
has below cited system level limitations:

[a] PCIe spec citation:
VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space.

[b] cpu arch citiation:
Intel 64 and IA-32 Architectures Software Developer’s Manual:
The processor’s I/O address space is separate and distinct from
the physical-memory address space. The I/O address space consists
of 64K individually addressable 8-bit I/O ports, numbered 0 through H.

[c] PCIe spec citation:
If a bridge implements an I/O address range,...I/O address range will be
aligned to a 4 KB boundary.

Overview:
-
Above usecase requirements is solved by PCI PF group owner accessing
its group member PCI VFs legacy registers using an admin virtqueue of
the group owner PCI PF.

Two new admin virtqueue commands are added which read/write PCI VF
registers.

Software usage example:
---
One way to use and map to the guest VM is by using vfio driver
framework in Linux kernel.

+--+
|pci_dev_id = 0x100X   |
+---|pci_rev_id = 0x0  |-+
|vfio device|BAR0 = I/O region | |
|   |Other attributes  | |
|   +--+ |
||
+   +--+ +-+ |
|   |I/O BAR to AQ | | Other vfio  | |
|   |rd/wr mapper  | | functionalities | |
|   +--+ +-+ |
||
+--+-+---+
   | |
   Legacy regionDriver notification
access   |
   | |
  +++   +++
  | +-+ |   | PCI VF device A |
  | | AQ  |-+>+-+ |
  | +-+ |   |   | | legacy regs | |
  | PCI PF device   |   |   | +-+ |
  +-+   |   +-+
|
|   +++
|   | PCI VF device N |
+>+-+ |
| | legacy regs | |
| +-+ |
+-+

2. Virtio pci driver to bind to the listed device id and
   use it in the host.

3. Use it in a light weight hypervisor to run bare-metal OS.

Please review.

Alternatives considered:

1. Exposing BAR0 as MMIO BAR that follows legacy registers template
Pros:
a. Kind of works with legacy drivers as some of them have used API
   which is agnostic to MMIO vs IOBAR.
b. Does not require hypervisor intervantion
Cons:
a. Device reset is extremely hard to implement in device at scale as
   driver does not wait for device reset completion
b. Device register width related problems persist that hypervisor if
   wishes, it cannot be fixed.

2. Accessing VF registers by tunneling it through new legacy PCI capability
Pros:
a. Self contained, but cannot work with future PCI SIOV devices
Cons:
a. Equally slow as AQ access
b. Still requires new capability for notification access
c. Requires hardware to build low level registers access which is 

[virtio-dev] [PATCH v9 4/4] transport-pci: Introduce group legacy group member config region access

2023-07-04 Thread Parav Pandit
This patch links how in a PCI transport a group owner can access group
member (PCI VFs) legacy registers using a legacy registers access
commands using administration virtqueue infrastructure.

Additionally it extend the PCI notification capability through which a
PCI VF device indicates to the driver which PCI BAR region to be used
for driver notifications.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
Signed-off-by: Parav Pandit 
---
changelog:
v7->v8:
- addressed comments from Michael
- made bar offset 64-bit
- prefix legacy specific structure with _legacy
- moved generic normative from pci to generic section
- added link to virtio pci capabilities when referring to bar 0
- remove 'should' from generic description
v6->v7:
- addressed comments from Michael
- removed driver normative about I/O BAR emulation as it does not
  make much sense for the spec
- removed references to administration virtqueue
- rewrote driver legacy region access without guest and hypervisor
  wording
- added normative for notification region
- added normative for PCI IDs for device which support legacy commands
v5->v6:
- aligned pci capability to 4B as required by PCI spec
- added text for the PCI capability for the group member device
v4->v5:
- split pci transport and generic command section to new patch
- removed multiple references to the VF
- written the description of the command as generic with member
  and group device terminology
- reflected many section names to remove VF
v3->v4:
- moved noted to the conformance section details in next patch
- removed queue notify address query AQ command on Michael's suggestion,
  though it is fine. Instead replaced with extending virtio_pci_notify_cap
  to indicate that legacy queue notifications can be done on the
  notification location.
- fixed spelling errors.
- replaced administrative virtqueue to administration virtqueue
- added queue notification capability register to indicate legacy q
  notification supported
v2->v3:
- adddressed Jason and Michael's comment to split single register
  access command to common config and device specific commands.
- dropped the suggetion to introduce enable/disable command as
  admin command cap bit already covers it.
v1->v2:
- addressed comments from Michael
- added theory of operation
- grammar corrections
- removed group fields description from individual commands as
  it is already present in generic section
- added endianness normative for legacy device registers region
- renamed the file to drop vf and add legacy prefix

- added overview in commit log
- renamed subsection to reflect command
---
 admin-cmds-legacy-interface.tex |  2 +-
 conformance.tex |  1 +
 transport-pci-legacy-regs.tex   | 41 +
 transport-pci.tex   |  2 ++
 4 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 transport-pci-legacy-regs.tex

diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-legacy-interface.tex
index 09001d5..571b256 100644
--- a/admin-cmds-legacy-interface.tex
+++ b/admin-cmds-legacy-interface.tex
@@ -150,7 +150,7 @@ \subsubsection{Legacy Interfaces}\label{sec:Basic 
Facilities of a Virtio Device
 by the device.
 
 Refer to the specific transport section for the definition of the
-\field{region_data}.
+\field{region_data}. For PCI transport refer to section \ref{sec:Virtio 
Transport Options / Virtio Over PCI Bus / Legacy Interface: Group Member Device 
Configuration Region Access}.
 
 This command is currently only defined for the PCI SR-IOV group type.
 
diff --git a/conformance.tex b/conformance.tex
index dc00e84..b3f2c92 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -267,6 +267,7 @@ \section{Conformance Targets}\label{sec:Conformance / 
Conformance Targets}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI 
Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / 
PCI-specific Initialization And Device Operation / Device Initialization / 
Virtio Device Configuration Layout Detection / Legacy Interface: A Note on 
Device Layout Detection}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / 
PCI-specific Initialization And Device Operation / Device Initialization / 
Virtqueue Configuration / Legacy Interface: A Note on Virtqueue Configuration}
+\item Section \ref{devicenormative:Virtio Transport Options / Virtio Over PCI 
Bus / Legacy Interface: Group Member Device Configuration Region Access}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over MMIO / Legacy 
interface}
 \item Section \ref{sec:Virtio Transport Options / Virtio over channel I/O / 
Device Initialization / Setting the Virtio Revision / Legacy Interfaces: A Note 
on Setting the Virtio Revision}
 \item Section \ref{sec:Virtio Transport Options / Virtio over channel I/O / 
Device Initialization / Configuring a Virtqueue / Legacy Interface: A Note on 

[virtio-dev] [PATCH v9 1/4] admin: Split opcode table rows with a line

2023-07-04 Thread Parav Pandit
Currently all opcode appears to be in a single row.
Separate them with a line similar to other tables.

Signed-off-by: Parav Pandit 
---
changelog:
v2->v3:
- new patch
---
 admin.tex | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/admin.tex b/admin.tex
index 2efd4d7..e51f9e6 100644
--- a/admin.tex
+++ b/admin.tex
@@ -114,7 +114,9 @@ \subsection{Group administration commands}\label{sec:Basic 
Facilities of a Virti
 opcode & Name & Command Description \\
 \hline \hline
 0x & VIRTIO_ADMIN_CMD_LIST_QUERY & Provides to driver list of commands 
supported for this group type\\
+\hline
 0x0001 & VIRTIO_ADMIN_CMD_LIST_USE & Provides to device list of commands used 
for this group type \\
+\hline
 0x0002 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}\\
 \hline
 0x8000 - 0x & - & Reserved for future commands (possibly using a different 
structure)\\
-- 
2.26.2


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



[virtio-dev] RE: [PATCH v8 3/4] admin: Add group member legacy register access commands

2023-07-04 Thread Parav Pandit


> From: Michael S. Tsirkin 
> Sent: Tuesday, July 4, 2023 6:17 PM
> To: Parav Pandit 
> Cc: virtio-comm...@lists.oasis-open.org; coh...@redhat.com;
> david.edmond...@oracle.com; virtio-dev@lists.oasis-open.org;
> sbu...@marvell.com; jasow...@redhat.com; Yishai Hadas
> ; Maor Gottlieb ; Shahaf Shuler
> 
> Subject: Re: [PATCH v8 3/4] admin: Add group member legacy register access
> commands
> 
> On Tue, Jul 04, 2023 at 10:15:34PM +, Parav Pandit wrote:
> > > From: Michael S. Tsirkin 
> > > Sent: Tuesday, July 4, 2023 6:01 PM
> > >
> > > In some systems, there's need to support utilizing legacy drivers
> > > with devices that do not directly support the legacy interface.
> > > In such scenarios, an owner device of a group can provide the legacy
> > > interface functionality for group members.
> > > The driver of an owner device can then access the legacy interface
> > > of a member device on behalf of the legacy member device driver.
> > >
> > In the last line, I will change from 'legacy member device driver' to just
> 'member device driver'.
> 
> I think it's important to stress legacy since normal driver that follows the 
> spec
> does not access legacy interface at all.
> 
Alright.
Will keep legacy.

> 
> > I will fix rest of the comments including having pad bytes after offset and 
> > have
> similar changes in rest of the places.


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



[virtio-dev] Re: [PATCH v8 3/4] admin: Add group member legacy register access commands

2023-07-04 Thread Michael S. Tsirkin
On Tue, Jul 04, 2023 at 10:15:34PM +, Parav Pandit wrote:
> > From: Michael S. Tsirkin 
> > Sent: Tuesday, July 4, 2023 6:01 PM
> > 
> > In some systems, there's need to support utilizing legacy drivers with 
> > devices
> > that do not directly support the legacy interface.
> > In such scenarios, an owner device of a group can provide the legacy 
> > interface
> > functionality for group members.
> > The driver of an owner device can then access the legacy interface of a 
> > member
> > device on behalf of the legacy member device driver.
> >
> In the last line, I will change from 'legacy member device driver' to just 
> 'member device driver'.

I think it's important to stress legacy since normal driver
that follows the spec does not access legacy interface at all.


> I will fix rest of the comments including having pad bytes after offset and 
> have similar changes in rest of the places.


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



[virtio-dev] RE: [PATCH v8 3/4] admin: Add group member legacy register access commands

2023-07-04 Thread Parav Pandit
> From: Michael S. Tsirkin 
> Sent: Tuesday, July 4, 2023 6:01 PM
> 
> In some systems, there's need to support utilizing legacy drivers with devices
> that do not directly support the legacy interface.
> In such scenarios, an owner device of a group can provide the legacy interface
> functionality for group members.
> The driver of an owner device can then access the legacy interface of a member
> device on behalf of the legacy member device driver.
>
In the last line, I will change from 'legacy member device driver' to just 
'member device driver'.

I will fix rest of the comments including having pad bytes after offset and 
have similar changes in rest of the places.


[virtio-dev] RE: [PATCH v8 0/4] admin: Access legacy registers using admin commands

2023-07-04 Thread Parav Pandit


> From: Michael S. Tsirkin 
> Sent: Tuesday, July 4, 2023 11:41 AM
> > Do you have any further comments on 3rd and 4th patch?
> > I fixed your comments of v7, and some more were fixed.
> 
> 
> Hope to send review tonight, max tomorrow.
Ok. thanks a lot.

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



[virtio-dev] Re: [PATCH v8 0/4] admin: Access legacy registers using admin commands

2023-07-04 Thread Michael S. Tsirkin
On Tue, Jul 04, 2023 at 01:26:20PM +, Parav Pandit wrote:
> Hi Michael,
> 
> > From: Parav Pandit 
> > Sent: Friday, June 30, 2023 12:20 AM
> > 
> > This short series introduces legacy registers access commands for the owner
> > group member access the legacy registers of the member VFs.
> > This short series introduces legacy region access commands by the group 
> > owner
> > device for its member devices.
> > Currently it is applicable to the PCI PF and VF devices. If in future any 
> > SIOV
> > devices to support legacy registers, they can be easily supported using same
> > commands by using the group member identifiers of the future SIOV devices.
> > 
> > More details as overview, motivation, use case are further described below.
> > 
> > Patch summary:
> > --
> > patch-1 split rows of admin opcode tables by a line
> > patch-2 fix section numbering
> > patch-3 add generic legacy region access commands
> > patch-4 add pci specific definition
> 
> [..]
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> > Signed-off-by: Parav Pandit 
> > 
> > ---
> > changelog:
> > v7->v8:
> > - remove empty line at the end of file
> > - removed white space at the end
> > - addressed comments from Michael add link to pci
> > - renamed region to region_data
> > - made region_data width to be 16 bytes to cover for 8 bytes offset
> > - moved generic notification region related normative from pci to
> >   generic section
> > - made bar offset 64-bit
> > - prefix legacy specific structure with _legacy
> > - moved generic normative from pci to generic section
> > - added link to virtio pci capabilities when referring to bar 0
> > - remove 'should' from generic description
> 
> Do you have any further comments on 3rd and 4th patch?
> I fixed your comments of v7, and some more were fixed.


Hope to send review tonight, max tomorrow.


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



Re: [virtio-dev] [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user

2023-07-04 Thread Alex Bennée


Stefano Garzarella  writes:

> On Tue, Jul 04, 2023 at 01:36:00PM +0100, Alex Bennée wrote:
>>Currently QEMU has to know some details about the back-end to be able
>>to setup the guest. While various parts of the setup can be delegated
>>to the backend (for example config handling) this is a very piecemeal
>>approach.
>>
>>This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE)
>>which the back-end can advertise which allows a probe message to be
>>sent to get all the details QEMU needs to know in one message.
>>
>>Signed-off-by: Alex Bennée 
>>
>>---
>>Initial RFC for discussion. I intend to prototype this work with QEMU
>>and one of the rust-vmm vhost-user daemons.
>
> Thanks for starting this discussion!
>
> I'm comparing with vhost-vdpa IOCTLs, so my questions may be
> superficial, but they help me understand the differences.

I did have a quick read-through to get a handle on vhost-vdpa but the
docs are fairly empty. However I see there is more detail in the linux
commit so after looking at that I do wonder:

 * The kernel commit defines a subset of VIRTIO_F feature flags. Should
   we do the same for this interface?

 * The VDPA GET/SET STATUS and GET/SET CONFIG ioctls are already covered
   by the equivalent VHOST_USER messages?

>>---
>> docs/interop/vhost-user.rst | 37 +
>> hw/virtio/vhost-user.c  |  8 
>> 2 files changed, 45 insertions(+)
>>
>>diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
>>index 5a070adbc1..85b1b1583a 100644
>>--- a/docs/interop/vhost-user.rst
>>+++ b/docs/interop/vhost-user.rst
>>@@ -275,6 +275,21 @@ Inflight description
>>
>> :queue size: a 16-bit size of virtqueues
>>
>>+Backend specifications
>>+^^
>>+
>>++---+-+++
>>+| device id | config size |   min_vqs  |   max_vqs  |
>>++---+-+++
>>+
>>+:device id: a 32-bit value holding the VirtIO device ID
>>+
>>+:config size: a 32-bit value holding the config size (see 
>>``VHOST_USER_GET_CONFIG``)
>>+
>>+:min_vqs: a 32-bit value holding the minimum number of vqs supported
>
> Why do we need the minimum?

We need to know the minimum number because some devices have fixed VQs
that must be present.

>
>>+
>>+:max_vqs: a 32-bit value holding the maximum number of vqs supported, must 
>>be >= min_vqs
>
> Is this overlap with VHOST_USER_GET_QUEUE_NUM?

Yes it does and I considered implementing a bunch of messages to fill in
around what we already have. However that seemed like it would add a
bunch of complexity to the interface when we could get all the initial
data in one go.

>
>>+
>> C structure
>> ---
>>
>>@@ -296,6 +311,7 @@ In QEMU the vhost-user message is implemented with the 
>>following struct:
>>   VhostUserConfig config;
>>   VhostUserVringArea area;
>>   VhostUserInflight inflight;
>>+  VhostUserBackendSpecs specs;
>>   };
>>   } QEMU_PACKED VhostUserMsg;
>>
>>@@ -316,6 +332,7 @@ replies. Here is a list of the ones that do:
>> * ``VHOST_USER_GET_VRING_BASE``
>> * ``VHOST_USER_SET_LOG_BASE`` (if ``VHOST_USER_PROTOCOL_F_LOG_SHMFD``)
>> * ``VHOST_USER_GET_INFLIGHT_FD`` (if 
>> ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD``)
>>+* ``VHOST_USER_GET_BACKEND_SPECS`` (if ``VHOST_USER_PROTOCOL_F_STANDALONE``)
>>
>> .. seealso::
>>
>>@@ -885,6 +902,13 @@ Protocol features
>>   #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS  15
>>   #define VHOST_USER_PROTOCOL_F_STATUS   16
>>   #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17
>>+  #define VHOST_USER_PROTOCOL_F_STANDALONE   18
>>+
>>+Some features are only valid in the presence of other supporting
>>+features. In the case of ``VHOST_USER_PROTOCOL_F_STANDALONE`` the
>>+backend must also support ``VHOST_USER_PROTOCOL_F_CONFIG`` and
>>+``VHOST_USER_PROTOCOL_F_STATUS``.
>>+
>
> What about adding a new section where we will describe what we mean
> with "standalone" devices?
>
> For example that the entire virtio device is emulated in the backend,
> etc.
>
> By the way, I was thinking more about F_FULL_DEVICE, but I'm not good
> with names, so I'll just throw out an idea :-)

Naming things is hard ;-)

I could add a new section although AIUI there is nothing really in
existing daemons which is split between the front-end and back-end. The
stubs are basically boilerplate and ensure DT/PCIe hubs make the device
appear so once things are discovered QEMU never really gets involved
aside from being a dumb relay.

>
> Thanks,
> Stefano
>
>>
>> Front-end message types
>> ---
>>@@ -1440,6 +1464,19 @@ Front-end message types
>>   query the back-end for its device status as defined in the Virtio
>>   specification.
>>
>>+``VHOST_USER_GET_BACKEND_SPECS``
>>+  :id: 41
>>+  :request payload: N/A
>>+  :reply payload: ``Backend specifications``
>>+
>>+  When the ``VHOST_USER_PROTOCOL_F_STANDALONE`` protocol feature has been

Re: [virtio-dev] [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user

2023-07-04 Thread Stefano Garzarella

On Tue, Jul 04, 2023 at 01:36:00PM +0100, Alex Bennée wrote:

Currently QEMU has to know some details about the back-end to be able
to setup the guest. While various parts of the setup can be delegated
to the backend (for example config handling) this is a very piecemeal
approach.

This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE)
which the back-end can advertise which allows a probe message to be
sent to get all the details QEMU needs to know in one message.

Signed-off-by: Alex Bennée 

---
Initial RFC for discussion. I intend to prototype this work with QEMU
and one of the rust-vmm vhost-user daemons.


Thanks for starting this discussion!

I'm comparing with vhost-vdpa IOCTLs, so my questions may be
superficial, but they help me understand the differences.


---
docs/interop/vhost-user.rst | 37 +
hw/virtio/vhost-user.c  |  8 
2 files changed, 45 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 5a070adbc1..85b1b1583a 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -275,6 +275,21 @@ Inflight description

:queue size: a 16-bit size of virtqueues

+Backend specifications
+^^
+
++---+-+++
+| device id | config size |   min_vqs  |   max_vqs  |
++---+-+++
+
+:device id: a 32-bit value holding the VirtIO device ID
+
+:config size: a 32-bit value holding the config size (see 
``VHOST_USER_GET_CONFIG``)
+
+:min_vqs: a 32-bit value holding the minimum number of vqs supported


Why do we need the minimum?


+
+:max_vqs: a 32-bit value holding the maximum number of vqs supported, must be 
>= min_vqs


Is this overlap with VHOST_USER_GET_QUEUE_NUM?


+
C structure
---

@@ -296,6 +311,7 @@ In QEMU the vhost-user message is implemented with the 
following struct:
  VhostUserConfig config;
  VhostUserVringArea area;
  VhostUserInflight inflight;
+  VhostUserBackendSpecs specs;
  };
  } QEMU_PACKED VhostUserMsg;

@@ -316,6 +332,7 @@ replies. Here is a list of the ones that do:
* ``VHOST_USER_GET_VRING_BASE``
* ``VHOST_USER_SET_LOG_BASE`` (if ``VHOST_USER_PROTOCOL_F_LOG_SHMFD``)
* ``VHOST_USER_GET_INFLIGHT_FD`` (if ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD``)
+* ``VHOST_USER_GET_BACKEND_SPECS`` (if ``VHOST_USER_PROTOCOL_F_STANDALONE``)

.. seealso::

@@ -885,6 +902,13 @@ Protocol features
  #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS  15
  #define VHOST_USER_PROTOCOL_F_STATUS   16
  #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17
+  #define VHOST_USER_PROTOCOL_F_STANDALONE   18
+
+Some features are only valid in the presence of other supporting
+features. In the case of ``VHOST_USER_PROTOCOL_F_STANDALONE`` the
+backend must also support ``VHOST_USER_PROTOCOL_F_CONFIG`` and
+``VHOST_USER_PROTOCOL_F_STATUS``.
+


What about adding a new section where we will describe what we mean
with "standalone" devices?

For example that the entire virtio device is emulated in the backend,
etc.

By the way, I was thinking more about F_FULL_DEVICE, but I'm not good
with names, so I'll just throw out an idea :-)

Thanks,
Stefano



Front-end message types
---
@@ -1440,6 +1464,19 @@ Front-end message types
  query the back-end for its device status as defined in the Virtio
  specification.

+``VHOST_USER_GET_BACKEND_SPECS``
+  :id: 41
+  :request payload: N/A
+  :reply payload: ``Backend specifications``
+
+  When the ``VHOST_USER_PROTOCOL_F_STANDALONE`` protocol feature has been
+  successfully negotiated, this message is submitted by the front-end to
+  query the back-end for its capabilities. This is intended to remove
+  the need for the front-end to know ahead of time what the VirtIO
+  device the backend emulates is.
+
+  The reply contains the device id, size of the config space and the
+  range of VirtQueues the backend supports.

Back-end message types
--
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index c4e0cbd702..28b021d5d3 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -202,6 +202,13 @@ typedef struct VhostUserInflight {
uint16_t queue_size;
} VhostUserInflight;

+typedef struct VhostUserBackendSpecs {
+uint32_t device_id;
+uint32_t config_size;
+uint32_t min_vqs;
+uint32_t max_vqs;
+} VhostUserBackendSpecs;
+
typedef struct {
VhostUserRequest request;

@@ -226,6 +233,7 @@ typedef union {
VhostUserCryptoSession session;
VhostUserVringArea area;
VhostUserInflight inflight;
+VhostUserBackendSpecs specs;
} VhostUserPayload;

typedef struct VhostUserMsg {
--
2.39.2


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





[virtio-dev] RE: [PATCH v8 0/4] admin: Access legacy registers using admin commands

2023-07-04 Thread Parav Pandit
Hi Michael,

> From: Parav Pandit 
> Sent: Friday, June 30, 2023 12:20 AM
> 
> This short series introduces legacy registers access commands for the owner
> group member access the legacy registers of the member VFs.
> This short series introduces legacy region access commands by the group owner
> device for its member devices.
> Currently it is applicable to the PCI PF and VF devices. If in future any SIOV
> devices to support legacy registers, they can be easily supported using same
> commands by using the group member identifiers of the future SIOV devices.
> 
> More details as overview, motivation, use case are further described below.
> 
> Patch summary:
> --
> patch-1 split rows of admin opcode tables by a line
> patch-2 fix section numbering
> patch-3 add generic legacy region access commands
> patch-4 add pci specific definition

[..]
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> Signed-off-by: Parav Pandit 
> 
> ---
> changelog:
> v7->v8:
> - remove empty line at the end of file
> - removed white space at the end
> - addressed comments from Michael add link to pci
> - renamed region to region_data
> - made region_data width to be 16 bytes to cover for 8 bytes offset
> - moved generic notification region related normative from pci to
>   generic section
> - made bar offset 64-bit
> - prefix legacy specific structure with _legacy
> - moved generic normative from pci to generic section
> - added link to virtio pci capabilities when referring to bar 0
> - remove 'should' from generic description

Do you have any further comments on 3rd and 4th patch?
I fixed your comments of v7, and some more were fixed.


[virtio-dev] [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user

2023-07-04 Thread Alex Bennée
Currently QEMU has to know some details about the back-end to be able
to setup the guest. While various parts of the setup can be delegated
to the backend (for example config handling) this is a very piecemeal
approach.

This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE)
which the back-end can advertise which allows a probe message to be
sent to get all the details QEMU needs to know in one message.

Signed-off-by: Alex Bennée 

---
Initial RFC for discussion. I intend to prototype this work with QEMU
and one of the rust-vmm vhost-user daemons.
---
 docs/interop/vhost-user.rst | 37 +
 hw/virtio/vhost-user.c  |  8 
 2 files changed, 45 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 5a070adbc1..85b1b1583a 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -275,6 +275,21 @@ Inflight description
 
 :queue size: a 16-bit size of virtqueues
 
+Backend specifications
+^^
+
++---+-+++
+| device id | config size |   min_vqs  |   max_vqs  |
++---+-+++
+
+:device id: a 32-bit value holding the VirtIO device ID
+
+:config size: a 32-bit value holding the config size (see 
``VHOST_USER_GET_CONFIG``)
+
+:min_vqs: a 32-bit value holding the minimum number of vqs supported
+
+:max_vqs: a 32-bit value holding the maximum number of vqs supported, must be 
>= min_vqs
+
 C structure
 ---
 
@@ -296,6 +311,7 @@ In QEMU the vhost-user message is implemented with the 
following struct:
   VhostUserConfig config;
   VhostUserVringArea area;
   VhostUserInflight inflight;
+  VhostUserBackendSpecs specs;
   };
   } QEMU_PACKED VhostUserMsg;
 
@@ -316,6 +332,7 @@ replies. Here is a list of the ones that do:
 * ``VHOST_USER_GET_VRING_BASE``
 * ``VHOST_USER_SET_LOG_BASE`` (if ``VHOST_USER_PROTOCOL_F_LOG_SHMFD``)
 * ``VHOST_USER_GET_INFLIGHT_FD`` (if ``VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD``)
+* ``VHOST_USER_GET_BACKEND_SPECS`` (if ``VHOST_USER_PROTOCOL_F_STANDALONE``)
 
 .. seealso::
 
@@ -885,6 +902,13 @@ Protocol features
   #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS  15
   #define VHOST_USER_PROTOCOL_F_STATUS   16
   #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17
+  #define VHOST_USER_PROTOCOL_F_STANDALONE   18
+
+Some features are only valid in the presence of other supporting
+features. In the case of ``VHOST_USER_PROTOCOL_F_STANDALONE`` the
+backend must also support ``VHOST_USER_PROTOCOL_F_CONFIG`` and
+``VHOST_USER_PROTOCOL_F_STATUS``.
+
 
 Front-end message types
 ---
@@ -1440,6 +1464,19 @@ Front-end message types
   query the back-end for its device status as defined in the Virtio
   specification.
 
+``VHOST_USER_GET_BACKEND_SPECS``
+  :id: 41
+  :request payload: N/A
+  :reply payload: ``Backend specifications``
+
+  When the ``VHOST_USER_PROTOCOL_F_STANDALONE`` protocol feature has been
+  successfully negotiated, this message is submitted by the front-end to
+  query the back-end for its capabilities. This is intended to remove
+  the need for the front-end to know ahead of time what the VirtIO
+  device the backend emulates is.
+
+  The reply contains the device id, size of the config space and the
+  range of VirtQueues the backend supports.
 
 Back-end message types
 --
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index c4e0cbd702..28b021d5d3 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -202,6 +202,13 @@ typedef struct VhostUserInflight {
 uint16_t queue_size;
 } VhostUserInflight;
 
+typedef struct VhostUserBackendSpecs {
+uint32_t device_id;
+uint32_t config_size;
+uint32_t min_vqs;
+uint32_t max_vqs;
+} VhostUserBackendSpecs;
+
 typedef struct {
 VhostUserRequest request;
 
@@ -226,6 +233,7 @@ typedef union {
 VhostUserCryptoSession session;
 VhostUserVringArea area;
 VhostUserInflight inflight;
+VhostUserBackendSpecs specs;
 } VhostUserPayload;
 
 typedef struct VhostUserMsg {
-- 
2.39.2


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



[virtio-dev] Re: [PATCH v21] virtio-net: support inner header hash

2023-07-04 Thread Cornelia Huck
On Mon, Jul 03 2023, "Michael S. Tsirkin"  wrote:

> On Mon, Jul 03, 2023 at 11:27:11PM +0800, Heng Qi wrote:
>> 1. Currently, a received encapsulated packet has an outer and an inner 
>> header, but
>> the virtio device is unable to calculate the hash for the inner header. The 
>> same
>> flow can traverse through different tunnels, resulting in the encapsulated
>> packets being spread across multiple receive queues (refer to the figure 
>> below).
>> However, in certain scenarios, we may need to direct these encapsulated 
>> packets of
>> the same flow to a single receive queue. This facilitates the processing
>> of the flow by the same CPU to improve performance (warm caches, less 
>> locking, etc.).
>> 
>>client1client2
>>   |+---+ |
>>   +--->|tunnels|<+
>>+---+
>>   |  |
>>   v  v
>>   +-+
>>   | monitoring host |
>>   +-+
>> 
>> To achieve this, the device can calculate a symmetric hash based on the 
>> inner headers
>> of the same flow.
>> 
>> 2. For legacy systems, they may lack entropy fields which modern protocols 
>> have in
>> the outer header, resulting in multiple flows with the same outer header but
>> different inner headers being directed to the same receive queue. This 
>> results in
>> poor receive performance.
>> 
>> To address this limitation, inner header hash can be used to enable the 
>> device to advertise
>> the capability to calculate the hash for the inner packet, regaining better 
>> receive performance.
>> 
>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/173
>> Signed-off-by: Heng Qi 
>> Reviewed-by: Xuan Zhuo 
>> Reviewed-by: Parav Pandit 
>
> Cornelia last minute comments before I start a vote?

Please just go ahead.


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