Hey Jordan, 1) I have used git send-mail, but due to a corruption of my edk2 clone I did some 'funky' stuff to get it working. I expect stuff to be better with the next patch. 2) Will submit a V2 with line-breaks hopefully today, though might be tomorrow. 3) As said in the commit message, as far as I am aware, C preprocessing is not available for ASM (non-NASM), hence the values are hard-coded there still. Is there a way to use PCDs in ASM? 4) Can't the ASM file actually be removed? I don't see it referenced anywhere anymore. Is it kept for the case external packages expect it to be present? Should I clarify the commit message, such as using 'NASM ResetVector'?
Thanks for your input! Regards, Marvin. > -----Original Message----- > From: Jordan Justen [mailto:jordan.l.jus...@intel.com] > Sent: Wednesday, November 2, 2016 11:31 PM > To: Marvin Häuser <marvin.haeu...@outlook.com>; edk2- > de...@lists.01.org > Cc: ler...@redhat.com > Subject: Re: [PATCH v1 1/1] OvmfPkg/ResetVector: Depend on PCD values of > the page tables. > > The "v1 1/1" isn't needed in the subject. For the first version of a single > patch > series, I would expect to just see [PATCH]. (Obviously this is not too > important.) > > Email headers seem to indicate that you aren't using git send-email. > This will cause troubles if someday you have a multi-patch series. > > On 2016-11-02 11:00:34, Marvin Häuser wrote: > > > > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > > b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > > > > ; > > ; Top level Page Directory Pointers (1 * 512GB entry) > > ; > > - mov dword[0x800000], 0x801000 + PAGE_PDP_ATTR > > + mov dword[OVMF_SEC_PAGE_TABLES_BASE], > OVMF_SEC_PAGE_TABLES_BASE + 0x1000 + PAGE_PDP_ATTR > > > > ; > > ; Next level Page Directory Pointers (4 * 1GB entries => 4GB) > > ; > > - mov dword[0x801000], 0x802000 + PAGE_PDP_ATTR > > - mov dword[0x801008], 0x803000 + PAGE_PDP_ATTR > > - mov dword[0x801010], 0x804000 + PAGE_PDP_ATTR > > - mov dword[0x801018], 0x805000 + PAGE_PDP_ATTR > > + mov dword[OVMF_SEC_PAGE_TABLES_BASE + 0x1000], > OVMF_SEC_PAGE_TABLES_BASE + 0x2000 + PAGE_PDP_ATTR > > + mov dword[OVMF_SEC_PAGE_TABLES_BASE + 0x1008], > OVMF_SEC_PAGE_TABLES_BASE + 0x3000 + PAGE_PDP_ATTR > > + mov dword[OVMF_SEC_PAGE_TABLES_BASE + 0x1010], > OVMF_SEC_PAGE_TABLES_BASE + 0x4000 + PAGE_PDP_ATTR > > + mov dword[OVMF_SEC_PAGE_TABLES_BASE + 0x1018], > OVMF_SEC_PAGE_TABLES_BASE + 0x5000 + PAGE_PDP_ATTR > > These line are too long. I guess you can use '\' at the end of a line to > continue > it, or maybe add a PT_ADDR() macro that adds in > OVMF_SEC_PAGE_TABLES_BASE. > > > diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb > > b/OvmfPkg/ResetVector/ResetVector.nasmb > > index 31ac06a..b47f647 100644 > > --- a/OvmfPkg/ResetVector/ResetVector.nasmb > > +++ b/OvmfPkg/ResetVector/ResetVector.nasmb > > @@ -53,6 +53,12 @@ > > %include "Ia32/SearchForSecEntry.asm" > > > > %ifdef ARCH_X64 > > + %ifndef OVMF_SEC_PAGE_TABLES_BASE > > + #include <AutoGen.h> > > + %define OVMF_SEC_PAGE_TABLES_BASE FixedPcdGet32 > (PcdOvmfSecPageTablesBase) > > + %define OVMF_SEC_PAGE_TABLES_SIZE FixedPcdGet32 > > + (PcdOvmfSecPageTablesSize) %endif > > + > > %include "Ia32/Flat32ToFlat64.asm" > > %include "Ia32/PageTables64.asm" > > %endif > > diff --git a/OvmfPkg/ResetVector/ResetVectorCode.asm > > b/OvmfPkg/ResetVector/ResetVectorCode.asm > > index 052c821..5b49387 100644 > > --- a/OvmfPkg/ResetVector/ResetVectorCode.asm > > +++ b/OvmfPkg/ResetVector/ResetVectorCode.asm > > @@ -40,6 +40,15 @@ > > %include "Ia32/SearchForSecEntry.asm" > > > > %ifdef ARCH_X64 > > + %ifndef OVMF_SEC_PAGE_TABLES_BASE > > + ; > > + ; This range should match with PcdOvmfSecPageTablesBase and > > + ; PcdOvmfSecPageTablesSize which are declared in the FDF files. > > + ; > > + %define OVMF_SEC_PAGE_TABLES_BASE 0x800000 > > + %define OVMF_SEC_PAGE_TABLES_SIZE 6 * 0x1000 > > I thought we were using the PCDs? > > -Jordan _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
