Re: [edk2] [PATCH 5/8] OvmfPkg/EmuVariableFvbRuntimeDxe: Disable if flash variables are supported

2013-10-31 Thread Laszlo Ersek
On 10/28/13 22:27, Jordan Justen wrote:
 If QEMU flash is supported, then the PcdFlashNvStorageVariableBase64
 will be set by the flash FVB driver. If it is set to a non-zero value,
 then we disable memory based variables.
 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Jordan Justen jordan.l.jus...@intel.com
 ---
  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c |6 ++
  1 file changed, 6 insertions(+)
 
 diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c 
 b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
 index c7c3fcb..7a8beb3 100644
 --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
 +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
 @@ -819,6 +819,12 @@ FvbInitialize (
  return EFI_INVALID_PARAMETER;
}
  
 +  if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
 +DEBUG ((EFI_D_INFO, Disabling EMU Variable FVB since 
 +flash variables appear to be supported.\n));
 +return EFI_ABORTED;
 +  }
 +
//
// By default we will initialize the FV contents.  But, if
// PcdEmuVariableNvStoreReserved is non-zero, then we will
 

This function (the entry point of the EmuVariableFvbRuntimeDxe driver)
would set PcdFlashNvStorageVariableBase64 (64-bit Base address of the
NV variable range in flash device) later on, unconditionally. The OVMF
default value is 0.

Seems logical to me.

One question: how do you ensure that the flash FVB driver in patch #6
runs before this one? (It does seem to work in practice.)

... Ah I can see it in patch #7.

Reviewed-by: Laszlo Ersek ler...@redhat.com

Thanks!
Laszlo

--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [PATCH 4/8] OvmfPkg/AcpiPlatformDxe/Qemu: Allow high runtime memory regions

2013-10-31 Thread Laszlo Ersek
On 10/28/13 22:27, Jordan Justen wrote:
 Previously we would only search for MMIO regions that were also
 above every EfiGcdMemoryTypeReserved and EfiGcdMemoryTypeSystemMemory
 region.
 
 Now we just search for the largest EfiGcdMemoryTypeMemoryMappedIo
 region.
 
 This will allow us to mark the flash memory as a runtime memory
 region in order to allow runtime access of variables stored in
 flash.

What happens if this patch is not included?

According to my testing with a 5GB guest, the patch lowers the top of
the MMIO range
- from 0xFEEF
- to   0xFBFF

The FDF files set PcdOvmfFdBaseAddress to 0xFFF0 (1MB build) or
0xFFE0 (2MB build). Then patch #6 marks the memory from there
upwards as runtime memory.

  0xE000 == 3584 MB  MMIO start (both new  old)
  0xFBFF == 4032 MB - 1 BMMIO end (new)
  0xFEEF == 4079 MB - 1 BMMIO end (old)
  0xFFE0 == 4094 MB  PcdOvmfFdBaseAddress (2MB build)
  0xFFF0 == 4095 MB  PcdOvmfFdBaseAddress (1MB build)

So there doesn't seem to be an overlap with or without this patch.

But, even if there were an overlap that the patch eliminates (ie. if
PcdOvmfFdBaseAddress fell between the new and old MMIO ends), shouldn't
this patch rather find the bounding box (like before), and clamp it down
explicitly with PcdOvmfFdBaseAddress?

Because, the largest MMIO range that patch #4 currently finds is
arbitrary (random) AFAICT. PcdOvmfFdBaseAddress is also (sort of)
arbitrary. I have no idea if anything guarantees that the largest single
MMIO range won't intersect with PcdOvmfFdBaseAddress.

But, again, what if there's an intersection? We're going to report an
MMIO range to the guest kernel (via _CRS) part of which is actually
memory. Probably worth avoiding indeed.

Thanks!
Laszlo

--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] OuputString at the last column of the last row

2013-10-31 Thread Sergey Isakov
Hi,
On 31.10.2013, at 14:18, Li, Elvin wrote:

 Hi Ivy,
   I have no existing solution to resolve your issue. EDKII open source 
 setup browser does not draw at the last line, you may consider this way right 
 now. Another possible way is that we may make OutputString not to scroll 
 screen when cursor is hidden by user (that means user want to draw instead of 
 output string). I need some time to figure out the formal solution and go 
 back to you. If others have better ideas, welcome!
