On Tue, May 14, 2013 at 3:10 AM, Laszlo Ersek <[email protected]> wrote:
> On 04/29/13 03:14, Laszlo Ersek wrote:
> > On 04/28/13 02:28, Jordan Justen wrote:
> >> On Tue, Apr 23, 2013 at 3:18 AM, Laszlo Ersek <[email protected]> wrote:
> >>> @@ -199,13 +198,9 @@ VirtioPrepare (
> >>>
> >>>    In *Indices:
> >>>
> >>> -  @param [in] HeadIdx           The index identifying the head buffer 
> >>> (first
> >>> -                                buffer appended) belonging to this same
> >>> -                                request.
> >>> -
> >>> -  @param [in out] NextAvailIdx  On input, the index identifying the next
> >>> -                                descriptor available to carry the 
> >>> buffer. On
> >>> -                                output, incremented by one, modulo 2^16.
> >>> +  @param [in out] NextDescIdx  On input, the index identifying the next
> >>> +                               descriptor to carry the buffer. On output,
> >>> +                               incremented by one, modulo 2^16.
> >>
> >> Can you use @param just with the actual parameters?
> >
> > Should I just move the "@param [in out]" string from NextDescIdx to
> > Indices, or should I also drop NextDescIdx completely?

Use @param[in], @param[out], and @param[in,out], and only for
parameters listed in the C code. For a parameter that is a structure,
you can document fields within the area for that parameter. At least,
this seems to be what our coding style wants.

> > Also, may I follow up with a separate patch for this? To me it would
> > seem a bit more logical to keep these changes separate, and (for some
> > reason) it also feels safer.
> >
> >> Do you think an addition to OvmfPkg/README for Virtio usage would be 
> >> useful?
> >
> > I don't know. People will (or will not) be aware how to configure virtio
> > devices in qemu, independently of OVMF. What do you have in mind?

I guess I what I had in mind was a quick one liner that I could use to
test it. I trust that you test things, but I always feel better
committing something if I can at least minimally test it.

I found our previous discussion about this, and managed to construct
this command line, which is able to at least start booting an ISO
image:
qemu-system-x86_64 -L path/to/ovmf \
  -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x5 \
  -drive 
file=path/to/cd.iso,if=none,media=cdrom,id=drive-scsi0-0-0-0,readonly=on,format=raw
\
  -device 
scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1

Is all this really required? Can it be simplified any?

By the way, do you know any live-CD's that support fully booting
virtio via uefi?

Another thing I'd like to see documented in the README is redirecting
the default debug messages to a log file.

> Ping... this fix is important. Please tell me in direct terms what I
> must do; I'd like to see this committed.

I committed it. I'd still like to see the parameter comments changed,
but I wanted to (at least minimally) test the change.

-Jordan

------------------------------------------------------------------------------
AlienVault Unified Security Management (USM) platform delivers complete
security visibility with the essential security capabilities. Easily and
efficiently configure, manage, and operate all of your security controls
from a single console and one unified framework. Download a free trial.
http://p.sf.net/sfu/alienvault_d2d
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to