On Thu, Jun 08, 2023 at 07:00:32PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <[email protected]>
> > Sent: Thursday, June 8, 2023 2:31 PM
> 
> > > > I'll do a proper review after the forum. Generally lots of small
> > > > things. Went looking just to give you a couple of
> > > > examples:
> > > >           too many mentions of VFs and PFs.
> > > >           text should talk about owner and member. Minimise
> > > >           mention of VFs to make it easier to extend to
> > > >           different group types.
> > > >
> > > True but most additions are in PCI transport chapter.
> > > But will change to member and owner.
> > 
> > Another thing that bothers me is that it references admin commands that are
> > defined later in the spec. I don't like it that we are making the reader 
> > jump back
> > and forth ...
> > Maybe it's better to put this in the admin command chapter.
> > 
> I considered that before.
> In its current form, there is very less back-n-forth jump.
> This is because admin chapter mostly enumerates the opcode.
> 
> And PCI legacy chapter talks about rest of the theory and command details.
> 
> When/if we have more transports for this, probably a generic place will be 
> suitable.
> 
> Moving now to admin will surely have more back-n-forth as it needs to talk 
> about PCI part and that PCI part will reside in PCI section.

well the point is that it's *back" not *forth*.
IOW add the command description in admin chapter,
make it point to legacy description in pci chapter which reader has
already read.

> > > > another example:
> > > >         +The PCI VF device SHOULD NOT expose PCI BAR 0 when it prefers 
> > > > to
> > > > support
> > > >
> > > > VFs don't expose BARs at all. PF exposes VF BARs in SRIOV capability.
> > > >
> > > Yes, it is exposed by PF, the wording of "PCI VF device exposing" is not 
> > > right.
> > > I will reword it.
> > 
> > So here's an example wording, I don't insist on it exactly but the point is 
> > to show
> > how we should use spec terminology whereever
> > possible:
> > 
> > If an owner of an SRIOV group supports all of
> > VIRTIO_ADMIN_CMD_LCC_REG_WRITE,
> > VIRTIO_ADMIN_CMD_LCC_REG_READ .... then it SHOULD NOT expose VF BAR0
> > (of non 0 size) as part of its SRIOV capability; this is to facilitate 
> > emulating IO
> > BAR0 for the legacy interface in software.
> > 
> Yes good one. Will use.

right and my point is try to word all of it like this.


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to