[virtio-dev] Re: [PATCH v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 03:48:42PM +, Parav Pandit wrote:
>
> > From: Michael S. Tsirkin
> > Sent: Tuesday, February 8, 2022 9:08 PM
>
>
> > On Tue, Feb 08, 2022 at 03:35:58PM +, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin
> > > > Sent: Tuesday, February 8, 2022 8:56 PM
> > >
> > > > > I might have been a bit too vague about what I had been thinking
> > > > > about. Let's do a sketch (intentionally without concrete sizes):
> > > > >
> > > > > +---+
> > > > > | command |
> > > > > +---+
> > > > > | target type (0 - self, 1 - dev id, 2 - group id, ... |
> > > > > +---+
> > > > > | dev id|
> > > > > +---+
> > > > > | group id |
> > > > > +---+
> > > > > | command-specific data |
> > > > > +---+
> > > > > | response part |
> > > > > +---+
> > > > >
> > > > > 'dev id' would be valid for 'target type' == 1, 'group id' would
> > > > > be valid for 'target type' == 2. Alternatively, 'dev id' and 'group
> > > > > id'
> > > > > could be a single 'target id' field; if there's nothing better to
> > > > > use, it can simply contain a uuid.
> > > >
> > > > I am not sure why do we have both dev id and group id.
> > > > They are never used together right?
> > > > Maybe just have an id length field if we can't agree on how much
> > > > space to reserve.
> > > This is what I propose in a previous email.
> > > A device id can be duplicate in different groups.
> > > So to build hierarchy group id will be desired.
> > >
> > > So id[] array can contain nested one or multiple fields.
> >
> > I guess nesting is for when there's like an SF within a VF?
> That is one case, but I imagine that if SF is on top of VF, than VF will
> directly manage it. PF should not get involved in nested management.
>
> Nesting is for the case, where there is explicit group. A simple example is,
> A NIC has two PCI functions, both support SR-IOV and VFs.
> But only one PF is managing the VFs for msix, mac etc configuration.
Or maybe that's the case of "it hurts when I do this - then don't do that".
> In this case there are two groups g0, g1.
> grp0 belongs to PF0.
> grp1 belongs to PF1.
>
> So a id definition looks like below.
>
> struct pf_identifier {
> u8 device; /* pci device of bdf */
> u8 function;/* pci function of bdf */
> } __attribute__(packed);
>
> struct pci_vf_grouped_id {
> struct pf_identifier pf_id;
> u16 vf_num;
> };
>
> struct device_identfier {
> u8 id_type; /* below id type enum */
> u16 id_len;
> union {
> u8 raw[0];
> struct pci_vf_grouped_id gvf_id;
> } id_data;
> };
>
> enum id_type {
> VIRTIO_PCI_VF = 0, /* implicitly grouped VF */
> VIRTIO_PCI_REMOTE_VF = 1, /* part of hirechical group */
> VIRTIO_SF,
> VIRTIO_PCI_REMOTE_SF, /* part of hirechical group */
> VIRTIO_MMIO,
> /* more */
> };
-
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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 06:52:09PM +, Parav Pandit wrote:
>
> > From: Michael S. Tsirkin
> > Sent: Tuesday, February 8, 2022 9:10 PM
> >
> > On Tue, Feb 08, 2022 at 03:06:16PM +, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin
> > > > Sent: Tuesday, February 8, 2022 7:29 PM
> > > >
> > > > > Do we have a concrete example of a command that can be targeted
> > > > > for same
> > > > device and a target device, which requires differentiating their
> > > > destination? If so, lets discuss and then it make sense to add for the
> > > > well-
> > defined use case.
> > > >
> > > > So e.g. things like controlling NIC's MAC can reasonably be part of
> > > > the same device.
> > > A mac address of NIC can be programmed via the existing control VQ for the
> > self.
> >
> > Not if it's disabled for the guest.
> >
> Its unrelated.
> The idea was to issue same command in same way by two devices = primary and
> secondary.
> And in case of primary, it will refer to secondary device.
> And in second case secondary tells to self.
>
> So if its disabled by guest, it doesn't matter how guest transports it,
> either via CVQ or AQ.
> Its disabled.
> Why to re-invent command that exists on CVQ to AQ?
I can go into it but it's beside the point. I was just trying to help
you come up with use-cases. Don't like it - come up with your own ones.
> > OK, and Cornelia also said she thinks 64 is necessary.
> >
> > > So if we really want to cover variety of cases like [1] and some more
> > > complex nested cases, we better define, Device identifier as below,
> > > struct device_identifier {
> > > u8 id_length;
> > > u8 id[];/* variable length field
> > > };
> > > For implicitly grouped VFs of a PF, id can be 2 bytes.
> > > For more advance cases it can be a structure consist one or more
> > combination of (a) host id or controller id (b) PF BDF, (c) sf id (d) PASID
> > and
> > more.
> > >
> > > [1]
> > > https://www.kernel.org/doc/Documentation/networking/devlink/devlink-po
> > > rt.rst
> >
> > I'm fine with this too.
> >
> Since we aim for future proofing and flexibility, variable length id is
> better than constant u64. It covers the u64 case anyway.
>
> > --
> > MST
-
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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 03:06:16PM +, Parav Pandit wrote:
>
> > From: Michael S. Tsirkin
> > Sent: Tuesday, February 8, 2022 7:29 PM
> >
> > > Do we have a concrete example of a command that can be targeted for same
> > device and a target device, which requires differentiating their
> > destination? If
> > so, lets discuss and then it make sense to add for the well-defined use
> > case.
> >
> > So e.g. things like controlling NIC's MAC can reasonably be part of the same
> > device.
> A mac address of NIC can be programmed via the existing control VQ for the
> self.
Not if it's disabled for the guest.
> Lets come up with some other example.
Go ahead.
> > > So better to first come up with a valid use case and a device that
> > > supports it,
> > which needs a group.
> > > Otherwise a target id can be a long string of PCI device = :03:00.0
> > > or a
> > BDF or a VF number or a VF of a different PCI PF or a SF number 32-bit or SF
> > UUID or a VF on a remote DPU system or PCI device on transparent bridge or
> > something else.
> >
> > Well PASID is IIRC just 20 bit on express.
> There are off line discussion and some on the mailing list too (I don't have
> a link to few months/year old email),
> That PASID is being overloaded to identify the SF and process both. And I
> tend to agree to it.
>
> So I won't be surprised if a new SF function id takes different route than
> PASID.
> More below.
>
> > I find it unlikely that we'll need more
> > than 64 bit. Yes, it's hard to predict the future but just doing 16 bit
> > here seems
> > frankly like a premature optimization. UUID for a transient thing such as
> > SF just
> > seems unnecessary. 32 or 64 bit seem both acceptable.
> >
> > > Without knowing the grouping and a nonexistence device we shouldn't
> > complicate the commands.
> > >
> > > There are enough opcodes (64K) that can define new structure for more
> > complex devices.
> >
> > I think you are asking for a bit much frankly, it's up to you to build the
> > interface.
> I likely didn't understand above point.
>
> > Just like with code, if the design does not feel robust the bar is much
> > higher
> > even if one can not prove there's a locking problem.
>
> > Same here, this interface
> > design does not yet feel very robust yet - so either we build it in a way
> > that
> > seems robust based basically on our gut feeling, or actually spend time
> > predicting and addressing future use-cases to prove they can be addressed.
>
> > I dare say we've developed some intuition about what makes an extensible
> > interface and where things are likely to go here at the TC, so I wouldn't
> > discard
> > all feedback as unnecessary complication even if it does not always come
> > with
> > concrete use-case examples.
> I do not doubt your intuition.
> I am open to feedback, but we haven't really established at least single
> explicit grouping example and ask is to add some arbitrary reserved bytes for
> it.
>
> >From my DPU experience, over last 3 years, we have built SF and VF device on
> >parent PF-A, and their management interface on parent PF-B.
> On top of that it is expected to be uniquely indefinable in multi-host env,
> that brings the notion of multiple controllers by a single management device.
> And also make it work uniformly with same interface when parent and
> management device are same.
> You can see an extendible interface at [1].
>
> With this base line of [1], I do not agree that defining a u32
> device_identifier (to contain 20-bit PASID) will be sufficient in future.
>
OK, and Cornelia also said she thinks 64 is necessary.
> So if we really want to cover variety of cases like [1] and some more complex
> nested cases, we better define,
> Device identifier as below,
> struct device_identifier {
> u8 id_length;
> u8 id[];/* variable length field
> };
> For implicitly grouped VFs of a PF, id can be 2 bytes.
> For more advance cases it can be a structure consist one or more combination
> of (a) host id or controller id (b) PF BDF, (c) sf id (d) PASID and more.
>
> [1]
> https://www.kernel.org/doc/Documentation/networking/devlink/devlink-port.rst
I'm fine with this too.
--
MST
-
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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 03:35:58PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Tuesday, February 8, 2022 8:56 PM > > > > I might have been a bit too vague about what I had been thinking > > > about. Let's do a sketch (intentionally without concrete sizes): > > > > > > +---+ > > > | command | > > > +---+ > > > | target type (0 - self, 1 - dev id, 2 - group id, ... | > > > +---+ > > > | dev id| > > > +---+ > > > | group id | > > > +---+ > > > | command-specific data | > > > +---+ > > > | response part | > > > +---+ > > > > > > 'dev id' would be valid for 'target type' == 1, 'group id' would be > > > valid for 'target type' == 2. Alternatively, 'dev id' and 'group id' > > > could be a single 'target id' field; if there's nothing better to use, > > > it can simply contain a uuid. > > > > I am not sure why do we have both dev id and group id. > > They are never used together right? > > Maybe just have an id length field if we can't agree on how much space to > > reserve. > This is what I propose in a previous email. > A device id can be duplicate in different groups. > So to build hierarchy group id will be desired. > > So id[] array can contain nested one or multiple fields. I guess nesting is for when there's like an SF within a VF? -- MST - 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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 03:33:36PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Tuesday, February 8, 2022 8:58 PM > > On Tue, Feb 08, 2022 at 03:11:28PM +, Parav Pandit wrote: > > > > > > > From: Cornelia Huck > > > > Sent: Tuesday, February 8, 2022 8:29 PM > > > > > > > very specific (essentially PCI-specific), we need to at least answer > > > > the question "how could this work for things that are not PCI?" > > > Why would one want to execute PCI VF MSI-X configuration command for a > > non PCI device? > > > > E.g. there were proposals to add msi support to virtio-mmio. > > What prevents mmio specific command say mmio_dev_interrupt_config()? > Which fields do overlap with msix config of pci vf? > How is the mmio device identified? if mmio starts supporting msi-x then # of vectors can reasonably be specified in exactly the same way. -- MST - 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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08 2022, "Michael S. Tsirkin" wrote: > On Tue, Feb 08, 2022 at 03:59:13PM +0100, Cornelia Huck wrote: >> On Tue, Feb 08 2022, "Michael S. Tsirkin" wrote: >> >> > On Tue, Feb 08, 2022 at 01:32:12PM +, Parav Pandit wrote: >> >> >> >> > From: Cornelia Huck >> >> > Sent: Tuesday, February 8, 2022 6:50 PM >> >> > >> >> > On Tue, Feb 08 2022, Parav Pandit wrote: >> >> > >> >> > >> From: Michael S. Tsirkin >> >> > >> Sent: Tuesday, February 8, 2022 12:13 PM >> >> > > >> >> > >> On Tue, Feb 08, 2022 at 06:25:41AM +, Parav Pandit wrote: >> >> > >> > >> >> > >> > > From: Michael S. Tsirkin >> >> > >> > > Sent: Monday, February 7, 2022 4:09 PM >> >> > >> > > >> >> > >> > > Next, trying to think about scalable iov extensions. So we will >> >> > >> > > have groups of VFs and then SFs as the next level. >> >> > >> > > How does one differentiate between the two? >> >> > >> > > Maybe reserve a field for "destination type"? >> >> > >> > > >> >> > >> > We already discussed this in v2. >> >> > >> > SF will have different identification than 16-bits. And no one >> >> > >> > knows what >> >> > >> that would be. >> >> > >> > We just cannot reserve some arbitrary bytes for unknown. >> >> > >> > You suggested in v2 to reserve 4 bytes for sf_id, and I explained >> >> > >> > you that 4 >> >> > >> bytes may not be enough. >> >> > >> > >> >> > >> > Whether SFs are on top of VFs or SFs are on top of PFs or both is >> >> > >> > completely >> >> > >> different spec. >> >> > >> > Whether PF will manage SFs of the VFs or it will be done nested >> >> > >> > manner by >> >> > >> VF etc is a completely different discussion than what is being >> >> > >> proposed here. >> >> > >> > Whether PF will manage the SF is yet another big question. We work >> >> > >> > with >> >> > >> users and they dislike this. >> >> > >> > To address it, some OS has a different management interface (not >> >> > >> > visible to >> >> > >> PF) for SF life cycle even though SFs are anchored on a PF. >> >> > >> > >> >> > >> > So SF/iov extension discussion has long way to go for community to >> >> > >> > first >> >> > >> understand the use cases before crafting some extension. >> >> > >> > >> >> > >> > So lets not complicate and mix things here for a blurry definition >> >> > >> > of scalable >> >> > >> iov/sf extension. >> >> > >> >> >> > >> Some reserved bytes won't hurt. 2 bytes for type gives us 64k types, >> >> > >> sounds like that should be enough. >> >> > > It doesn't stop there. >> >> > > Mentioning some destination type, interrupt type, etc also requires >> >> > > reserving >> >> > bytes for different device id type, interrupt type and more. >> >> > > We past this stage long ago after discussing this in v1 at [1]. >> >> > > It is just better and cleaner to define a different structure to >> >> > > describe SF/iov >> >> > and its configuration. >> >> > >> >> > I have the feeling that we might be overcomplicating this. We have some >> >> > groups of targets (a device, a group, that more complicated SF thingy), >> >> > and we >> >> > want to distinguish between them. That's easy enough to cover via some >> >> > kind of >> >> > enum-equivalent (0 == same dev, 1 == target a dev id, 2 == target a >> >> > group id, 3 >> >> > == multi-layer target) and some spec how 1 and 2 should look like (as >> >> > I'd expect >> >> > them to be common for many different things). >> >> Do we have a concrete example of a command that can be targeted for same >> >> device and a target device, which requires differentiating their >> >> destination? If so, lets discuss and then it make sense to add for the >> >> well-defined use case. >> > >> > So e.g. things like controlling NIC's MAC can reasonably be part of >> > the same device. >> >> Yes, that would be an example. >> >> I might have been a bit too vague about what I had been thinking >> about. Let's do a sketch (intentionally without concrete sizes): >> >> +---+ >> | command | >> +---+ >> | target type (0 - self, 1 - dev id, 2 - group id, ... | >> +---+ >> | dev id| >> +---+ >> | group id | >> +---+ >> | command-specific data | >> +---+ >> | response part | >> +---+ >> >> 'dev id' would be valid for 'target type' == 1, 'group id' would be >> valid for 'target type' == 2. Alternatively, 'dev id' and 'group id' >> could be a single 'target id' field; if there's nothing better to use, >> it can simply contain a uuid. > >
[virtio-dev] Re: [PATCH v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 03:11:28PM +, Parav Pandit wrote: > > > From: Cornelia Huck > > Sent: Tuesday, February 8, 2022 8:29 PM > > > very specific (essentially PCI-specific), we need to at least answer the > > question > > "how could this work for things that are not PCI?" > Why would one want to execute PCI VF MSI-X configuration command for a non > PCI device? E.g. there were proposals to add msi support to virtio-mmio. -- MST - 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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 03:59:13PM +0100, Cornelia Huck wrote: > On Tue, Feb 08 2022, "Michael S. Tsirkin" wrote: > > > On Tue, Feb 08, 2022 at 01:32:12PM +, Parav Pandit wrote: > >> > >> > From: Cornelia Huck > >> > Sent: Tuesday, February 8, 2022 6:50 PM > >> > > >> > On Tue, Feb 08 2022, Parav Pandit wrote: > >> > > >> > >> From: Michael S. Tsirkin > >> > >> Sent: Tuesday, February 8, 2022 12:13 PM > >> > > > >> > >> On Tue, Feb 08, 2022 at 06:25:41AM +, Parav Pandit wrote: > >> > >> > > >> > >> > > From: Michael S. Tsirkin > >> > >> > > Sent: Monday, February 7, 2022 4:09 PM > >> > >> > > > >> > >> > > Next, trying to think about scalable iov extensions. So we will > >> > >> > > have groups of VFs and then SFs as the next level. > >> > >> > > How does one differentiate between the two? > >> > >> > > Maybe reserve a field for "destination type"? > >> > >> > > > >> > >> > We already discussed this in v2. > >> > >> > SF will have different identification than 16-bits. And no one > >> > >> > knows what > >> > >> that would be. > >> > >> > We just cannot reserve some arbitrary bytes for unknown. > >> > >> > You suggested in v2 to reserve 4 bytes for sf_id, and I explained > >> > >> > you that 4 > >> > >> bytes may not be enough. > >> > >> > > >> > >> > Whether SFs are on top of VFs or SFs are on top of PFs or both is > >> > >> > completely > >> > >> different spec. > >> > >> > Whether PF will manage SFs of the VFs or it will be done nested > >> > >> > manner by > >> > >> VF etc is a completely different discussion than what is being > >> > >> proposed here. > >> > >> > Whether PF will manage the SF is yet another big question. We work > >> > >> > with > >> > >> users and they dislike this. > >> > >> > To address it, some OS has a different management interface (not > >> > >> > visible to > >> > >> PF) for SF life cycle even though SFs are anchored on a PF. > >> > >> > > >> > >> > So SF/iov extension discussion has long way to go for community to > >> > >> > first > >> > >> understand the use cases before crafting some extension. > >> > >> > > >> > >> > So lets not complicate and mix things here for a blurry definition > >> > >> > of scalable > >> > >> iov/sf extension. > >> > >> > >> > >> Some reserved bytes won't hurt. 2 bytes for type gives us 64k types, > >> > >> sounds like that should be enough. > >> > > It doesn't stop there. > >> > > Mentioning some destination type, interrupt type, etc also requires > >> > > reserving > >> > bytes for different device id type, interrupt type and more. > >> > > We past this stage long ago after discussing this in v1 at [1]. > >> > > It is just better and cleaner to define a different structure to > >> > > describe SF/iov > >> > and its configuration. > >> > > >> > I have the feeling that we might be overcomplicating this. We have some > >> > groups of targets (a device, a group, that more complicated SF thingy), > >> > and we > >> > want to distinguish between them. That's easy enough to cover via some > >> > kind of > >> > enum-equivalent (0 == same dev, 1 == target a dev id, 2 == target a > >> > group id, 3 > >> > == multi-layer target) and some spec how 1 and 2 should look like (as > >> > I'd expect > >> > them to be common for many different things). > >> Do we have a concrete example of a command that can be targeted for same > >> device and a target device, which requires differentiating their > >> destination? If so, lets discuss and then it make sense to add for the > >> well-defined use case. > > > > So e.g. things like controlling NIC's MAC can reasonably be part of > > the same device. > > Yes, that would be an example. > > I might have been a bit too vague about what I had been thinking > about. Let's do a sketch (intentionally without concrete sizes): > > +---+ > | command | > +---+ > | target type (0 - self, 1 - dev id, 2 - group id, ... | > +---+ > | dev id| > +---+ > | group id | > +---+ > | command-specific data | > +---+ > | response part | > +---+ > > 'dev id' would be valid for 'target type' == 1, 'group id' would be > valid for 'target type' == 2. Alternatively, 'dev id' and 'group id' > could be a single 'target id' field; if there's nothing better to use, > it can simply contain a uuid. I am not sure why do we have both dev id and group id. They are never used together right? Maybe just have an id length field if we can't agree on how much space
Re: [virtio-dev] RE: [PATCH v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08 2022, Parav Pandit wrote: >> From: Cornelia Huck >> Sent: Tuesday, February 8, 2022 8:29 PM > >> very specific (essentially PCI-specific), we need to at least answer the >> question >> "how could this work for things that are not PCI?" > Why would one want to execute PCI VF MSI-X configuration command for a non > PCI device? Well, that obviously does not make sense! But we also obviously still want to be able to target admin commands in general to non-PCI devices. - 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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08 2022, "Michael S. Tsirkin" wrote: > On Tue, Feb 08, 2022 at 01:32:12PM +, Parav Pandit wrote: >> >> > From: Cornelia Huck >> > Sent: Tuesday, February 8, 2022 6:50 PM >> > >> > On Tue, Feb 08 2022, Parav Pandit wrote: >> > >> > >> From: Michael S. Tsirkin >> > >> Sent: Tuesday, February 8, 2022 12:13 PM >> > > >> > >> On Tue, Feb 08, 2022 at 06:25:41AM +, Parav Pandit wrote: >> > >> > >> > >> > > From: Michael S. Tsirkin >> > >> > > Sent: Monday, February 7, 2022 4:09 PM >> > >> > > >> > >> > > Next, trying to think about scalable iov extensions. So we will >> > >> > > have groups of VFs and then SFs as the next level. >> > >> > > How does one differentiate between the two? >> > >> > > Maybe reserve a field for "destination type"? >> > >> > > >> > >> > We already discussed this in v2. >> > >> > SF will have different identification than 16-bits. And no one >> > >> > knows what >> > >> that would be. >> > >> > We just cannot reserve some arbitrary bytes for unknown. >> > >> > You suggested in v2 to reserve 4 bytes for sf_id, and I explained >> > >> > you that 4 >> > >> bytes may not be enough. >> > >> > >> > >> > Whether SFs are on top of VFs or SFs are on top of PFs or both is >> > >> > completely >> > >> different spec. >> > >> > Whether PF will manage SFs of the VFs or it will be done nested >> > >> > manner by >> > >> VF etc is a completely different discussion than what is being proposed >> > >> here. >> > >> > Whether PF will manage the SF is yet another big question. We work >> > >> > with >> > >> users and they dislike this. >> > >> > To address it, some OS has a different management interface (not >> > >> > visible to >> > >> PF) for SF life cycle even though SFs are anchored on a PF. >> > >> > >> > >> > So SF/iov extension discussion has long way to go for community to >> > >> > first >> > >> understand the use cases before crafting some extension. >> > >> > >> > >> > So lets not complicate and mix things here for a blurry definition >> > >> > of scalable >> > >> iov/sf extension. >> > >> >> > >> Some reserved bytes won't hurt. 2 bytes for type gives us 64k types, >> > >> sounds like that should be enough. >> > > It doesn't stop there. >> > > Mentioning some destination type, interrupt type, etc also requires >> > > reserving >> > bytes for different device id type, interrupt type and more. >> > > We past this stage long ago after discussing this in v1 at [1]. >> > > It is just better and cleaner to define a different structure to >> > > describe SF/iov >> > and its configuration. >> > >> > I have the feeling that we might be overcomplicating this. We have some >> > groups of targets (a device, a group, that more complicated SF thingy), >> > and we >> > want to distinguish between them. That's easy enough to cover via some >> > kind of >> > enum-equivalent (0 == same dev, 1 == target a dev id, 2 == target a group >> > id, 3 >> > == multi-layer target) and some spec how 1 and 2 should look like (as I'd >> > expect >> > them to be common for many different things). >> Do we have a concrete example of a command that can be targeted for same >> device and a target device, which requires differentiating their >> destination? If so, lets discuss and then it make sense to add for the >> well-defined use case. > > So e.g. things like controlling NIC's MAC can reasonably be part of > the same device. Yes, that would be an example. I might have been a bit too vague about what I had been thinking about. Let's do a sketch (intentionally without concrete sizes): +---+ | command | +---+ | target type (0 - self, 1 - dev id, 2 - group id, ... | +---+ | dev id| +---+ | group id | +---+ | command-specific data | +---+ | response part | +---+ 'dev id' would be valid for 'target type' == 1, 'group id' would be valid for 'target type' == 2. Alternatively, 'dev id' and 'group id' could be a single 'target id' field; if there's nothing better to use, it can simply contain a uuid. > >> > The SF thingy can be covered >> > by 3, and we'll probably want to make that one command-specific, as the >> > whole >> > setup does not have enough things we can standardize on. >> This comment has underlying assumption that there are nested layers. And >> that assumption may not be true at all. >> At least for iov extension of intel and SF of Mellanox (already open source >> for 1+ year) doesn't have the nested layers
[virtio-dev] Re: [PATCH v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 10:34:24AM +0200, Max Gurtovoy wrote: > "Admin virtqueue is one of the management interface that used to send > administrative > commands to manipulate various features of the *device* and/or to manipulate > various features, if possible, of *another device* within the same group" > > It's not only for groups. First this sentence is unclear what does "to manipulate" refer to? What is done to manipulate these things? I think the vq is to send commands. The commands are to manipulate etc. So pls cut this up and use shorter sentences please. And I think the vq is useful mostly for groups. But yes, thinkably if admin commands are submitted via e.g. MMIO then they can refer to the same device where they are sent, not to a group. And this is exactly why Cornelia's suggesting a way to differentiate. We should not need a completely separate command just because it's going to the same device and not to another one. -- MST - 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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 01:32:12PM +, Parav Pandit wrote: > > > From: Cornelia Huck > > Sent: Tuesday, February 8, 2022 6:50 PM > > > > On Tue, Feb 08 2022, Parav Pandit wrote: > > > > >> From: Michael S. Tsirkin > > >> Sent: Tuesday, February 8, 2022 12:13 PM > > > > > >> On Tue, Feb 08, 2022 at 06:25:41AM +, Parav Pandit wrote: > > >> > > > >> > > From: Michael S. Tsirkin > > >> > > Sent: Monday, February 7, 2022 4:09 PM > > >> > > > > >> > > Next, trying to think about scalable iov extensions. So we will > > >> > > have groups of VFs and then SFs as the next level. > > >> > > How does one differentiate between the two? > > >> > > Maybe reserve a field for "destination type"? > > >> > > > > >> > We already discussed this in v2. > > >> > SF will have different identification than 16-bits. And no one > > >> > knows what > > >> that would be. > > >> > We just cannot reserve some arbitrary bytes for unknown. > > >> > You suggested in v2 to reserve 4 bytes for sf_id, and I explained > > >> > you that 4 > > >> bytes may not be enough. > > >> > > > >> > Whether SFs are on top of VFs or SFs are on top of PFs or both is > > >> > completely > > >> different spec. > > >> > Whether PF will manage SFs of the VFs or it will be done nested > > >> > manner by > > >> VF etc is a completely different discussion than what is being proposed > > >> here. > > >> > Whether PF will manage the SF is yet another big question. We work > > >> > with > > >> users and they dislike this. > > >> > To address it, some OS has a different management interface (not > > >> > visible to > > >> PF) for SF life cycle even though SFs are anchored on a PF. > > >> > > > >> > So SF/iov extension discussion has long way to go for community to > > >> > first > > >> understand the use cases before crafting some extension. > > >> > > > >> > So lets not complicate and mix things here for a blurry definition > > >> > of scalable > > >> iov/sf extension. > > >> > > >> Some reserved bytes won't hurt. 2 bytes for type gives us 64k types, > > >> sounds like that should be enough. > > > It doesn't stop there. > > > Mentioning some destination type, interrupt type, etc also requires > > > reserving > > bytes for different device id type, interrupt type and more. > > > We past this stage long ago after discussing this in v1 at [1]. > > > It is just better and cleaner to define a different structure to describe > > > SF/iov > > and its configuration. > > > > I have the feeling that we might be overcomplicating this. We have some > > groups of targets (a device, a group, that more complicated SF thingy), and > > we > > want to distinguish between them. That's easy enough to cover via some kind > > of > > enum-equivalent (0 == same dev, 1 == target a dev id, 2 == target a group > > id, 3 > > == multi-layer target) and some spec how 1 and 2 should look like (as I'd > > expect > > them to be common for many different things). > Do we have a concrete example of a command that can be targeted for same > device and a target device, which requires differentiating their destination? > If so, lets discuss and then it make sense to add for the well-defined use > case. So e.g. things like controlling NIC's MAC can reasonably be part of the same device. > > The SF thingy can be covered > > by 3, and we'll probably want to make that one command-specific, as the > > whole > > setup does not have enough things we can standardize on. > This comment has underlying assumption that there are nested layers. And that > assumption may not be true at all. > At least for iov extension of intel and SF of Mellanox (already open source > for 1+ year) doesn't have the nested layers as today as far as I understand. > > > > Does that make sense? This standardizes the simpler setups, and gives enough > > flexibility for the more complicated ones. If we have another simpler setup > > in > > the future, it can become type 4, 5, etc. > I feel that without a use case we are over complicating the commands by > introducing group/target id etc. > > So better to first come up with a valid use case and a device that supports > it, which needs a group. > Otherwise a target id can be a long string of PCI device = :03:00.0 or a > BDF or a VF number or a VF of a different PCI PF or a SF number 32-bit or SF > UUID or a VF on a remote DPU system or PCI device on transparent bridge or > something else. Well PASID is IIRC just 20 bit on express. I find it unlikely that we'll need more than 64 bit. Yes, it's hard to predict the future but just doing 16 bit here seems frankly like a premature optimization. UUID for a transient thing such as SF just seems unnecessary. 32 or 64 bit seem both acceptable. > Without knowing the grouping and a nonexistence device we shouldn't > complicate the commands. > > There are enough opcodes (64K) that can define new structure for more complex > devices. I think you are asking for a bit much frankly, it's up to you to build the
[virtio-dev] RE: [PATCH v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08 2022, Parav Pandit wrote: >> From: Michael S. Tsirkin >> Sent: Tuesday, February 8, 2022 12:13 PM > >> On Tue, Feb 08, 2022 at 06:25:41AM +, Parav Pandit wrote: >> > >> > > From: Michael S. Tsirkin >> > > Sent: Monday, February 7, 2022 4:09 PM >> > > >> > > Next, trying to think about scalable iov extensions. So we will have >> > > groups of VFs and then SFs as the next level. >> > > How does one differentiate between the two? >> > > Maybe reserve a field for "destination type"? >> > > >> > We already discussed this in v2. >> > SF will have different identification than 16-bits. And no one knows what >> that would be. >> > We just cannot reserve some arbitrary bytes for unknown. >> > You suggested in v2 to reserve 4 bytes for sf_id, and I explained you that >> > 4 >> bytes may not be enough. >> > >> > Whether SFs are on top of VFs or SFs are on top of PFs or both is >> > completely >> different spec. >> > Whether PF will manage SFs of the VFs or it will be done nested manner by >> VF etc is a completely different discussion than what is being proposed here. >> > Whether PF will manage the SF is yet another big question. We work with >> users and they dislike this. >> > To address it, some OS has a different management interface (not visible to >> PF) for SF life cycle even though SFs are anchored on a PF. >> > >> > So SF/iov extension discussion has long way to go for community to first >> understand the use cases before crafting some extension. >> > >> > So lets not complicate and mix things here for a blurry definition of >> > scalable >> iov/sf extension. >> >> Some reserved bytes won't hurt. 2 bytes for type gives us 64k types, sounds >> like >> that should be enough. > It doesn't stop there. > Mentioning some destination type, interrupt type, etc also requires reserving > bytes for different device id type, interrupt type and more. > We past this stage long ago after discussing this in v1 at [1]. > It is just better and cleaner to define a different structure to describe > SF/iov and its configuration. I have the feeling that we might be overcomplicating this. We have some groups of targets (a device, a group, that more complicated SF thingy), and we want to distinguish between them. That's easy enough to cover via some kind of enum-equivalent (0 == same dev, 1 == target a dev id, 2 == target a group id, 3 == multi-layer target) and some spec how 1 and 2 should look like (as I'd expect them to be common for many different things). The SF thingy can be covered by 3, and we'll probably want to make that one command-specific, as the whole setup does not have enough things we can standardize on. Does that make sense? This standardizes the simpler setups, and gives enough flexibility for the more complicated ones. If we have another simpler setup in the future, it can become type 4, 5, etc. - 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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08 2022, Max Gurtovoy wrote:
> On 2/8/2022 8:45 AM, Michael S. Tsirkin wrote:
>> On Tue, Feb 08, 2022 at 02:41:44AM +0200, Max Gurtovoy wrote:
>>> On 2/7/2022 6:18 PM, Michael S. Tsirkin wrote:
On Mon, Feb 07, 2022 at 04:58:19PM +0200, Max Gurtovoy wrote:
> On 2/7/2022 12:39 PM, Michael S. Tsirkin wrote:
>> On Thu, Feb 03, 2022 at 09:57:13AM +0200, Max Gurtovoy wrote:
>>> +\begin{lstlisting}
>>> +struct virtio_admin_cmd {
>>> +/* Device-readable part */
>>> +u16 command;
>>> +u8 command_specific_data[];
>>> +
>>> +/* Device-writable part */
>>> +u8 status;
>>> +u8 command_specific_error;
>>> +u8 command_specific_result[];
>>> +};
>>> +\end{lstlisting}
>> ok this abstraction is an improvement, thanks!
>>
>> What I'd like to see is moving a bit more format to this generic
>> structure.
>>
>>From what I could gather, some commands affect a group as a whole, and
>> some commands just a single member of the group. We could have a
>> "destination" field for that, and a special "all of the group"
>> destination for commands affecting the whole group.
>>
>>
>> Next, trying to think about scalable iov extensions. So we
>> will have groups of VFs and then SFs as the next level.
>> How does one differentiate between the two?
>> Maybe reserve a field for "destination type"?
> For now we have only a PCI group that composed of VFs and the PF.
>
> What you suggest, IMO is a definition of a generic virtio group/subsystem
> that I've mentioned in the discussion of V1.
>
> Once we have virtio group - it should have a group id and them the admin
> command can have a field calld group_id for commands that are targeted to
> the whole group.
>
> Some commands are referring to a specific device in the group so only a
> vdev_id is needed.
>
> Some commands are even targeted to the same device to query some info (we
> have examples in this series for that), so in this case there is no need
> for
> vdev_id nor group_id.
>
> So I'm sure sure we can improve common virtio_admin_cmd structure to have
> these attributes since they are not mandatory because of the reasons I've
> mentioned.
I'm not sure I understand 100%, but try to address in the next
revision and we'll discuss.
>>> I meant to say that I'm *not* sure we can improve the common structure...
>>>
>>> It was a typo.
>>>
>>> And I don't understand why this info can't be in the command_specific_data
>>> because of all the reasons I mentioned above.
>> It can, but as declared admin commands are there to handle
>> groups of VFs, so let's standardize how they refer to groups.
>
> "Admin virtqueue is one of the management interface that used to send
> administrative
> commands to manipulate various features of the *device* and/or to manipulate
> various features, if possible, of *another device* within the same group"
>
> It's not only for groups.
What I don't understand: Why can't we simply add target information to
the common definition? If a certain command doesn't need that target
information, it is simply ignored.
The benefit of reserving a field for target information and specifying
how groups and devices are supposed to be addressed is that not every
command will have to roll their own.
-
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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 02:41:44AM +0200, Max Gurtovoy wrote:
>
> On 2/7/2022 6:18 PM, Michael S. Tsirkin wrote:
> > On Mon, Feb 07, 2022 at 04:58:19PM +0200, Max Gurtovoy wrote:
> > > On 2/7/2022 12:39 PM, Michael S. Tsirkin wrote:
> > > > On Thu, Feb 03, 2022 at 09:57:13AM +0200, Max Gurtovoy wrote:
> > > > > +\begin{lstlisting}
> > > > > +struct virtio_admin_cmd {
> > > > > +/* Device-readable part */
> > > > > +u16 command;
> > > > > +u8 command_specific_data[];
> > > > > +
> > > > > +/* Device-writable part */
> > > > > +u8 status;
> > > > > +u8 command_specific_error;
> > > > > +u8 command_specific_result[];
> > > > > +};
> > > > > +\end{lstlisting}
> > > > ok this abstraction is an improvement, thanks!
> > > >
> > > > What I'd like to see is moving a bit more format to this generic
> > > > structure.
> > > >
> > > > From what I could gather, some commands affect a group as a whole, and
> > > > some commands just a single member of the group. We could have a
> > > > "destination" field for that, and a special "all of the group"
> > > > destination for commands affecting the whole group.
> > > >
> > > >
> > > > Next, trying to think about scalable iov extensions. So we
> > > > will have groups of VFs and then SFs as the next level.
> > > > How does one differentiate between the two?
> > > > Maybe reserve a field for "destination type"?
> > > For now we have only a PCI group that composed of VFs and the PF.
> > >
> > > What you suggest, IMO is a definition of a generic virtio group/subsystem
> > > that I've mentioned in the discussion of V1.
> > >
> > > Once we have virtio group - it should have a group id and them the admin
> > > command can have a field calld group_id for commands that are targeted to
> > > the whole group.
> > >
> > > Some commands are referring to a specific device in the group so only a
> > > vdev_id is needed.
> > >
> > > Some commands are even targeted to the same device to query some info (we
> > > have examples in this series for that), so in this case there is no need
> > > for
> > > vdev_id nor group_id.
> > >
> > > So I'm sure sure we can improve common virtio_admin_cmd structure to have
> > > these attributes since they are not mandatory because of the reasons I've
> > > mentioned.
> > I'm not sure I understand 100%, but try to address in the next
> > revision and we'll discuss.
>
> I meant to say that I'm *not* sure we can improve the common structure...
>
> It was a typo.
>
> And I don't understand why this info can't be in the command_specific_data
> because of all the reasons I mentioned above.
It can, but as declared admin commands are there to handle
groups of VFs, so let's standardize how they refer to groups.
> >
> > > > The point of all this is to allow making sense of commands and
> > > > e.g. virtualizing them for nested virt without necessarily
> > > > knowing all of the detail about the specific command.
> > > I don't understand this, sorry.
> > Basically try to move stuff into generic format so it's possible
> > to understand things without knowing detail of the command.
>
> But we don't develop a networking protocol here. The management device is
> not sending packets towards its managed devices, right ?
>
> This is an interface for a specific device that can manage others but also
> manage itself.
>
> We didn't introduce a notion of broadcasting admin commands for other
> devices.
It's all entities communicating by message passing whether
you call these "commands", "packets" or whatever.
--
MST
-
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 v3 1/4] Add virtio Admin virtqueue
On Tue, Feb 08, 2022 at 06:25:41AM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Monday, February 7, 2022 4:09 PM > > > > Next, trying to think about scalable iov extensions. So we will have groups > > of > > VFs and then SFs as the next level. > > How does one differentiate between the two? > > Maybe reserve a field for "destination type"? > > > We already discussed this in v2. > SF will have different identification than 16-bits. And no one knows what > that would be. > We just cannot reserve some arbitrary bytes for unknown. > You suggested in v2 to reserve 4 bytes for sf_id, and I explained you that 4 > bytes may not be enough. > > Whether SFs are on top of VFs or SFs are on top of PFs or both is completely > different spec. > Whether PF will manage SFs of the VFs or it will be done nested manner by VF > etc is a completely different discussion than what is being proposed here. > Whether PF will manage the SF is yet another big question. We work with users > and they dislike this. > To address it, some OS has a different management interface (not visible to > PF) for SF life cycle even though SFs are anchored on a PF. > > So SF/iov extension discussion has long way to go for community to first > understand the use cases before crafting some extension. > > So lets not complicate and mix things here for a blurry definition of > scalable iov/sf extension. Some reserved bytes won't hurt. 2 bytes for type gives us 64k types, sounds like that should be enough. -- MST - 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 v3 1/4] Add virtio Admin virtqueue
On Mon, Feb 07, 2022 at 04:08:41PM +0100, Cornelia Huck wrote: > On Mon, Feb 07 2022, Max Gurtovoy wrote: > > > On 2/7/2022 1:51 PM, Cornelia Huck wrote: > >> On Mon, Feb 07 2022, "Michael S. Tsirkin" wrote: > >> > >>> On Mon, Feb 07, 2022 at 12:14:33PM +0200, Max Gurtovoy wrote: > On 2/3/2022 3:09 PM, Cornelia Huck wrote: > > On Thu, Feb 03 2022, Max Gurtovoy wrote: > >> +commands to manipulate various features of the device and/or to > >> manipulate > >> +various features, if possible, of another device within the same > >> group (e.g. PCI VFs > > Maybe add > > > > "Which devices are actually considered a group is transport specific." > > > > ? > Not sure we want to restrict ourselves for that. > >>> do restrict this please, if we want to extend the scope we can > >>> always do that down the road. > >> I'm also not sure how grouping can _not_ be transport specific... the > >> PF/VF example is obviously a pci thing; for ccw, in a non-virtio > >> context, there's sometimes the concept of some subchannels/devices being > >> grouped together with no clear hierarchy, and for mmio, I don't really > >> have an idea how "grouping" might work there. > > > > Yes today it's transport specific. > > > > But if one day there will be a definition for virtio fabric (over > > TCP/RDMA) it might not be true. > > I don't think that is contadictory; we can simply extend the meaning of > what "grouping" means when needed. Yes but as long it is transport specific I don't see why not call this out. -- MST - 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 v3 1/4] Add virtio Admin virtqueue
On Mon, Feb 07, 2022 at 04:58:19PM +0200, Max Gurtovoy wrote:
>
> On 2/7/2022 12:39 PM, Michael S. Tsirkin wrote:
> > On Thu, Feb 03, 2022 at 09:57:13AM +0200, Max Gurtovoy wrote:
> > > +\begin{lstlisting}
> > > +struct virtio_admin_cmd {
> > > +/* Device-readable part */
> > > +u16 command;
> > > +u8 command_specific_data[];
> > > +
> > > +/* Device-writable part */
> > > +u8 status;
> > > +u8 command_specific_error;
> > > +u8 command_specific_result[];
> > > +};
> > > +\end{lstlisting}
> > ok this abstraction is an improvement, thanks!
> >
> > What I'd like to see is moving a bit more format to this generic structure.
> >
> > From what I could gather, some commands affect a group as a whole, and
> > some commands just a single member of the group. We could have a
> > "destination" field for that, and a special "all of the group"
> > destination for commands affecting the whole group.
> >
> >
> > Next, trying to think about scalable iov extensions. So we
> > will have groups of VFs and then SFs as the next level.
> > How does one differentiate between the two?
> > Maybe reserve a field for "destination type"?
>
> For now we have only a PCI group that composed of VFs and the PF.
>
> What you suggest, IMO is a definition of a generic virtio group/subsystem
> that I've mentioned in the discussion of V1.
>
> Once we have virtio group - it should have a group id and them the admin
> command can have a field calld group_id for commands that are targeted to
> the whole group.
>
> Some commands are referring to a specific device in the group so only a
> vdev_id is needed.
>
> Some commands are even targeted to the same device to query some info (we
> have examples in this series for that), so in this case there is no need for
> vdev_id nor group_id.
>
> So I'm sure sure we can improve common virtio_admin_cmd structure to have
> these attributes since they are not mandatory because of the reasons I've
> mentioned.
I'm not sure I understand 100%, but try to address in the next
revision and we'll discuss.
> >
> > The point of all this is to allow making sense of commands and
> > e.g. virtualizing them for nested virt without necessarily
> > knowing all of the detail about the specific command.
> I don't understand this, sorry.
Basically try to move stuff into generic format so it's possible
to understand things without knowing detail of the command.
--
MST
-
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 v3 1/4] Add virtio Admin virtqueue
On Mon, Feb 07 2022, Max Gurtovoy wrote: > On 2/7/2022 1:51 PM, Cornelia Huck wrote: >> On Mon, Feb 07 2022, "Michael S. Tsirkin" wrote: >> >>> On Mon, Feb 07, 2022 at 12:14:33PM +0200, Max Gurtovoy wrote: On 2/3/2022 3:09 PM, Cornelia Huck wrote: > On Thu, Feb 03 2022, Max Gurtovoy wrote: >> +commands to manipulate various features of the device and/or to >> manipulate >> +various features, if possible, of another device within the same group >> (e.g. PCI VFs > Maybe add > > "Which devices are actually considered a group is transport specific." > > ? Not sure we want to restrict ourselves for that. >>> do restrict this please, if we want to extend the scope we can >>> always do that down the road. >> I'm also not sure how grouping can _not_ be transport specific... the >> PF/VF example is obviously a pci thing; for ccw, in a non-virtio >> context, there's sometimes the concept of some subchannels/devices being >> grouped together with no clear hierarchy, and for mmio, I don't really >> have an idea how "grouping" might work there. > > Yes today it's transport specific. > > But if one day there will be a definition for virtio fabric (over > TCP/RDMA) it might not be true. I don't think that is contadictory; we can simply extend the meaning of what "grouping" means when needed. - 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 v3 1/4] Add virtio Admin virtqueue
On Mon, Feb 07 2022, "Michael S. Tsirkin" wrote: > On Mon, Feb 07, 2022 at 12:14:33PM +0200, Max Gurtovoy wrote: >> >> On 2/3/2022 3:09 PM, Cornelia Huck wrote: >> > On Thu, Feb 03 2022, Max Gurtovoy wrote: >> > > +commands to manipulate various features of the device and/or to >> > > manipulate >> > > +various features, if possible, of another device within the same group >> > > (e.g. PCI VFs >> > Maybe add >> > >> > "Which devices are actually considered a group is transport specific." >> > >> > ? >> >> Not sure we want to restrict ourselves for that. > > do restrict this please, if we want to extend the scope we can > always do that down the road. I'm also not sure how grouping can _not_ be transport specific... the PF/VF example is obviously a pci thing; for ccw, in a non-virtio context, there's sometimes the concept of some subchannels/devices being grouped together with no clear hierarchy, and for mmio, I don't really have an idea how "grouping" might work there. >> > > +When VIRTIO_F_ADMIN_VQ is negotiated with the device, driver will send >> > > all admin commands >> > > +through the admin virtqueue. >> > That sounds a bit like the driver might use an alternative interface for >> > the admin commands as well? What about >> >> Yes if there will be an alternative for AQ and this feature bit will not be >> negotiated so the driver will use a different channel. >> >> This was explicitly discussed in previous versions. >> >> What is the issue with this assumption ? > > it's not an issue down the road but we want to be clear > that right now that is the only way, to make sure > reader does not waste time looking for more ways > in the spec. maybe just say so. Yes, we should be explicit that admin commands are independent of the conduit they are using, and that currently the only conduit is the admin vq. Someone reading the spec does not know about previous discussions on the mailing list. Maybe reorder this? First have a section where the admin commands are defined, and then have a section that lists the different channels admin commands can use, where the admin vq is the only one currently supported? - 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 v3 1/4] Add virtio Admin virtqueue
On Thu, Feb 03, 2022 at 09:57:13AM +0200, Max Gurtovoy wrote:
> +\begin{lstlisting}
> +struct virtio_admin_cmd {
> +/* Device-readable part */
> +u16 command;
> +u8 command_specific_data[];
> +
> +/* Device-writable part */
> +u8 status;
> +u8 command_specific_error;
> +u8 command_specific_result[];
> +};
> +\end{lstlisting}
ok this abstraction is an improvement, thanks!
What I'd like to see is moving a bit more format to this generic structure.
>From what I could gather, some commands affect a group as a whole, and
some commands just a single member of the group. We could have a
"destination" field for that, and a special "all of the group"
destination for commands affecting the whole group.
Next, trying to think about scalable iov extensions. So we
will have groups of VFs and then SFs as the next level.
How does one differentiate between the two?
Maybe reserve a field for "destination type"?
The point of all this is to allow making sense of commands and
e.g. virtualizing them for nested virt without necessarily
knowing all of the detail about the specific command.
--
MST
-
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 v3 1/4] Add virtio Admin virtqueue
On Mon, Feb 07, 2022 at 12:14:33PM +0200, Max Gurtovoy wrote:
>
> On 2/3/2022 3:09 PM, Cornelia Huck wrote:
> > On Thu, Feb 03 2022, Max Gurtovoy wrote:
> >
> > > In one of the many use cases a user wants to manipulate features and
> > > configuration of the virtio devices regardless of the device type
> > > (net/block/console). Some of this configuration is generic enough. i.e
> > > Number of MSI-X vectors of a virtio PCI VF device. There is a need to do
> > > such features query and manipulation by its parent PCI PF.
> > >
> > > Currently virtio specification defines control virtqueue to manipulate
> > > features and configuration of the device it operates on. However,
> > > control virtqueue commands are device type specific, which makes it very
> > > difficult to extend for device agnostic commands.
> > >
> > > To support this requirement in elegant way, this patch introduces a new
> > > admin virtqueue interface. Admin virtqueue is proposed as one of the
> > > interfaces to issue admin commands that have the same command format for
> > > all type of virtio devices.
> > >
> > > Manipulate features via admin virtqueue is asynchronous, scalable, easy
> > > to extend and doesn't require additional and expensive on-die resources
> > > to be allocated for every new feature that will be added in the future.
> > >
> > > Subsequent patches make use of this admin virtqueue.
> > >
> > > Reviewed-by: Parav Pandit
> > > Signed-off-by: Max Gurtovoy
> > > ---
> > > admin.tex | 94 +
> > > conformance.tex | 1 +
> > > content.tex | 8 +++--
> > > 3 files changed, 101 insertions(+), 2 deletions(-)
> > > create mode 100644 admin.tex
> > >
> > > diff --git a/admin.tex b/admin.tex
> > > new file mode 100644
> > > index 000..fa9c993
> > > --- /dev/null
> > > +++ b/admin.tex
> > > @@ -0,0 +1,94 @@
> > [some wording only, have not yet thought about the rest]
> >
> > > +\section{Admin Virtqueues}\label{sec:Basic Facilities of a Virtio Device
> > > / Admin Virtqueues}
> > > +
> > > +Admin virtqueue is one of the management interface that used to send
> > > administrative
> > "An admin virtqueue is a management interface of a device that can be
> > used..."
>
> OK thanks.
>
> >
> > > +commands to manipulate various features of the device and/or to
> > > manipulate
> > > +various features, if possible, of another device within the same group
> > > (e.g. PCI VFs
> > Maybe add
> >
> > "Which devices are actually considered a group is transport specific."
> >
> > ?
>
> Not sure we want to restrict ourselves for that.
do restrict this please, if we want to extend the scope we can
always do that down the road.
> >
> > > +of a parent PCI PF device are grouped together. These devices can be
> > > optionally
> > > +managed by its parent PCI PF using its admin virtqueue.).
> > I would move the content in the brackets to a separate paragraph. Maybe
> >
> > "An example of a group is PCI virtual functions (VFs) being grouped
> > together with their parent PCI physical function (PF). These VFs can be
> > optionally managed by their parent PF using its admin virtqueue."
>
> Sounds good.
>
>
> >
> > > +
> > > +An admin virtqueue exists for a certain device if VIRTIO_F_ADMIN_VQ is
> > > +negotiated. The index of the admin virtqueue exposed by the device in a
> > s/exposed/is exposed/
> thanks.
> >
> > > +transport specific manner.
> > > +
> > > +When VIRTIO_F_ADMIN_VQ is negotiated with the device, driver will send
> > > all admin commands
> > > +through the admin virtqueue.
> > That sounds a bit like the driver might use an alternative interface for
> > the admin commands as well? What about
>
> Yes if there will be an alternative for AQ and this feature bit will not be
> negotiated so the driver will use a different channel.
>
> This was explicitly discussed in previous versions.
>
> What is the issue with this assumption ?
it's not an issue down the road but we want to be clear
that right now that is the only way, to make sure
reader does not waste time looking for more ways
in the spec. maybe just say so.
> >
> > "If VIRTIO_F_ADMIN_VQ has been negotiated, the driver used the admin
> > virtqueue to send admin commands."
>
> maybe:
>
> "If VIRTIO_F_ADMIN_VQ has been negotiated, the driver will use the admin
> virtqueue to send
> all admin commands."
>
>
> >
-
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 v3 1/4] Add virtio Admin virtqueue
On Thu, Feb 03 2022, Max Gurtovoy wrote:
> In one of the many use cases a user wants to manipulate features and
> configuration of the virtio devices regardless of the device type
> (net/block/console). Some of this configuration is generic enough. i.e
> Number of MSI-X vectors of a virtio PCI VF device. There is a need to do
> such features query and manipulation by its parent PCI PF.
>
> Currently virtio specification defines control virtqueue to manipulate
> features and configuration of the device it operates on. However,
> control virtqueue commands are device type specific, which makes it very
> difficult to extend for device agnostic commands.
>
> To support this requirement in elegant way, this patch introduces a new
> admin virtqueue interface. Admin virtqueue is proposed as one of the
> interfaces to issue admin commands that have the same command format for
> all type of virtio devices.
>
> Manipulate features via admin virtqueue is asynchronous, scalable, easy
> to extend and doesn't require additional and expensive on-die resources
> to be allocated for every new feature that will be added in the future.
>
> Subsequent patches make use of this admin virtqueue.
>
> Reviewed-by: Parav Pandit
> Signed-off-by: Max Gurtovoy
> ---
> admin.tex | 94 +
> conformance.tex | 1 +
> content.tex | 8 +++--
> 3 files changed, 101 insertions(+), 2 deletions(-)
> create mode 100644 admin.tex
>
> diff --git a/admin.tex b/admin.tex
> new file mode 100644
> index 000..fa9c993
> --- /dev/null
> +++ b/admin.tex
> @@ -0,0 +1,94 @@
[some wording only, have not yet thought about the rest]
> +\section{Admin Virtqueues}\label{sec:Basic Facilities of a Virtio Device /
> Admin Virtqueues}
> +
> +Admin virtqueue is one of the management interface that used to send
> administrative
"An admin virtqueue is a management interface of a device that can be used..."
> +commands to manipulate various features of the device and/or to manipulate
> +various features, if possible, of another device within the same group (e.g.
> PCI VFs
Maybe add
"Which devices are actually considered a group is transport specific."
?
> +of a parent PCI PF device are grouped together. These devices can be
> optionally
> +managed by its parent PCI PF using its admin virtqueue.).
I would move the content in the brackets to a separate paragraph. Maybe
"An example of a group is PCI virtual functions (VFs) being grouped
together with their parent PCI physical function (PF). These VFs can be
optionally managed by their parent PF using its admin virtqueue."
> +
> +An admin virtqueue exists for a certain device if VIRTIO_F_ADMIN_VQ is
> +negotiated. The index of the admin virtqueue exposed by the device in a
s/exposed/is exposed/
> +transport specific manner.
> +
> +When VIRTIO_F_ADMIN_VQ is negotiated with the device, driver will send all
> admin commands
> +through the admin virtqueue.
That sounds a bit like the driver might use an alternative interface for
the admin commands as well? What about
"If VIRTIO_F_ADMIN_VQ has been negotiated, the driver used the admin
virtqueue to send admin commands."
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
