Re: [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification

[Stefan Hajnoczi <stefanha@redhat.com>]

On Sun, Aug 01, 2021 at 01:46:03PM +0300, Max Gurtovoy wrote:
> 
> On 7/29/2021 11:48 AM, Stefan Hajnoczi wrote:
> > On Wed, Jul 28, 2021 at 05:20:29PM +0300, Max Gurtovoy wrote:
> > > On 7/28/2021 4:42 PM, Stefan Hajnoczi wrote:
> > > > On Wed, Jul 28, 2021 at 01:59:26PM +0300, Max Gurtovoy wrote:
> > > > > On 7/28/2021 11:52 AM, Stefan Hajnoczi wrote:
> > > > > > On Tue, Jul 27, 2021 at 06:29:49PM +0300, Max Gurtovoy wrote:
> > > > > > > On 7/27/2021 5:28 PM, Cornelia Huck wrote:
> > > > > > > > On Tue, Jul 27 2021, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > > > > > > > On Mon, Jul 26, 2021 at 07:52:53PM +0300, Max Gurtovoy wrote:
> > > > > > > > How many different (groups of) commands can we reasonably expect? Do we
> > > > > > > > need a generic discovery command, or can we get away with a feature bit
> > > > > > > > covering each new group of commands?
> > > > > > > I can't predict the future but IMO we need a discovery command.
> > > > > > > 
> > > > > > > We have many devices and more can be added in the future.
> > > > > > A <u8 class, u8 command> space is 65536 bits or 8KB. I think admin
> > > > > > commands would not be included in VIRTIO Feature Bits but instead
> > > > > > reported via a separate admin command that returns up to 8KB of data:
> > > > > > 
> > > > > >      struct virtio_admin_report_cmds {
> > > > > >          /* Bitmap of available admin commands [Device->Driver]
> > > > > >           * bool command_present =
> > > > > >           *        command_bits[class * 32 + command / 8] & (command % 8);
> > > > > >           */
> > > > > >          u8 command_bits[8192];
> > > > > >      };
> > > > > Yes, I divided it to multiple commands per class to cover the case we will
> > > > > need more than 1 bit to describe a command.
> > > > > 
> > > > > But I guess we can add it later on.
> > > > > 
> > > > > I think the above should be:
> > > > > 
> > > > > bool command_present = command_bits[class * 32 + command / 8] & (1 << (command % 8));
> > > > > 
> > > > > isn't it ?
> > > > You're right. I forgot to shift the bit :D.
> > > > 
> > > > > Also what do you think about renaming <class, command> to <opcode, opmod> ?
> > > > I need to understand how opcode and opmod values are used. I'm not sure.
> > > Same as class and command just with different naming.
> > Is opcode the same as command? What does opmod do?
> 
> no. opcode == class and opmod == command.

I find those terms confusing. "class" and "command" was clearer to me,
at least.

Stefan

Attachment: signature.asc
Description: PGP signature