Re: [edk2] [PATCH 0/2] Implement NULL pointer detection feature

2017-08-27 Thread Yao, Jiewen
Comment in line. From: Wang, Jian J Sent: Monday, August 28, 2017 11:24 AM To: Yao, Jiewen ; edk2-devel@lists.01.org Subject: RE: [edk2] [PATCH 0/2] Implement NULL pointer detection feature 1) I think this feature should be 'FALSE' by default. I forgot to reset its

Re: [edk2] [Patch] BaseTools: Enable --whole-archive in GCC tool chain as the default option

2017-08-27 Thread Gao, Liming
Laszlo: I will update the patch with your comments. Ard: We collect the size impact in Ovmf platform. Its impact is small. So, my patch enable this option for all targets. Below is the data collected on OvmfIa32X64.dsc with GCC5 tool chain. Raw image is a little bigger. But, the

Re: [edk2] [PATCH 0/2] Implement NULL pointer detection feature

2017-08-27 Thread Wang, Jian J
1) I think this feature should be 'FALSE' by default. I forgot to reset its default value. This feature makes use of page mechanism to detect NULL pointer. So any ARCH doesn't support paging in EDK-II can't use it. Currently validated platform includes VLV2 and Denlow. Let me know if all

Re: [edk2] [patch] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Add NULL pointer check

2017-08-27 Thread Dong, Eric
Reviewed-by: Eric Dong -Original Message- From: Bi, Dandan Sent: Friday, August 25, 2017 10:59 AM To: edk2-devel@lists.01.org Cc: Dong, Eric ; Gao, Liming Subject: [patch] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Add

Re: [edk2] [PATCH 0/2] Implement NULL pointer detection feature

2017-08-27 Thread Yao, Jiewen
Thank you to enable this feature. I have 2 comments, after a very quick review. 1) I notice it is enabled by default "gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetection|TRUE". Would you please provide the information on how many open source platforms are validated? Such as, IA

[edk2] [patch] MdeModulePkg/UefiHiiLib: Fix incorrect check for string length

2017-08-27 Thread Dandan Bi
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=681 For string opcode,when checking the valid string length, it should exclude the Null-terminated character. And for string in NameValue storage, need to exclude the varname and the need to convert the Config string length to Unicode string

[edk2] [PATCH 2/2] Implement NULL pointer detection for EDK-II SMM Core and driver

2017-08-27 Thread Wang, Jian J
This feature is for debug purpose which helps to detect potential NULL pointer access in code at run-time in SMM mode. Cc: Eric Dong Cc: Star Zeng Cc: Jiewen Yao Suggested-by: Wolman, Ayellet

[edk2] [PATCH 1/2] Implement NULL pointer detection for EDK-II Core

2017-08-27 Thread Wang, Jian J
This feature is for debug purpose which helps to detect potential NULL pointer access in code at run-time. Cc: Star Zeng Cc: Jiewen Yao Cc: Eric Dong Suggested-by: Wolman, Ayellet Contributed-under:

[edk2] [PATCH 0/2] Implement NULL pointer detection feature

2017-08-27 Thread Wang, Jian J
This patch is the implementation of NULL pointer detection feature, which is one of the small features of Special Pool. Wang, Jian J (2): Implement NULL pointer detection for EDK-II Core Implement NULL pointer detection for EDK-II SMM Core and driver MdeModulePkg/Core/Dxe/DxeMain.inf

Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Fix memory protection crash

2017-08-27 Thread Zeng, Star
Thanks. :) Pushed this patch at https://github.com/tianocore/edk2/commit/714c2603018a99a514c42c2b511c821f30ba9cdf. And sent the patch at https://lists.01.org/pipermail/edk2-devel/2017-August/013768.html for the definition movement. Star -Original Message- From: edk2-devel

