Jeff,

I do not have any objections about this patch.
Milliseconds units looks reasonable.

Thanks!

On Fri, 2014-05-30 at 05:37 +0000, Fan, Jeff wrote:
> Mike,
> 
> Thanks your clarification.  I create one patch to rename TimeoutInSeconds to 
> Timeout to avoid this confusion in attachment. Please review it. 
> 
> If you and others have no additional comments, I will check-in it.
> 
> Thanks!
> Jeff
> -----Original Message-----
> From: Mike Maslenkin [mailto:miha...@parallels.com] 
> Sent: Tuesday, May 27, 2014 6:26 PM
> To: Fan, Jeff
> Cc: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] [PATCH 1/1] IsaFloppyDxe: fixed timeout
> 
> The platform I'm using is a Parallels Virtual Machine.
> 
> I agree with your comment about variable rename or adding some comments to 
> avoid confusion. We misunderstood this place because of lack of comments. 
> 
> BTW I added trace to Recalibrate () function and tried qemu(1.7.1) with OVMF 
> (https://svn.code.sf.net/p/edk2/code/trunk/edk2@15026) .
> 
> So here is a result of 'fgrep  Recalibrate $f  | wc -l':
> 1. command:
>   qemu-system-x86_64 -boot c -m 128 -hda 'test.img' -cdrom 
> '/var/iso/en_windows_8_enterprise_x64_dvd_917522.iso'  -bios ./OVMF.fd  
> -serial file:serial.log
>   serial_1fdd_noimage.log:               6
>   serial_2fdd_noimage.log:               12
> 
> 2. command:
>   qemu-system-x86_64 -boot c -m 128 -hda 'test.img' -cdrom 
> '/var/iso/en_windows_8_enterprise_x64_dvd_917522.iso'  -bios ./OVMF.fd -fda 
> 'floppy.fdd' -serial file:serial.log
>   serial_1fdd_fda.log                   1
>   serial_2fdd_fda.log                   7
> 
> Here "2fdd" means default OVMF setting with 2 floppies and "1fdd" means 
> applied SET gPcAtChipsetPkgTokenSpaceGuid.PcdIsaAcpiFloppyBEnable = FALSE
> 
> So by default there is a worst case used that causes significant delay on 
> start.
> 
> Fortunately it looks like IsaFloppy can be excluded from FW image at all.
> The major reason of floppy requirement for us is an unattended installation, 
> but in case of Windows access to floppy occurs after ExitBootServices and via 
> ACPI FDC0(PNP0700) device. 
> 
> On Wed, 2014-05-21 at 08:20 +0000, Fan, Jeff wrote:
> > Mike,
> > 
> > Which platform do you use? And how much boot performance improvement could 
> > you get on your platform with this fix?
> > 
> > Jeff
> > -----Original Message-----
> > From: Mike Maslenkin [mailto:mike.maslen...@gmail.com]
> > Sent: Wednesday, May 21, 2014 3:01 PM
> > To: edk2-devel@lists.sourceforge.net
> > Cc: Fan, Jeff
> > Subject: Re: [edk2] [PATCH 1/1] IsaFloppyDxe: fixed timeout
> > 
> > 
> > On Tue, 2014-05-20 at 22:30 -0700, Jordan Justen wrote:
> > > On Tue, May 20, 2014 at 7:46 PM, Fan, Jeff <jeff....@intel.com> wrote:
> > > > Mike,
> > > >
> > > > I have one question, how do you find this issue?  Do you encounter 
> > > > one real issue on your platform or you find it by code review?
> > We experienced a long boot time. This patch was in series that tried to fix 
> > this.
> > And from the timeouts perspective it increases possible delay.
> > But in case of virtual machine these conditions must be very fast, because 
> > of required flags can be set immediately after some command accepted. In 
> > case of real floppy device there could be a real timeouts that really 
> > required for access to physical device. So using seconds here would be 
> > right.
> > 
> > > 
> > > I will say that I've seen timeouts on floppy often account for a 
> > > large part of the OVMF boot time. I haven't figured out the correct 
> > > way to fix it though.
> > I can point to another place where unconditional exists:
> > look at Recalibrate() function:
> > 
> >     for (Index = 0; Index < sizeof (FDD_COMMAND_PACKET2); Index++) {
> >       if (EFI_ERROR (DataOutByte (FdcDev, CommandPointer++))) {
> >         return EFI_DEVICE_ERROR;
> >       }
> >     }
> >     //
> >     // Experience value
> >     //
> >     MicroSecondDelay (250000);
> >     //
> >     // need modify according to 1.44M or 2.88M
> >     //
> >     if (EFI_ERROR (SenseIntStatus (FdcDev, &StatusRegister0,
> > &PresentCylinderNumber))) {
> >       return EFI_DEVICE_ERROR;
> >     }
> > 
> > We need to optimize it.
> > 
> > > It is fairly unfortunate considering floppies are almost never used with 
> > > OVMF.
> > 
> > What about unattended installations?
> > 
> > > But, Mike's change seems to increase the timeout, so I guess that 
> > > won't help OVMF. :)
> > 
> > Will try to improve  that :)
> > 
> 
> 
> 



------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their 
applications. Written by three acclaimed leaders in the field, 
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to