Re: [PATCH] Update qemu-kvm to allow a larger BIOS image.

[Sheng Yang <sheng@linux.intel.com>]

On Thursday 16 July 2009 10:58:53 Jordan Justen wrote:
> On Wed, Jul 15, 2009 at 6:34 PM, Sheng Yang <sheng@linux.intel.com> wrote:
> > On Thursday 16 July 2009 08:12:15 Jordan Justen wrote:
> > > Previously the KVM kernel module would allocate the address range
> > > of 0xfffbc000-0xfffbcfff for the EPT Indentity Page Tables.
> > > A separate patch is moving that to 0xfeffc000-0xfeffcfff.
> >
> > Hi Jordan
> >
> > You need one more patch for upstream kvm, to include the modification of
> > vmx.h.
>
> I sent a separate patch for vmx.h for the
> git://git.kernel.org/pub/scm/virt/kvm/kvm.git tree.  Do I
> need a patch for another copy of vmx.h?

Oh, sorry, overlook that one...(I just think the second one is a update to the 
first...) :(
>
> > > This patch updates qemu-kvm to move the VMX TSS Pages & update the
> > > KVM BIOS code to update the E820 BIOS call memory reservations.
> > >
> > > Previously, the VMC TSS Pages were located at 0xfffbd000-0xfffbffff.
> > > This change moved them to 0xfeffd000-0xfeffffff.
> > >
> > > This change also updates the KVM bios such that the E820 locations
> > > are returned properly for these two changes.
> > >
> > > From a high level, these are the effects of these two changes:
> > >
> > > Previously, the KVM would only accommodate a 256KB BIOS image.  With
> > > these changes, the BIOS image may now grow to 16MB.
> > >
> > > Motivation for making these changes:
> > >
> > > A larger firmware image size allows alternative BIOS images to be
> > > used with KVM.  Some possible uses are to enable UEFI firmware
> > > or coreboot firmware.  Additionally, an alternative firmware might
> > > include a linux kernel+initrd payload, which would require several
> > > megabytes.
> >
> > I think if you update bios to UEFI, the E820 should be represented by
> > UEFI rather than current bios?
>
> Yeah, that is true.  But, that firmware would be separate from the qemu-kvm
> tree at this time, right?

Yes, so currently from QEmu-kvm side, I think it may not necessary to update, 
for the patches haven't followed up yet...
>
> But, in this patch it is critical that the 'VMX TSS Pages' are moved within
> qemu-kvm so the conflict with the larger bios.bin is removed.

Well, for we are using a small size bios now, and larger bios would be totally 
another story, I don't think push the modification now to the upstream make 
sense. These modification can go with further patches with UEFI at any time in 
the future.

-- 
regards
Yang, Sheng

>
> > --
> > regards
> > Yang, Sheng
> >
> > > Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> > > ---
> > >  kvm/bios/rombios.c        |    8 ++++----
> > >  kvm/include/x86/asm/vmx.h |    2 +-
> > >  qemu-kvm-x86.c            |    2 +-
> > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/kvm/bios/rombios.c b/kvm/bios/rombios.c
> > > index 6186199..2d0c153 100644
> > > --- a/kvm/bios/rombios.c
> > > +++ b/kvm/bios/rombios.c
> > > @@ -4596,14 +4596,14 @@ ASM_END
> > >                      case 5:
> > >                          /* 4 pages before the bios, 3 pages for vmx
> > > tss pages, * the other page for EPT real mode pagetable */
> > > -                        set_e820_range(ES, regs.u.r16.di, 0xfffbc000L,
> > > -                                       0xfffc0000L, 0, 0, 2);
> > > +                        set_e820_range(ES, regs.u.r16.di, 0xfeffc000L,
> > > +                                       0xff000000L, 0, 0, 2);
> > >                          regs.u.r32.ebx = 6;
> > >                          break;
> > >                      case 6:
> > > -                        /* 256KB BIOS area at the end of 4 GB */
> > > +                        /* 16MB BIOS area at the end of 4 GB */
> > >                          set_e820_range(ES, regs.u.r16.di,
> > > -                                       0xfffc0000L, 0x00000000L ,0, 0,
> > > 2); +                                       0xff000000L, 0x00000000L
> > > ,0, 0, 2); if (extra_highbits_memory_size ||
> > > extra_lowbits_memory_size) regs.u.r32.ebx = 7;
> > >                          else
> > > diff --git a/kvm/include/x86/asm/vmx.h b/kvm/include/x86/asm/vmx.h
> > > index df8d4f9..99e2bb9 100644
> > > --- a/kvm/include/x86/asm/vmx.h
> > > +++ b/kvm/include/x86/asm/vmx.h
> > > @@ -403,7 +403,7 @@ enum vmcs_field {
> > >  #define VMX_EPT_EXECUTABLE_MASK                      0x4ull
> > >  #define VMX_EPT_IGMT_BIT                     (1ull << 6)
> > >
> > > -#define VMX_EPT_IDENTITY_PAGETABLE_ADDR              0xfffbc000ul
> > > +#define VMX_EPT_IDENTITY_PAGETABLE_ADDR              0xfeffc000ul
> > >
> > >
> > >  #define ASM_VMX_VMCLEAR_RAX       ".byte 0x66, 0x0f, 0xc7, 0x30"
> > > diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> > > index daf60b6..b5306aa 100644
> > > --- a/qemu-kvm-x86.c
> > > +++ b/qemu-kvm-x86.c
> > > @@ -63,7 +63,7 @@ static int kvm_init_tss(kvm_context_t kvm)
> > >                * this address is 3 pages before the bios, and the bios
> > > should present * as unavaible memory
> > >                */
> > > -             r = kvm_set_tss_addr(kvm, 0xfffbd000);
> > > +             r = kvm_set_tss_addr(kvm, 0xfeffd000);
> > >               if (r < 0) {
> > >                       fprintf(stderr, "kvm_init_tss: unable to set tss
> > > addr\n"); return r;
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html