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

2023-07-05 Thread Michael S. Tsirkin
On Wed, Jul 05, 2023 at 04:34:02PM +, Parav Pandit wrote:
> 
> 
> > From: Cornelia Huck 
> > Sent: Wednesday, July 5, 2023 6:24 AM
> > > +
> > > +In some systems, there is a need to support utilizing the legacy
> > > +driver with the device that do not directly support the legacy
> > > +interface. In such scenarios,
> > 
> > Maybe better:
> > 
> > "In some systems, there is a need to support utilizing legacy drivers with 
> > devices
> > that do not directly support the legacy interface."
> > 
> > ?
> >
> Can you please explain how plural is better if it is?
> I see this a lot intermixed in the spec.
> Why cannot we describe things using single object "the driver" and "the 
> device"?
> I prefer to describe things without plurality as done in many parts of the 
> spec.
> Please let me know.

It's ok too, but it must be consistent: "do" goes with plural. For
singular - "does".  Also this is the 1st time we mention this driver and
this device, so should be "a":

In some systems, there is a need to support utilizing a legacy driver 
with
a device that does not directly support the legacy interface.


> > > +a group owner device can provide the legacy interface functionality
> > > +for the group member devices. The driver of an owner device can then
> > > +access the legacy interface of a member device on behalf of the legacy
> > member device driver.
> > 
> > (...)
> > 
> > > +\paragraph{Legacy Driver Notification Query}\label{par:Basic
> > > +Facilities of a Virtio Device / Device groups / Group administration
> > > +commands / Legacy Interface / Legacy Driver Notifications Query}
> > > +
> > > +Even though the driver notifications can be communicated through the
> > > +administration command, if the group owner device or group member
> > > +device supports such notifications using a memory-mapped operation or
> > > +I/O operation, they are sent to the device by accessing such a
> > > +notification region using a memory or an I/O operation.
> > 
> > I find this sentence confusing, probably because it is too complex... what 
> > about
> > 
> > "If the group owner device or the group member device support driver
> > notifications via a memory-mapped operation or I/O operation, these
> > notifications are sent to the device via accessing such a notification 
> > region using
> > a memory or I/O operation {in addition to|instead of} communicating the
> > notifications through the admistration command."
> > 
> With instead of, it reads simpler. Will change it.
> 
> > (I'm not sure whether it is either or both.)
> > 
> > > +
> > > +A group owner device optionally support querying driver notifications
> > > +region
> > 
> > "can optionally support" ?
> >
> Ack.
>  
> > > +using VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command.
> > > +
> > > +The driver sets command \field{opcode} to
> > VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY.
> > > +This command does not use \field{command_specific_data}.
> > > +
> > > +When command completes successfully, \field{command_specific_result}
> > 
> > "the command"
> > 
> Ack
> 
> > > +uses following structure:


-
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 v9 3/4] admin: Add group member legacy register access commands

2023-07-05 Thread Parav Pandit



> From: Cornelia Huck 
> Sent: Wednesday, July 5, 2023 6:24 AM
> > +
> > +In some systems, there is a need to support utilizing the legacy
> > +driver with the device that do not directly support the legacy
> > +interface. In such scenarios,
> 
> Maybe better:
> 
> "In some systems, there is a need to support utilizing legacy drivers with 
> devices
> that do not directly support the legacy interface."
> 
> ?
>
Can you please explain how plural is better if it is?
I see this a lot intermixed in the spec.
Why cannot we describe things using single object "the driver" and "the device"?
I prefer to describe things without plurality as done in many parts of the spec.
Please let me know.
 
> > +a group owner device can provide the legacy interface functionality
> > +for the group member devices. The driver of an owner device can then
> > +access the legacy interface of a member device on behalf of the legacy
> member device driver.
> 
> (...)
> 
> > +\paragraph{Legacy Driver Notification Query}\label{par:Basic
> > +Facilities of a Virtio Device / Device groups / Group administration
> > +commands / Legacy Interface / Legacy Driver Notifications Query}
> > +
> > +Even though the driver notifications can be communicated through the
> > +administration command, if the group owner device or group member
> > +device supports such notifications using a memory-mapped operation or
> > +I/O operation, they are sent to the device by accessing such a
> > +notification region using a memory or an I/O operation.
> 
> I find this sentence confusing, probably because it is too complex... what 
> about
> 
> "If the group owner device or the group member device support driver
> notifications via a memory-mapped operation or I/O operation, these
> notifications are sent to the device via accessing such a notification region 
> using
> a memory or I/O operation {in addition to|instead of} communicating the
> notifications through the admistration command."
> 
With instead of, it reads simpler. Will change it.

> (I'm not sure whether it is either or both.)
> 
> > +
> > +A group owner device optionally support querying driver notifications
> > +region
> 
> "can optionally support" ?
>
Ack.
 
> > +using VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command.
> > +
> > +The driver sets command \field{opcode} to
> VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY.
> > +This command does not use \field{command_specific_data}.
> > +
> > +When command completes successfully, \field{command_specific_result}
> 
> "the command"
> 
Ack

> > +uses following structure:


-
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 v9 3/4] admin: Add group member legacy register access commands

2023-07-05 Thread Cornelia Huck
On Wed, Jul 05 2023, Parav Pandit  wrote:

> diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-legacy-interface.tex
> new file mode 100644
> index 000..09001d5
> --- /dev/null
> +++ b/admin-cmds-legacy-interface.tex
> @@ -0,0 +1,205 @@
> +\subsubsection{Legacy Interfaces}\label{sec:Basic Facilities of a Virtio 
> Device / Device groups / Group
> +administration commands / Legacy Interface}
> +
> +In some systems, there is a need to support utilizing the legacy driver with
> +the device that do not directly support the legacy interface. In such 
> scenarios,

Maybe better:

"In some systems, there is a need to support utilizing legacy drivers with
devices that do not directly support the legacy interface."

?

> +a group owner device can provide the legacy interface functionality for the
> +group member devices. The driver of an owner device can then access the 
> legacy
> +interface of a member device on behalf of the legacy member device driver.

(...)

> +\paragraph{Legacy Driver Notification Query}\label{par:Basic Facilities of a 
> Virtio Device / Device groups / Group administration commands / Legacy 
> Interface / Legacy Driver Notifications Query}
> +
> +Even though the driver notifications can be communicated through the
> +administration command, if the group owner device or group member device
> +supports such notifications using a memory-mapped operation or I/O operation,
> +they are sent to the device by accessing such a notification region using 
> +a memory or an I/O operation.

I find this sentence confusing, probably because it is too
complex... what about

"If the group owner device or the group member device support driver
notifications via a memory-mapped operation or I/O operation, these
notifications are sent to the device via accessing such a notification
region using a memory or I/O operation {in addition to|instead of}
communicating the notifications through the admistration command."

(I'm not sure whether it is either or both.)

> +
> +A group owner device optionally support querying driver notifications region

"can optionally support" ?

> +using VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command.
> +
> +The driver sets command \field{opcode} to 
> VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY.
> +This command does not use \field{command_specific_data}.
> +
> +When command completes successfully, \field{command_specific_result}

"the command"

> +uses following structure:


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