[virtio-dev] RE: [PATCH v14] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin
> Sent: Tuesday, July 11, 2023 5:14 AM
> > Looking at this sentence again, maybe make this
> >
> > "A \field{flags} value of 0x1 indicates that the notification address
> > is that of the owner device, a value of 0x2 indicates that the
> > notification address is that of the member device, and a value of 0
> indicates..."
> >
> > if you're tweaking it anyway?
>
> Oh yes, of course. Ack to that.
Done in v15.
-
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 v14] admin: Add group member legacy register access commands
On Tue, Jul 11, 2023 at 11:10:09AM +0200, Cornelia Huck wrote:
> On Mon, Jul 10 2023, "Michael S. Tsirkin" wrote:
>
> > On Mon, Jul 10, 2023 at 07:17:52PM +0300, Parav Pandit wrote:
> >> +The \field{flags} value of 0x1 indicates that the notification address is
> >> of
> >> +the owner device, value of 0x2 indicates that the notification address is
> >> of
> >> +the member device, the value of 0 indicates that all the entries starting
> >> from
> >
> > , the -> and value of 0x0
>
> Looking at this sentence again, maybe make this
>
> "A \field{flags} value of 0x1 indicates that the notification address is
> that of the owner device, a value of 0x2 indicates that the notification
> address is that of the member device, and a value of 0 indicates..."
>
> if you're tweaking it anyway?
Oh yes, of course. Ack to that.
> >
> >
> >> +that entry are invalid entries in \field{entries}. All other values in
> >> +\field{flags} are reserved.
-
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 v14] admin: Add group member legacy register access commands
On Mon, Jul 10 2023, "Michael S. Tsirkin" wrote:
> On Mon, Jul 10, 2023 at 07:17:52PM +0300, Parav Pandit wrote:
>> +The \field{flags} value of 0x1 indicates that the notification address is of
>> +the owner device, value of 0x2 indicates that the notification address is of
>> +the member device, the value of 0 indicates that all the entries starting
>> from
>
> , the -> and value of 0x0
Looking at this sentence again, maybe make this
"A \field{flags} value of 0x1 indicates that the notification address is
that of the owner device, a value of 0x2 indicates that the notification
address is that of the member device, and a value of 0 indicates..."
if you're tweaking it anyway?
>
>
>> +that entry are invalid entries in \field{entries}. All other values in
>> +\field{flags} are reserved.
-
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 v14] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin
> Sent: Monday, July 10, 2023 12:35 PM
[..]
> > +The \field{flags} value of 0x1 indicates that the notification
> > +address is of the owner device, value of 0x2 indicates that the
> > +notification address is of the member device, the value of 0
> > +indicates that all the entries starting from
>
> , the -> and value of 0x0
>
>
> > +that entry are invalid entries in \field{entries}. All other values
> > +in \field{flags} are reserved.
> > +
> > +The \field{bar} values 0x1 to 0x5 specify BAR1 to BAR5 respectively:
> > +when the \field{flags} is 0x1 this is specified by the Base Address
> > +Registers in the PCI header of the device, when the \field{flags} is
> > +0x2 this is specified by the VF BARn registers in the SR-IOV Extended
> > +Capability of the device.
> > +
> > +The \field{offset} indicates the notification address relative to BAR
> > +indicated in \field{bar}. This value is 2-byte aligned.
> > +
> > +When the command completes successfully,
> > +\field{command_specific_result} is in the format of \field{struct
> virtio_admin_cmd_legacy_notify_info_result}.
>
>
> > The
> > +device can supply up to 4 entries each with a different notification
> > +address. In this case, any of the entries can be used by the driver.
> > +The order of the entries serves as a preference hint to the driver.
> > +The driver is expected to utilize the entries placed earlier in the array
> > to the
> later ones.
>
> to -> in preference to
>
> > The driver is
> > +also expected to ignore any reserved entries and to skip all invalid
> > entries.
>
> is there a difference between ignore and skip?
> And what are reserved entries?
>
Reserved entries are the entries which has value > 0x2 for future.
> The driver is also expected to ignore any invalid entries, as well
> as the end of list entry if present and any entries following
> the end of list.
>
The driver is also expected to ignore any invalid or reserved entries,
as well
as the end of list entry if present and any entries following
the end of list.
Please see above if addition of "or reserved" looks fine.
Will fix rest of the comments now.
[virtio-dev] Re: [PATCH v14] admin: Add group member legacy register access commands
On Mon, Jul 10, 2023 at 07:17:52PM +0300, Parav Pandit wrote: > Introduce group member legacy common configuration and legacy device > configuration access read/write commands. > > Group member legacy registers access commands enable group owner driver > software to access legacy registers on behalf of the guest virtual > machine. > > 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 the administration > commands of the group owner PCI PF. > > Two types of administration commands are added which read/write PCI VF > registers. > > Software usage example: > === > > 1. 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 | | > | | forwarder| | | | > | +--+ +-+ | > || > +--+-+---+ >| | >Config region | > accessDriver notifications >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Continue to use the virtio pci driver to bind to the >listed device id and use it as in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > Signed-off-by: Parav Pandit > Signed-off-by: Michael S. Tsirkin OK good. Found one more issue, below. As long as we need another revision I suggest a couple more minor tweaks. > --- > changelog: > v13->v14: > - addressed below comments from Michael > - reworded BAR 1 to 5 text as_is suggested by Michael > - added text for skipping invalid entries > - replaced 'contains' with 'contain' > - removed 'related to the base address associated' > - addressed comments from Cornelia > - added article the at many places > - replaced does not to do not > - reworded the driver normative for flags parsing > v12->v13: > - added article > - add hyphen between little and endian > - mentioned vq index depth of 16-bit > - rewrote alternative approach line > - mention vq index, length and endianness in mmio description > - fixed padding bytes size from 7 to 6 bytes > - rewrote bar field description > - offset alignment text added > - added text to ignore reserved notification entries > - device and driver conformance lines added for notification info command > fie