What if implement ScrollLock? in this case after last line will be first line.

 
 Thanks
 Elvin
 -Original Message-
 From: Ivy Yang [mailto:ivy_y...@phoenix.com] 
 Sent: Thursday, October 31, 2013 12:10 PM
 To: edk2-devel@lists.sourceforge.net
 Subject: Re: [edk2] OuputString at the last column of the last row
 
 Elvin,
 
 Thanks for the reply. I can see your point with the spec. Could you advice me 
 on how to OuputString at the last column of the last row in application like 
 SetupBrowser properly without messing up the whole display?
 
 
 Thanks,
 Ivy
 
 
 --
 Android is increasing in popularity, but the open development platform that
 developers love is also attractive to malware creators. Download this white
 paper to learn more about secure code signing practices that can help keep
 Android apps secure.
 http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
 ___
 edk2-devel mailing list
 edk2-devel@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/edk2-devel
 
 --
 Android is increasing in popularity, but the open development platform that
 developers love is also attractive to malware creators. Download this white
 paper to learn more about secure code signing practices that can help keep
 Android apps secure.
 http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
 ___
 edk2-devel mailing list
 edk2-devel@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/edk2-devel
 


--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] Problems in Image Decoding in HII Database code

2013-10-31 Thread Tim Lewis
We noticed that the current HII Database code that processes the image packages 
incorrectly increments the ImageId for EXT1, EXT2 and EXT4 blocks.

The specification allows these forms of the IIBT block types to be used for any 
other blocks, including DUP and SKIP1 and SKIP2. But currently, EXT1 does this:

case EFI_HII_IIBT_EXT1:
  Length8 = *(ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8));
  ImageBlock += Length8;
  ImageIdCurrent++;
  break;
case EFI_HII_IIBT_EXT2:
  CopyMem (
Length16,
ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8),
sizeof (UINT16)
);
  ImageBlock += Length16;
  ImageIdCurrent++;
  break;
case EFI_HII_IIBT_EXT4:
  CopyMem (
Length32,
ImageBlock + sizeof (EFI_HII_IMAGE_BLOCK) + sizeof (UINT8),
sizeof (UINT32)
);
  ImageBlock += Length32;
  ImageIdCurrent++;
  break;

Notice the inconsistent behavior here between EXT1, EXT2 and EXT4. EXT1 does 
not copy the block, but EXT2 and EXT4 do. Also, notice that ImageIdCurrent is 
incremented. This leads to inconsistent behavior when SKIP1 and SKIP2 or DUP 
are encoded using any of these forms.