[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Centralize mPhysicalAddressBits definition

2017-08-27 Thread Star Zeng
Originally (before 714c2603018a99a514c42c2b511c821f30ba9cdf), mPhysicalAddressBits was only defined in X64 PageTbl.c, after 714c2603018a99a514c42c2b511c821f30ba9cdf, mPhysicalAddressBits is also defined in Ia32 PageTbl.c, then mPhysicalAddressBits is used in ConvertMemoryPageAttributes() for

[edk2] [PATCH v2 3/3] OvmfPkg/VirtioBlkDxe: negotiate VIRTIO_F_IOMMU_PLATFORM

2017-08-27 Thread Brijesh Singh
VirtioBlkDxe driver has been updated to use IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to device address. We do not need to do anything special when VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with VIRTIO_F_VERSION_1. Cc:

[edk2] [PATCH v2 2/3] Ovmfpkg/VirtioBlkDxe: map virtio-blk request and response buffers

2017-08-27 Thread Brijesh Singh
When device is behind the IOMMU, driver is require to pass the device address of virtio request, response and any memory referenced by those request/response to the bus master. The patch uses IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to map request and response buffers system

[edk2] [PATCH v2 1/3] OvmfPkg/VirtioBlkDxe: map VRING using VirtioRingMap()

2017-08-27 Thread Brijesh Singh
When device is behind the IOMMU then driver need to pass the device address when programing the bus master. The patch uses VirtioRingMap() to map the VRING system physical address to device address. Cc: Ard Biesheuvel Cc: Jordan Justen Cc:

[edk2] [PATCH v2 0/3] OvmfPkg/VirtioBlkDxe: map host address to device address

2017-08-27 Thread Brijesh Singh
The patch updates the VirtioBlkDxe to use IOMMU-like member functions to map the system physical address to device address for buffers (including vring, device specific request and response pointed by vring descriptor, and any furter memory reference by those request and response). Cc: Ard

Re: [edk2] [PATCH 2/3] Ovmfpkg/VirtioBlkDxe: map virtio-blk request and response buffers

2017-08-27 Thread Brijesh Singh
On 8/27/17 5:05 PM, Laszlo Ersek wrote: [...] > (4) I think we should be careful with BusMasterCommonBuffer operations > similarly to BusMasterWrite operations -- populate first, map second. > > This is to avoid exposing any stale data, even for a short window, to > the device. > > Speaking in

Re: [edk2] [PATCH 2/3] Ovmfpkg/VirtioBlkDxe: map virtio-blk request and response buffers

2017-08-27 Thread Laszlo Ersek
On 08/27/17 22:40, Laszlo Ersek wrote: > This patch is functionally correct. I'd like to request three stylistic > improvements: > > On 08/25/17 23:43, Brijesh Singh wrote: >> When device is behind the IOMMU, driver is require to pass the device >> address of virtio request, response and any

Re: [edk2] [PATCH 2/3] Ovmfpkg/VirtioBlkDxe: map virtio-blk request and response buffers

2017-08-27 Thread Laszlo Ersek
This patch is functionally correct. I'd like to request three stylistic improvements: On 08/25/17 23:43, Brijesh Singh wrote: > When device is behind the IOMMU, driver is require to pass the device > address of virtio request, response and any memory referenced by those > request/response to the

Re: [edk2] [PATCH 3/3] OvmfPkg/VirtioBlkDxe: negotiate VIRTIO_F_IOMMU_PLATFORM

2017-08-27 Thread Laszlo Ersek
On 08/25/17 23:43, Brijesh Singh wrote: > VirtioBlkDxe driver has been updated to use IOMMU-like member functions > from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to > device address. We do not need to do anything special when > VIRTIO_F_IOMMU_PLATFORM bit is present hence

Re: [edk2] [PATCH 1/3] OvmfPkg/VirtioBlkDxe: map VRING using VirtioRingMap()

2017-08-27 Thread Laszlo Ersek
On 08/25/17 23:43, Brijesh Singh wrote: > When device is behind the IOMMU then driver need to pass the device > address when programing the bus master. The patch uses VirtioRingMap() to > map the VRING system physical address to device address. > > Cc: Ard Biesheuvel >

Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Fix memory protection crash

2017-08-27 Thread Laszlo Ersek
On 08/26/17 04:24, Zeng, Star wrote: > Laszlo, > > I am ok to centralize the definition in the patch(V2 will cover it), how > about PiSmmCpuDxeSmm.c? Because I have now tested this patch (see below), at this point my preference would be that you commit v1 as-is, and then post a separate patch