Re: [PATCH v1 1/1] OvmfPkg/ResetVector: Depend on PCD values of the page tables.

[Jordan Justen <jordan.l.justen@intel.com>]

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