You may ask: why would anyone do this? Well, one of the ways we are considering 
to extend the image package format is adding new image types (such as 
IIBIT_32BIT). One of the problems with this is that older tools may correctly 
skip the unknown block, but they the ImageId assignment would be incorrect 
(since the older tools don't know it should be incremented). So the solution we 
were thinking is that new image types should be followed by a IIBT_SKIP1 (1) so 
that even older tools would correctly handle the increment of the ImageId. But 
of course, that will not work if the current EDK2 implementation makes all EXT4 
and EXT2 increment the ImageId!

Tim
--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [PATCH 4/8] OvmfPkg/AcpiPlatformDxe/Qemu: Allow high runtime memory regions

2013-10-31 Thread Jordan Justen
On Thu, Oct 31, 2013 at 2:40 AM, Laszlo Ersek ler...@redhat.com wrote:
 On 10/28/13 22:27, Jordan Justen wrote:
 Previously we would only search for MMIO regions that were also
 above every EfiGcdMemoryTypeReserved and EfiGcdMemoryTypeSystemMemory
 region.

 Now we just search for the largest EfiGcdMemoryTypeMemoryMappedIo
 region.

 This will allow us to mark the flash memory as a runtime memory
 region in order to allow runtime access of variables stored in
 flash.

 What happens if this patch is not included?

This branch (from the old code) is not taken w/o this patch
if (Mmio32MinBase  Mmio32MaxExclTop) {

And therefore, PciWindow32 is not set...

-Jordan

--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [PATCH 4/8] OvmfPkg/AcpiPlatformDxe/Qemu: Allow high runtime memory regions

2013-10-31 Thread Laszlo Ersek
On 10/31/13 18:53, Jordan Justen wrote:
 On Thu, Oct 31, 2013 at 2:40 AM, Laszlo Ersek ler...@redhat.com wrote:
 On 10/28/13 22:27, Jordan Justen wrote:
 Previously we would only search for MMIO regions that were also
 above every EfiGcdMemoryTypeReserved and EfiGcdMemoryTypeSystemMemory
 region.

 Now we just search for the largest EfiGcdMemoryTypeMemoryMappedIo
 region.

 This will allow us to mark the flash memory as a runtime memory
 region in order to allow runtime access of variables stored in
 flash.

(*)


 What happens if this patch is not included?
 
 This branch (from the old code) is not taken w/o this patch
 if (Mmio32MinBase  Mmio32MaxExclTop) {
 
 And therefore, PciWindow32 is not set...

Ah.

Can you please write more helpful / detailed commit messages and/or
cover letter?

This patchset is built in reverse (which isn't unusual, in order to
avoid regressions mid-series), but, unlike the submitter, the reviewer
has no clue about the future. In other words, when reviewing patch #4, I
have no idea about patch #6 or patch #7.

The paragraph I marked with (*) is way too laconic.

What's going on here is that in patch 7/8 a new driver is added to the
apriori list, hence it runs super-early. Specifically, earlier than the
code being patched in patch 4/8. This new driver marks a memory region
as runtime memory in patch 6/8 (again, at said very early time), and
this memory region ends exactly at 4GB. The marking is done by
*modifying the memory map*, which is what the code under patch 4/8 feeds
off.

Hence, the

find MMIO bounding box and clamp it from below with conventional
memory

logic is *precisely* busted by the conventional RAM range added
chronologically earlier at the top of the first four gigabytes. When the
MMIO bounding box is clamped above 4G, nothing remains.

 for (CurDesc = 0; CurDesc  NumDesc; ++CurDesc) {
   CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc;
   UINT64 ExclTop;

   Desc = AllDesc[CurDesc];
   ExclTop = Desc-BaseAddress + Desc-Length;

   if (ExclTop = BASE_4GB) {

The conventional RAM range in question satisfies this check.

 switch (Desc-GcdMemoryType) {
   case EfiGcdMemoryTypeNonExistent:
 break;

   case EfiGcdMemoryTypeReserved:
   case EfiGcdMemoryTypeSystemMemory:
 if (NonMmio32MaxExclTop  ExclTop) {
   NonMmio32MaxExclTop = ExclTop;
 }
 break;

   case EfiGcdMemoryTypeMemoryMappedIo:
 if (Mmio32MinBase  Desc-BaseAddress) {
   Mmio32MinBase = Desc-BaseAddress;
 }
 if (Mmio32MaxExclTop  ExclTop) {
   Mmio32MaxExclTop = ExclTop;
 }
 break;

   default:
 ASSERT(0);
 }
   }
 }

After the loop:
o  (Mmio32MinBase  Mmio32MaxExclTop) is true; some bounding box has
   been found,
o  but (NonMmio32MaxExclTop == BASE_4GB) holds as well.


 if (Mmio32MinBase  NonMmio32MaxExclTop) {
   Mmio32MinBase = NonMmio32MaxExclTop;
 }

This is the clamp from below stuff. Mmio32MinBase is raised to BASE_4GB.


 if (Mmio32MinBase  Mmio32MaxExclTop) {

After which this check (nonempty bounding box after clamping) clearly
fails, and we return EFI_UNSUPPORTED, and FWDT is not installed.

(The pre-patch code actually works as expected, what's unexpected is a
conventional RAM range just below 4G.)

So, my R-b stands, but I wish you had helped me understand this more
quickly.

I think I won't try to review 6/8, I'll just test it. But first I'd like
to hear what you think of the regression I described in another response
to 4/8.

Thanks
Laszlo


--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] UEFI Shell Question

2013-10-31 Thread Bruce Maynard
This has been bothering me for some time...

Why is the UEFI shell so slow?...doesn't matter what version you use.  
1.0/2.0/Internal/External.
It scrolls lines as fast as if it were connected to a serial 9600 baud modem 
(remember those?).
Is there any way to adjust the console print output?
Is seems like it is artificially delaying writing characters to the screen?
I guess no one uses it, so no one gives it much thought.
The shell apps I write scroll lines so slow that is it very bothersome,  but I 
cannot control the character output.
The UEFI shell commands all display output at this slow scrolling speed.
Any good reason why it's way?

-Bruce



--
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel