Re: [edk2-devel] [PATCH v4 32/35] OvmfPkg: Introduce PcdXenGrantFrames

["Laszlo Ersek" <lersek@redhat.com>]

On 07/29/19 17:39, Anthony PERARD wrote:
> Introduce PcdXenGrantFrames to replace a define in XenBusDxe and allow
> the same value to be used in a different module.
> 
> The reason for the number of page to be 4 doesn't exist anymore, so
> simply remove the comment.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  OvmfPkg/OvmfPkg.dec             | 3 +++
>  OvmfPkg/XenBusDxe/XenBusDxe.inf | 2 ++
>  OvmfPkg/XenBusDxe/XenBusDxe.h   | 1 +
>  OvmfPkg/XenBusDxe/GrantTable.c  | 3 +--
>  4 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 04d5e29272..d5fee805ef 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -225,6 +225,9 @@ [PcdsFixedAtBuild]
>    gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr|0x0|UINT32|0x17
>    gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize|0x0|UINT32|0x32
>  
> +  ## Number of page frames to use for storing grant table entries.
> +  gUefiOvmfPkgTokenSpaceGuid.PcdXenGrantFrames|4|UINT32|0x33
> +
>  [PcdsDynamic, PcdsDynamicEx]
>    gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
> diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.inf b/OvmfPkg/XenBusDxe/XenBusDxe.inf
> index 86e0fb8224..0b78cd0af4 100644
> --- a/OvmfPkg/XenBusDxe/XenBusDxe.inf
> +++ b/OvmfPkg/XenBusDxe/XenBusDxe.inf
> @@ -59,3 +59,5 @@ [Protocols]
>    gXenBusProtocolGuid
>    gXenIoProtocolGuid
>  
> +[FixedPcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdXenGrantFrames
> diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.h b/OvmfPkg/XenBusDxe/XenBusDxe.h
> index 8510361bca..b1dcc3549c 100644
> --- a/OvmfPkg/XenBusDxe/XenBusDxe.h
> +++ b/OvmfPkg/XenBusDxe/XenBusDxe.h
> @@ -22,6 +22,7 @@
>  #include <Library/UefiLib.h>
>  #include <Library/DevicePathLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
>  
>  
>  //

Good catch; in my v3 review at
<http://mid.mail-archive.com/4badd535-c23d-c64d-7bb3-fb42bbbf538a@redhat.com>
I didn't realize we'd be introducing the PcdLib dependency anew.

(1) However, in that case, we should list PcdLib in the [LibraryClasses]
section of the INF file as well.

With (1) fixed:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

There is no need to post v5 just because of this -- if a v5 is otherwise
unnecessary, I can fix this up for you before pushing v4.

Thanks
Laszlo

> diff --git a/OvmfPkg/XenBusDxe/GrantTable.c b/OvmfPkg/XenBusDxe/GrantTable.c
> index 6575e9b88c..1130404cd1 100644
> --- a/OvmfPkg/XenBusDxe/GrantTable.c
> +++ b/OvmfPkg/XenBusDxe/GrantTable.c
> @@ -22,8 +22,7 @@
>  
>  #define NR_RESERVED_ENTRIES 8
>  
> -/* NR_GRANT_FRAMES must be less than or equal to that configured in Xen */
> -#define NR_GRANT_FRAMES 4
> +#define NR_GRANT_FRAMES (FixedPcdGet32 (PcdXenGrantFrames))
>  #define NR_GRANT_ENTRIES (NR_GRANT_FRAMES * EFI_PAGE_SIZE / sizeof(grant_entry_v1_t))
>  
>  STATIC grant_entry_v1_t *GrantTable = NULL;
>