Re: [edk2] [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit

[zhengxiang (A)]

Hello Laszlo and Paolo,

Thanks for your review!

On 2017/12/13 19:16, Laszlo Ersek wrote:
> On 12/13/17 10:29, Paolo Bonzini wrote:
>> On 13/12/2017 09:35, Laszlo Ersek wrote:
>>> I consider the lack of a "VIRTIO_SCSI_F_MQ" feature bit an issue with
>>> the virtio specification (and consequently with vhost-scsi), not with
>>> the guest driver(s).
>> VIRTIO_SCSI_F_MQ does not exist because virtio-scsi has _always_
>> supported multiqueue and has always had a "num_queues" field in the
>> configuration space.  For virtio-net, VIRTIO_NET_F_MQ does not say
>> "the device or driver knows about multiqueue", it says "the device or
>> driver wants to read max_virtqueue_pairs" from configuration space.
>> It's perfectly fine for a device to propose VIRTIO_NET_F_MQ and set
>> max_virtqueue_pairs=1, or for a driver to negotiate VIRTIO_NET_F_MQ
>> and then skip initialization of some virtqueues.
>>
>> This also means that Maxime's patch to DPDK is also not enough. :)
>> Virtio-net actually does have a configuration mechanism for
>> multiqueue, namely the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command; the
>> driver sends that command specifying the number of the transmit and
>> receive queues to use.  However, in my understanding, that command is
>> only needed for the device to configure receive flow steering, so
>> virtio-scsi doesn't need that either.
>>
>>> Perhaps you can update vhost-scsi similarly to the last patch of
>>> Maxime's v4 series, even without "VIRTIO_SCSI_F_MQ" -- in the
>>> SET_FEATURES request handler, just destroy the unused virtqueues that
>>> have not been configured by the guest driver until that time?
>> Yes, this is the right solution.  We can assume that if the descriptor
>> address is equal to zero, the queue is not in use.  This is not in the
>> spec as far as I can see, but it is QEMU's assumption.  I will send a
>> patch to the virtio specification.

I would try this solution! However, is there any possibility that the allocated
descriptor address is exactly equal to zero and the queue is in use?

Moreover, is it feasible to skip the vhost_virtqueue_start() call for the unused
queues in vhost_dev_start() in QEMU?



Thanks,
Xiang


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/3] Fix the series issues in UefiPxeBcDxe.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Wu, Jiaxin
> Sent: Thursday, December 14, 2017 1:50 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [Patch 0/3] Fix the series issues in UefiPxeBcDxe.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin 
> 
> Jiaxin Wu (3):
>   NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.
>   NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback
> Protocol.
>   NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for
> NewStationIP/NewSubnetMask
> 
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c|  8 
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   |  5 +
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c|  2 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++--
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
>  5 files changed, 26 insertions(+), 17 deletions(-)
> 
> --
> 1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/2] MdePkg/Include: Reflect latest spec revision

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Wu, Hao A
> Sent: Thursday, December 14, 2017 9:00 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Gao, Liming ; 
> Kinney, Michael D 
> Subject: [PATCH 0/2] MdePkg/Include: Reflect latest spec revision
> 
> The series updates the header files within:
>   MdePkg/Include/Pi/
>   MdePkg/Include/Uefi/
> 
> to reflect the latest revision of the PI & UEFI spec respectively.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> 
> Hao Wu (2):
>   MdePkg/UefiSpec.h: Update the UEFI version to reflect new revision
>   MdePkg/Include/Pi: Modify specification number encoding
> 
>  MdePkg/Include/Pi/PiDxeCis.h   | 6 +++---
>  MdePkg/Include/Pi/PiPeiCis.h   | 4 ++--
>  MdePkg/Include/Uefi/UefiSpec.h | 7 ---
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> --
> 2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/3] Fix the series issues in UefiPxeBcDxe.

[Wu, Jiaxin]

Hi Siyuan and Ting,

Those issues are exposed by SCT manual test, please help to review them.

Thanks,
Jiaxin



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Thursday, December 14, 2017 1:50 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [edk2] [Patch 0/3] Fix the series issues in UefiPxeBcDxe.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin 
> 
> Jiaxin Wu (3):
>   NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.
>   NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback
> Protocol.
>   NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for
> NewStationIP/NewSubnetMask
> 
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c|  8 
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   |  5 +
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c|  2 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++--
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
>  5 files changed, 26 insertions(+), 17 deletions(-)
> 
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 2/3] NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback Protocol.

[Jiaxin Wu]

According UEFI Spec:
The PXE Base Code Callback Protocol must be on the same handle as the PXE
Base Code Protocol.

But current implementation doesn't follow that. This patch is fix that issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 6 +++---
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
index 0bb1d66..1f8895f 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
@@ -992,11 +992,11 @@ PxeBcInstallCallback (
   //
   // Check whether PxeBaseCodeCallbackProtocol already installed.
   //
   PxeBc  = >PxeBc;
   Status = gBS->HandleProtocol (
-  Private->Controller,
+  Private->Mode.UsingIpv6 ? Private->Ip6Nic->Controller : 
Private->Ip4Nic->Controller,
   ,
   (VOID **) >PxeBcCallback
   );
   if (Status == EFI_UNSUPPORTED) {
 
@@ -1008,11 +1008,11 @@ PxeBcInstallCallback (
 
 //
 // Install a default callback if user didn't offer one.
 //
 Status = gBS->InstallProtocolInterface (
->Controller,
+Private->Mode.UsingIpv6 ? >Ip6Nic->Controller : 
>Ip4Nic->Controller,
 ,
 EFI_NATIVE_INTERFACE,
 >LoadFileCallback
 );
 
@@ -1052,11 +1052,11 @@ PxeBcUninstallCallback (
 NewMakeCallback = FALSE;
 
 PxeBc->SetParameters (PxeBc, NULL, NULL, NULL, NULL, );
 
 gBS->UninstallProtocolInterface (
-  Private->Controller,
+  Private->Mode.UsingIpv6 ? Private->Ip6Nic->Controller : 
Private->Ip4Nic->Controller,
   ,
   >LoadFileCallback
   );
   }
 }
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
index ab9e494..1fc26c5 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
@@ -1923,11 +1923,11 @@ EfiPxeBcSetParameters (
 if (*NewMakeCallback) {
   //
   // Update the previous PxeBcCallback protocol.
   //
   Status = gBS->HandleProtocol (
-  Private->Controller,
+  Mode->UsingIpv6 ? Private->Ip6Nic->Controller : 
Private->Ip4Nic->Controller,
   ,
   (VOID **) >PxeBcCallback
   );
 
   if (EFI_ERROR (Status) || (Private->PxeBcCallback->Callback == NULL)) {
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 3/3] NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for NewStationIP/NewSubnetMask

[Jiaxin Wu]

According the UEFI Spec for PxeBc.SetStationIP():
If NewStationIP is NULL, then the current IP address will not be modified.
...
If NewSubnetMask is NULL, then the current subnet mask will not be modified.

Currently, EfiPxeBcSetStationIP() doesn't comply with UEFI Spec. This patch is
to fix the issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++--
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
index 538cb59..52f1e92 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
@@ -28,26 +28,26 @@
 
 **/
 EFI_STATUS
 PxeBcFlushStationIp (
   PXEBC_PRIVATE_DATA   *Private,
-  EFI_IP_ADDRESS   *StationIp,
+  EFI_IP_ADDRESS   *StationIp, OPTIONAL
   EFI_IP_ADDRESS   *SubnetMask OPTIONAL
   )
 {
   EFI_PXE_BASE_CODE_MODE   *Mode;
   EFI_STATUS   Status;
 
-  ASSERT (StationIp != NULL);
-
   Mode   = Private->PxeBc.Mode;
   Status = EFI_SUCCESS;
 
   if (Mode->UsingIpv6) {
 
-CopyMem (>Udp6CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv6_ADDRESS));
-CopyMem (>Ip6CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv6_ADDRESS));
+if (StationIp != NULL) {
+  CopyMem (>Udp6CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv6_ADDRESS));
+  CopyMem (>Ip6CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv6_ADDRESS));
+}
 
 //
 // Reconfigure the Ip6 instance to capture background ICMP6 packets with 
new station Ip address.
 //
 Private->Ip6->Cancel (Private->Ip6, >Icmp6Token);
@@ -58,15 +58,19 @@ PxeBcFlushStationIp (
   goto ON_EXIT;
 }
 
 Status = Private->Ip6->Receive (Private->Ip6, >Icmp6Token);
   } else {
-ASSERT (SubnetMask != NULL);
-CopyMem (>Udp4CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv4_ADDRESS));
-CopyMem (>Udp4CfgData.SubnetMask, SubnetMask, sizeof 
(EFI_IPv4_ADDRESS));
-CopyMem (>Ip4CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv4_ADDRESS));
-CopyMem (>Ip4CfgData.SubnetMask, SubnetMask, sizeof 
(EFI_IPv4_ADDRESS));
+if (StationIp != NULL) {
+  CopyMem (>Udp4CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv4_ADDRESS));
+  CopyMem (>Ip4CfgData.StationAddress, StationIp, sizeof 
(EFI_IPv4_ADDRESS));
+}
+
+if (SubnetMask != NULL) {
+  CopyMem (>Udp4CfgData.SubnetMask, SubnetMask, sizeof 
(EFI_IPv4_ADDRESS));
+  CopyMem (>Ip4CfgData.SubnetMask, SubnetMask, sizeof 
(EFI_IPv4_ADDRESS));
+}
 
 //
 // Reconfigure the Ip4 instance to capture background ICMP packets with 
new station Ip address.
 //
 Private->Ip4->Cancel (Private->Ip4, >IcmpToken);
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h 
b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h
index b8519ae..17bee5c 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h
@@ -1,9 +1,9 @@
 /** @file
   Support functions declaration for UefiPxeBc Driver.
 
-  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
   http://opensource.org/licenses/bsd-license.php.
@@ -38,11 +38,11 @@
 
 **/
 EFI_STATUS
 PxeBcFlushStationIp (
   PXEBC_PRIVATE_DATA   *Private,
-  EFI_IP_ADDRESS   *StationIp,
+  EFI_IP_ADDRESS   *StationIp, OPTIONAL
   EFI_IP_ADDRESS   *SubnetMask OPTIONAL
   );
 
 
 /**
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 1/3] NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.

[Jiaxin Wu]

According UEFI Spec, if valid PXE offer is not received, Pxe.Dhcp()
should return EFI_NO_RESPONSE, but currently, EFI_TIMEOUT is returned
from Pxe.Dhcp().

This patch is to fix the above issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c  | 2 +-
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
index fc50a82..0bb1d66 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
@@ -1240,11 +1240,11 @@ ON_EXIT:
   } else if (Status == EFI_OUT_OF_RESOURCES) {
 AsciiPrint ("\n  PXE-E09: Could not allocate I/O buffers.\n");
   } else if (Status == EFI_NO_MEDIA) {
 AsciiPrint ("\n  PXE-E12: Could not detect network connection.\n");
   } else if (Status == EFI_NO_RESPONSE) {
-AsciiPrint ("\n  PXE-E16: No offer received.\n");
+AsciiPrint ("\n  PXE-E16: No valid offer received.\n");
   } else if (Status == EFI_TIMEOUT) {
 AsciiPrint ("\n  PXE-E18: Server response timeout.\n");
   } else if (Status == EFI_ABORTED) {
 AsciiPrint ("\n  PXE-E21: Remote boot cancelled.\n");
   } else if (Status == EFI_ICMP_ERROR) {
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
index 97829e9..101c658 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
@@ -1705,10 +1705,15 @@ PxeBcDhcp4Dora (
   Status = Dhcp4->Start (Dhcp4, NULL);
   if (EFI_ERROR (Status) && Status != EFI_ALREADY_STARTED) {
 if (Status == EFI_ICMP_ERROR) {
   PxeMode->IcmpErrorReceived = TRUE;
 }
+
+if (Status == EFI_TIMEOUT && Private->OfferNum > 0) {
+  Status = EFI_NO_RESPONSE;
+}
+
 goto ON_EXIT;
   }
 
   //
   // Get the acquired IPv4 address and store them.
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 0/3] Fix the series issues in UefiPxeBcDxe.

[Jiaxin Wu]

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 

Jiaxin Wu (3):
  NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.
  NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback
Protocol.
  NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for
NewStationIP/NewSubnetMask

 NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c|  8 
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   |  5 +
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c|  2 +-
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++--
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
 5 files changed, 26 insertions(+), 17 deletions(-)

-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Documentation on "dh" shell command

[Andrew Fish]

> On Dec 13, 2017, at 9:15 PM, Udit Kumar  wrote:
> 
> Hi Jaben 
> 
> Dh says like 
> 14C: IPv4
> 14D: SimpleFileSystem DiskIO BlockIO 
> DevicePath(..R,0x7D5FDC4F,0x2,0xA001))
> 

The 1st hex number is a handle number, basically a User Interface name for the 
handle. The names after the colon represent protocol instances on the handle. 

You can get more info about the protocols by dumping the handle so: dh 14D

Thanks,

Andrew Fish

> But documentation is not saying what is column 1, 2 and so on 
> What I am looking for description of output of dh command with various 
> options 
> 
> Thanks
> Udit 
> 
>> -Original Message-
>> From: Carsey, Jaben [mailto:jaben.car...@intel.com 
>> ]
>> Sent: Wednesday, December 13, 2017 10:54 PM
>> To: Udit Kumar >; 
>> edk2-devel@lists.01.org 
>> Subject: RE: [edk2] Documentation on "dh" shell command
>> 
>> Udit,
>> 
>> There are some examples in the shell spec just before the table you found,
>> but no exact output requirement for dh. What handle information are you
>> looking for?  Maybe we can add that to the dynamic help for the command
>> (the stuff users can see from "dh =?").
>> 
>> -Jaben
>> 
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>> Udit Kumar
>>> Sent: Wednesday, December 13, 2017 4:48 AM
>>> To: edk2-devel@lists.01.org
>>> Subject: [edk2] Documentation on "dh" shell command
>>> Importance: High
>>> 
>>> Hi,
>>> Could you help me, where I can get documentation on output of dh shell
>>> command.
>>> UEFI_Shell_Spec_2_2.pdf , table 18 is giving details of sfo option
>>> only for each column.
>>> 
>>> Thanks
>>> Udit
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis 
>>> 
>>> ts.01.org %2Fmailman%2Flistinfo%2Fedk2-
>> devel=02%7C01%7Cudit.kumar%
>>> 
>> 40nxp.com 
>> %7C2626239c4c294e1f4bea08d5424e5da9%7C686ea1d3bc2b4c6fa
>> 92cd99
>>> 
>> c5c301635%7C0%7C0%7C636487826719107108=doxsRb2xej47877db0
>> h0z4uam
>>> 1mQh%2BKT2ryBpV4mJv4%3D=0
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org 
> https://lists.01.org/mailman/listinfo/edk2-devel 
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/Variable/RuntimeDxe: Modify function return status

[Zhang, Chao B]

Reviewed-by : Chao Zhang 

-Original Message-
From: Chen, Chen A 
Sent: Thursday, December 7, 2017 2:46 PM
To: edk2-devel@lists.01.org
Cc: Chen, Chen A ; Zhang, Chao B 
Subject: [PATCH] MdeModulePkg/Variable/RuntimeDxe: Modify function return status

Make VariableServiceSetVariable and VariableServiceQueryVariableInfo functions 
return status following UEFI 2.7 spec.

Cc: Zhang Chao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: chenc2 
---
 .../Universal/Variable/RuntimeDxe/Variable.c   | 26 --
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index f39be6b0b4..d7128fe105 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -3145,7 +3145,11 @@ VariableServiceSetVariable (
   //  Make sure if runtime bit is set, boot service bit is set also.
   //
   if ((Attributes & (EFI_VARIABLE_RUNTIME_ACCESS | 
EFI_VARIABLE_BOOTSERVICE_ACCESS)) == EFI_VARIABLE_RUNTIME_ACCESS) {
-return EFI_INVALID_PARAMETER;
+if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0) {
+  return EFI_UNSUPPORTED;
+} else {
+  return EFI_INVALID_PARAMETER;
+}
   } else if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
 if (!mVariableModuleGlobal->VariableGlobal.AuthSupport) {
   //
@@ -3168,15 +3172,16 @@ VariableServiceSetVariable (
   //
   if (((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) == 
EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
  && ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) == 
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
-return EFI_INVALID_PARAMETER;
+return EFI_UNSUPPORTED;
   }
 
   if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) == 
EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
-if (DataSize < AUTHINFO_SIZE) {
-  //
-  // Try to write Authenticated Variable without AuthInfo.
-  //
-  return EFI_SECURITY_VIOLATION;
+//
+//  If DataSize == AUTHINFO_SIZE and then PayloadSize is 0.
+//  Maybe it's the delete operation of common authenticated variable at 
user physical presence.
+//
+if (DataSize != AUTHINFO_SIZE) {
+  return EFI_UNSUPPORTED;
 }
 PayloadSize = DataSize - AUTHINFO_SIZE;
   } else if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) 
== EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { @@ -3522,6 +3527,13 @@ 
VariableServiceQueryVariableInfo (
 return EFI_INVALID_PARAMETER;
   }
 
+  if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0) {
+//
+//  Deprecated attribute, make this check as highest priority.
+//
+return EFI_UNSUPPORTED;
+  }
+
   if ((Attributes & EFI_VARIABLE_ATTRIBUTES_MASK) == 0) {
 //
 // Make sure the Attributes combination is supported by the platform.
--
2.13.2.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Documentation on "dh" shell command

[Udit Kumar]

Hi Jaben 

Dh says like 
14C: IPv4
14D: SimpleFileSystem DiskIO BlockIO DevicePath(..R,0x7D5FDC4F,0x2,0xA001))

But documentation is not saying what is column 1, 2 and so on 
What I am looking for description of output of dh command with various options 

Thanks
Udit 

> -Original Message-
> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> Sent: Wednesday, December 13, 2017 10:54 PM
> To: Udit Kumar ; edk2-devel@lists.01.org
> Subject: RE: [edk2] Documentation on "dh" shell command
> 
> Udit,
> 
> There are some examples in the shell spec just before the table you found,
> but no exact output requirement for dh. What handle information are you
> looking for?  Maybe we can add that to the dynamic help for the command
> (the stuff users can see from "dh =?").
> 
> -Jaben
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Udit Kumar
> > Sent: Wednesday, December 13, 2017 4:48 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] Documentation on "dh" shell command
> > Importance: High
> >
> > Hi,
> > Could you help me, where I can get documentation on output of dh shell
> > command.
> > UEFI_Shell_Spec_2_2.pdf , table 18 is giving details of sfo option
> > only for each column.
> >
> > Thanks
> > Udit
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel=02%7C01%7Cudit.kumar%
> >
> 40nxp.com%7C2626239c4c294e1f4bea08d5424e5da9%7C686ea1d3bc2b4c6fa
> 92cd99
> >
> c5c301635%7C0%7C0%7C636487826719107108=doxsRb2xej47877db0
> h0z4uam
> > 1mQh%2BKT2ryBpV4mJv4%3D=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add support for Watchdog driver

[Meenakshi Aggarwal]

Hi,

There is no clean way to register a handler with this watchdog controller.
Even if we do then there are chances that false notification will be sent to 
the module which has registered a handler.

We can go ahead with this implementation, i assume and i will share new 
revision of patch replacing EFI_INVALID_PARAMETER with EFI_UNSUPPORTED.

Please share your feedback.

Thanks,
Meenakshi

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Gao, Liming
> Sent: Sunday, December 10, 2017 7:01 PM
> To: Leif Lindholm 
> Cc: Kinney, Michael D ; edk2-
> de...@lists.01.org; ard.biesheu...@linaro.org
> Subject: Re: [edk2] [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP :
> Add support for Watchdog driver
> 
> Leif:
>   I have no strong opinion. PI spec doesn't require WdogRegisterHandler
> must be supported. So, this implementation doesn't break spec. For this
> platform, if there is no register handler or no critical register handler, 
> this
> Watchdog driver can be used.
> 
> Thanks
> Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Leif Lindholm
> > Sent: Thursday, December 7, 2017 11:34 PM
> > To: Gao, Liming 
> > Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Kinney, Michael D
> 
> > Subject: Re: [edk2] [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP :
> Add support for Watchdog driver
> >
> > Liming,
> >
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw
> ww.mail-archive.com%2Fedk2-
> devel%40lists.01.org%2Fmsg32761.html=02%7C01%7Cmeenakshi.aggar
> wal%40nxp.com%7C3b6ad3d8cfdd4a766c4a08d53fd23a09%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C0%7C636485094541353596=kEb4x9jl1ng
> %2FlumodoxsB5i4RD3NmTUgX9GN9KcKtkI%3D=0
> > Search for WdogRegisterHandler.
> >
> > This topic is entirely unrelated to any _usage_ of watchdog timer
> > protocol.
> >
> > The topic is only whether it is reasonable to _implement_
> > EFI_WATCHDOG_TIMER_ARCH_PROTOCOL for a hardware watchdog that
> *cannot*
> > cause a callback to a handler function.
> > Because when the hardware watchdog times out, it triggers a hard
> > system reset, without any software interaction.
> >
> > /
> > Leif
> >
> > On Thu, Dec 07, 2017 at 02:54:08PM +, Gao, Liming wrote:
> > > Leif:
> > >   I don't review the whole patch serial. Could you point your usage
> > >   case on watch dog timer protocol?
> > >
> > > Thanks
> > > Liming
> > > > -Original Message-
> > > > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > > > Sent: Thursday, December 7, 2017 7:04 PM
> > > > To: Gao, Liming 
> > > > Cc: Udit Kumar ; Kinney, Michael D
> ; Meenakshi Aggarwal
> > > > ; ard.biesheu...@linaro.org; edk2-
> de...@lists.01.org; Varun Sethi 
> > > > Subject: Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP :
> Add support for Watchdog driver
> > > >
> > > > Hi Liming,
> > > >
> > > > On Thu, Dec 07, 2017 at 07:11:38AM +, Gao, Liming wrote:
> > > > > Leif:
> > > > >   I don't see the core driver uses
> > > > >   WatchdogTimer->RegisterHandler(). When it returns unsupported, it
> > > > >   means the additional handler can't be registered. DxeCore uses
> > > > >   WatchdogTimer->SetTimerPeriod(). This service is implemented in
> > > > >   your driver.
> > > > >
> > > > >   Watchdog protocol is defined in PI spec. Spec describes that this
> > > > >   protocol provides the services required to implement the Boot
> > > > >   Service SetWatchdogTimer(). It provides a service to set the
> > > > >   amount of time to wait before firing the watchdog timer, and it
> > > > >   also provides a service to register a handler that is invoked when
> > > > >   the watchdog timer fires. This protocol can implement the watchdog
> > > > >   timer by using the event and timer Boot Services, or it can make
> > > > >   use of custom hardware. If no handler has been registered, or the
> > > > >   registered handler returns, then the system will be reset by
> > > > >   calling the Runtime Service ResetSystem(). So, this protocol is
> > > > >   required.
> > > >
> > > > I am not disputing that the protocol is not required. I am suggesting
> > > > that this hardware watchdog _cannot_ be used to register a handler.
> > > >
> > > > If this hardware watchdog does not get updated in time, that causes an
> > > > immediate hardware reset of the processor.
> > > >
> > > > Because of this, I believe EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is
> not the
> > > > appropriate way to make use of it.
> > > >
> > > > Please let me know whether you agree.
> > > >
> > > > Regards,
> > > >
> > > > Leif
> > > >
> > > > > Thanks
> > > > > Liming
> > > > > >-Original Message-
> > > > > >From: Leif Lindholm 

Re: [edk2] [PATCH] PerformancePkg: Remove it

[Ni, Ruiyu]

On 12/14/2017 9:46 AM, Zeng, Star wrote:

Ruiyu,

Do you need to update Maintainers.txt accordingly at the same time?


Yes I will do that after the PerformancePkg is deleted.
Just would like to split the big changes into two steps:)




Thanks,
Star

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] MdePkg: break #defines out of Uefi/UefiMultiPhase.h

[Ni, Ruiyu]

On 12/13/2017 8:26 PM, Leif Lindholm wrote:

Turns out all .vfr files in the tree interacting with DynamicPcds
manually copy the same set of EFI_VARIABLE_* definitions, since the rest
of UefiMultiPhase.h is incompatible with VfrCompile.

Split these out into a separate header file UefiMultiPhaseDefinitions.h
in order to make it possible to include just that portion into .vfr
files. Then include that from UefiMultiPhase.h.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
  MdePkg/Include/Uefi/UefiMultiPhase.h| 23 +---
  MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h | 39 
  2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/MdePkg/Include/Uefi/UefiMultiPhase.h 
b/MdePkg/Include/Uefi/UefiMultiPhase.h
index 0dcbb1b9ee..b360c9513b 100644
--- a/MdePkg/Include/Uefi/UefiMultiPhase.h
+++ b/MdePkg/Include/Uefi/UefiMultiPhase.h
@@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
  #ifndef __UEFI_MULTIPHASE_H__
  #define __UEFI_MULTIPHASE_H__
  
+#include "UefiMultiPhaseDefinitions.h"

+
  #include 
  ///
  /// Enumeration of memory types introduced in UEFI.
@@ -156,27 +158,6 @@ typedef struct {
  } EFI_TABLE_HEADER;
  
  ///

-/// Attributes of variable.
-///
-#define EFI_VARIABLE_NON_VOLATILE0x0001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS  0x0002
-#define EFI_VARIABLE_RUNTIME_ACCESS  0x0004
-///
-/// This attribute is identified by the mnemonic 'HR'
-/// elsewhere in this specification.
-///
-#define EFI_VARIABLE_HARDWARE_ERROR_RECORD   0x0008
-///
-/// Attributes of Authenticated Variable
-///
-#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS   0x0020
-#define EFI_VARIABLE_APPEND_WRITE0x0040
-///
-/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be 
considered reserved.
-///
-#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS  0x0010
-
-///
  /// AuthInfo is a WIN_CERTIFICATE using the wCertificateType
  /// WIN_CERTIFICATE_UEFI_GUID and the CertType
  /// EFI_CERT_TYPE_RSA2048_SHA256_GUID. If the attribute specifies
diff --git a/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h 
b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
new file mode 100644
index 00..df55a92dfa
--- /dev/null
+++ b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
@@ -0,0 +1,39 @@
+/** @file
+  This includes some definitions introduced in UEFI that will be used in both 
PEI and DXE phases.
+
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under
+the terms and conditions of the BSD License that accompanies this distribution.
+The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php.
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __UEFI_MULTIPHASE_DEFS_H__
+#define __UEFI_MULTIPHASE_DEFS_H__
+
+///
+/// Attributes of variable.
+///
+#define EFI_VARIABLE_NON_VOLATILE0x0001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS  0x0002
+#define EFI_VARIABLE_RUNTIME_ACCESS  0x0004
+///
+/// This attribute is identified by the mnemonic 'HR'
+/// elsewhere in this specification.
+///
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD   0x0008
+///
+/// Attributes of Authenticated Variable
+///
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS   0x0020
+#define EFI_VARIABLE_APPEND_WRITE0x0040
+///
+/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be 
considered reserved.
+///
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS  0x0010
+
+#endif


Can we just move the definitions to UefiBaseTypes.h?
Even vfrcompiler still complains, the syntax enhancement to
vfrcompiler should be simple.

I personally do not like creating more and more files.

--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/3] MdeModulePkg: use central variable definitions in DriverSampleDxe

[Zeng, Star]

Reviewed-by: Star Zeng  to MdeModulePkg change.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
Lindholm
Sent: Wednesday, December 13, 2017 8:26 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Zeng, Star 
Subject: [edk2] [PATCH 2/3] MdeModulePkg: use central variable definitions in 
DriverSampleDxe

Use UefiMultiPhaseDefinitions.h in Vfr.vfr instead of duplicating the 
definitions.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr 
b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
index c1682913fa..551d78c15f 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
+++ b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
@@ -14,6 +14,7 @@
 //**/
 
 
+#include 
 #include "NVDataStruc.h"
 
 //
@@ -35,14 +36,6 @@
 #define EFI_FRONT_PAGE_SUBCLASS   0x02
 #define EFI_SINGLE_USE_SUBCLASS   0x03
 
-//
-// EFI Variable attributes
-//
-#define EFI_VARIABLE_NON_VOLATILE   0x0001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002
-#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
-#define EFI_VARIABLE_READ_ONLY  0x0008
-
 #define EFI_USER_INFO_ACCESS_SETUP_ADMIN_GUID \
   { 0x85b75607, 0xf7ce, 0x471e, { 0xb7, 0xe4, 0x2a, 0xea, 0x5f, 0x72, 0x32, 
0xee } }
 
--
2.11.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Change BIOS Version

[lushifex]

Change BIOS Minor Version

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: lushifex 
---
 Platform/BroxtonPlatformPkg/BiosId.env | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/BroxtonPlatformPkg/BiosId.env 
b/Platform/BroxtonPlatformPkg/BiosId.env
index 14ffbb0..3a90693 100644
--- a/Platform/BroxtonPlatformPkg/BiosId.env
+++ b/Platform/BroxtonPlatformPkg/BiosId.env
@@ -31,5 +31,5 @@ BOARD_ID  = APLKRVP
 BOARD_REV = 3
 BUILD_TYPE= D
 VERSION_MAJOR = 0068
-VERSION_MINOR = 01
+VERSION_MINOR = 02
 BOARD_EXT = X64
-- 
2.7.0.windows.1


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V2] UefiCpuPkg: Check invalid RegisterCpuFeature parameter

[Song, BinX]

Hi All,

Thanks for your suggestion, I will update a V3 patch.

Best Regards,
Bell Song

From: Fan Jeff [mailto:vanjeff_...@hotmail.com]
Sent: Wednesday, December 13, 2017 11:35 PM
To: Ni, Ruiyu ; Laszlo Ersek ; Song, 
BinX ; edk2-devel@lists.01.org
Cc: Dong, Eric 
Subject: 答复: [edk2] [PATCH V2] UefiCpuPkg: Check invalid RegisterCpuFeature 
parameter


I agree to add one _MAX #define in library instance implementation instead of 
in class header file.



Jeff




From: edk2-devel 
> on 
behalf of Ni, Ruiyu >
Sent: Wednesday, December 13, 2017 4:49:01 PM
To: Laszlo Ersek; Song, BinX; 
edk2-devel@lists.01.org
Cc: Dong, Eric
Subject: Re: [edk2] [PATCH V2] UefiCpuPkg: Check invalid RegisterCpuFeature 
parameter

On 12/13/2017 4:44 PM, Laszlo Ersek wrote:
> On 12/13/17 03:35, Song, BinX wrote:
>> V2:
>> Update function name, add more detail description.
>> V1:
>> Check and assert invalid RegisterCpuFeature function parameter
>>
>> Cc: Eric Dong >
>> Cc: Laszlo Ersek >
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Bell Song >
>> ---
>>   .../Include/Library/RegisterCpuFeaturesLib.h   |  5 
>>   .../RegisterCpuFeaturesLib.c   | 29 
>> ++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h 
>> b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> index 9331e49..fc3ccda 100644
>> --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> @@ -71,6 +71,11 @@
>>   #define CPU_FEATURE_APIC_TPR_UPDATE_MESSAGE (32+9)
>>   #define CPU_FEATURE_ENERGY_PERFORMANCE_BIAS (32+10)
>>   #define CPU_FEATURE_PPIN(32+11)
>> +//
>> +// Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
>> +// If you define a feature bigger than it, please also replace it
>> +// in RegisterCpuFeatureLibIsFeatureValid function.
>> +//
>>   #define CPU_FEATURE_PROC_TRACE  (32+12)
>>
>>   #define CPU_FEATURE_BEFORE_ALL  BIT27
>> diff --git 
>> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
>> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> index dd6a82b..6ec26e1 100644
>> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> @@ -81,6 +81,34 @@ DumpCpuFeature (
>>   }
>>
>>   /**
>> +  Determines if the CPU feature is valid.
>> +
>> +  @param[in]  FeaturePointer to CPU feature
>> +
>> +  @retval TRUE  The CPU feature is valid.
>> +  @retval FALSE The CPU feature is invalid.
>> +**/
>> +BOOLEAN
>> +RegisterCpuFeatureLibIsFeatureValid (
>> +  IN UINT32Feature
>> +  )
>> +{
>> +  UINT32  Data;
>> +
>> +  Data = Feature;
>> +  Data &= ~(CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER | CPU_FEATURE_BEFORE_ALL 
>> | CPU_FEATURE_AFTER_ALL);
>> +  //
>> +  // Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
>> +  // If you define a feature bigger than it, please replace it at below.
>> +  //
>> +  if (Data > CPU_FEATURE_PROC_TRACE) {
>> +DEBUG ((DEBUG_ERROR, "Invalid CPU feature: 0x%x ", Feature));
>> +return FALSE;
>> +  }
>> +  return TRUE;
>> +}
>> +
>> +/**
>> Determines if the feature bit mask is in dependent CPU feature bit mask 
>> buffer.
>>
>> @param[in]  FeatureMaskPointer to CPU feature bit mask
>> @@ -444,6 +472,7 @@ RegisterCpuFeature (
>>
>> VA_START (Marker, InitializeFunc);
>> Feature = VA_ARG (Marker, UINT32);
>> +  ASSERT (RegisterCpuFeatureLibIsFeatureValid(Feature));
>> while (Feature != CPU_FEATURE_END) {
>>   ASSERT ((Feature & (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER))
>>   != (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER));
>>
>
> The consensus thus far seems to be that we should not add a separate
> _MAX macro for this purpose. I don't understand why -- in my opinion it
> would be easier to update the macro in one place only.
>
> Now, I realize we have a library class header file here, and a library
> instance. Those things are separate; it is conceivable that another
> library instance is developed independently, and thus we should not tie
> the MAX feature of *all* library instances to the same central class header.
>
> However, this separation is already being violated in this patch: the
> RegisterCpuFeatureLibIsFeatureValid() function is an implementation
> detail of the (currently only one) library instance. Thus, the lib 

Re: [edk2] MdeModulePkg/UefiPxeBcDxe: Question about IcmpErrorListenHandler in PxeBcImpl.c

[Guo Heyi]

Not yet; I can do that right now.

Regards,

Gary (Heyi Guo)

On Wed, Dec 13, 2017 at 07:39:21AM +, Wu, Jiaxin wrote:
> Hi Gary,
> 
> Do you have reported the Bugzilla for this issue? If not, can you report one 
> for it?
> 
> Thank you very much!
> Jiaxin
> 
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Guo Heyi
> > Sent: Monday, December 11, 2017 6:43 PM
> > To: Wu, Jiaxin 
> > Cc: Ni, Ruiyu ; Dong, Eric ; edk2-
> > de...@lists.01.org; Wu, Jiaxin ; Fu, Siyuan
> > ; Zeng, Star 
> > Subject: Re: [edk2] MdeModulePkg/UefiPxeBcDxe: Question about
> > IcmpErrorListenHandler in PxeBcImpl.c
> > 
> > Hi Jiaxin,
> > 
> > We tested and the patch worked. We also tested PXE boot and no new issue
> > was observed.
> > 
> > Thanks and regards,
> > 
> > Gary (Heyi Guo)
> > 
> > 
> > On Thu, Dec 07, 2017 at 08:20:28PM +0800, Heyi Guo wrote:
> > >
> > >
> > > 在 12/7/2017 6:40 PM, Wu, Jiaxin 写道:
> > > >>You say "IcmpErrorRcvToken is only used to get ICMP error from IP
> > > >>layer", does that mean only ICMP error packets will go to
> > > >>IcmpErrorListenHandler?
> > > >>If it is the case, how do we make it? I can only find a simple
> > > >>Ip4->Receive is called to receive IP4 packets; how are other types of
> > > >>IP4 packets filtered out?
> > > >No, all of the *ICMP* packets with the same station IP address will go to
> > IcmpErrorListenHandler() callback function. Because PXE driver has already
> > configured the current IP protocol only receive the ICMP packets:
> > > > ZeroMem (>Ip4ConfigData, sizeof
> > (EFI_IP4_CONFIG_DATA));
> > > > Private->Ip4ConfigData.DefaultProtocol   = EFI_IP_PROTO_ICMP;
> > > > Private->Ip4ConfigData.AcceptIcmpErrors  = TRUE;
> > > > Ip4ConfigData.DefaultProtocol   = EFI_IP_PROTO_ICMP;
> > > >So, it is only used to capture background ICMP packets (Including the
> > ICMP error message) with the same station IP address.
> > > Many thanks; it explains my question clearly.
> > >
> > > >
> > > >>If it is not, why don't we need to filter the packets in
> > > >>IcmpErrorListenHandler? If we recycle the packets in
> > > >>IcmpErrorListenHandler, will it cause the upper layer of protocols fail
> > > >>to fetch RxData?
> > > >IcmpErrorListenHandler() should filter the packets and only handle the
> > ICMP error message. But currently, the code logic is incorrect. I generated
> > one patch as attached one for your reference, can you help to verify
> > whether it works or not? (Ignore my previous suggestion check).
> > > Sure we can verify it.
> > > >In my opinion, the RxData should be recycled since it has been recorded 
> > > >in
> > Mode->IcmpError.
> > > Agree.
> > >
> > > Thanks,
> > >
> > > Gary (Heyi Guo)
> > > >
> > > >Thanks,
> > > >Jiaxin
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >>-Original Message-
> > > >>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > Of
> > > >>Heyi Guo
> > > >>Sent: Thursday, December 7, 2017 4:18 PM
> > > >>To: Wu, Jiaxin ; edk2-devel@lists.01.org
> > > >>Cc: Ni, Ruiyu ; Fu, Siyuan ;
> > Dong,
> > > >>Eric ; Zeng, Star 
> > > >>Subject: Re: [edk2] MdeModulePkg/UefiPxeBcDxe: Question about
> > > >>IcmpErrorListenHandler in PxeBcImpl.c
> > > >>
> > > >>Hi Jiaxin,
> > > >>
> > > >>Thanks for your reply.
> > > >>
> > > >>You say "IcmpErrorRcvToken is only used to get ICMP error from IP
> > > >>layer", does that mean only ICMP error packets will go to
> > > >>IcmpErrorListenHandler?
> > > >>
> > > >>If it is the case, how do we make it? I can only find a simple
> > > >>Ip4->Receive is called to receive IP4 packets; how are other types of
> > > >>IP4 packets filtered out?
> > > >>
> > > >>If it is not, why don't we need to filter the packets in
> > > >>IcmpErrorListenHandler? If we recycle the packets in
> > > >>IcmpErrorListenHandler, will it cause the upper layer of protocols fail
> > > >>to fetch RxData?
> > > >>
> > > >>Please forgive me if my questions are too stupid :)
> > > >>
> > > >>Regards,
> > > >>
> > > >>Gary (Heyi Guo)
> > > >>
> > > >>
> > > >>在 12/7/2017 3:48 PM, Wu, Jiaxin 写道:
> > > >>>Hi Gary,
> > > >>>
> > > >>>IcmpErrorRcvToken is only used to get ICMP error from IP layer, and
> > the
> > > >>data will be copied to Mode->IcmpError. So, I think the RxData should be
> > > >>recycled.
> > > >>>Besides, EFI_IP_PROTO_ICMP should be also checked in the call
> > function
> > > >>but currently it's not:
> > > >>>if (!EFI_IP4_EQUAL (>Header->DestinationAddress,
> > 
> > > >>>StationIp.v4)) {
> > > >>>  //
> > > >>>  // The dest address is not equal to Station Ip address, discard 
> > > >>> it.
> > > >>>  //
> > > >>>  goto CleanUp;
> > > >>>}
> > > 

[edk2] [PATCH 1/2] MdePkg/UefiSpec.h: Update the UEFI version to reflect new revision

[Hao Wu]

Cc: Liming Gao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdePkg/Include/Uefi/UefiSpec.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h
index 92575aea3f..ee016b48de 100644
--- a/MdePkg/Include/Uefi/UefiSpec.h
+++ b/MdePkg/Include/Uefi/UefiSpec.h
@@ -1,8 +1,8 @@
 /** @file
   Include file that supports UEFI.
 
-  This include file must contain things defined in the UEFI 2.6 specification.
-  If a code construct is defined in the UEFI 2.6 specification it must be 
included
+  This include file must contain things defined in the UEFI 2.7 specification.
+  If a code construct is defined in the UEFI 2.7 specification it must be 
included
   by this include file.
 
 Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
@@ -1775,6 +1775,7 @@ EFI_STATUS
 // EFI Runtime Services Table
 //
 #define EFI_SYSTEM_TABLE_SIGNATURE  SIGNATURE_64 ('I','B','I',' 
','S','Y','S','T')
+#define EFI_2_70_SYSTEM_TABLE_REVISION  ((2 << 16) | (70))
 #define EFI_2_60_SYSTEM_TABLE_REVISION  ((2 << 16) | (60))
 #define EFI_2_50_SYSTEM_TABLE_REVISION  ((2 << 16) | (50))
 #define EFI_2_40_SYSTEM_TABLE_REVISION  ((2 << 16) | (40))
@@ -1785,7 +1786,7 @@ EFI_STATUS
 #define EFI_2_00_SYSTEM_TABLE_REVISION  ((2 << 16) | (00))
 #define EFI_1_10_SYSTEM_TABLE_REVISION  ((1 << 16) | (10))
 #define EFI_1_02_SYSTEM_TABLE_REVISION  ((1 << 16) | (02))
-#define EFI_SYSTEM_TABLE_REVISION   EFI_2_60_SYSTEM_TABLE_REVISION
+#define EFI_SYSTEM_TABLE_REVISION   EFI_2_70_SYSTEM_TABLE_REVISION
 #define EFI_SPECIFICATION_VERSION   EFI_SYSTEM_TABLE_REVISION
 
 #define EFI_RUNTIME_SERVICES_SIGNATURE  SIGNATURE_64 
('R','U','N','T','S','E','R','V')
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/2] MdePkg/Include/Pi: Modify specification number encoding

[Hao Wu]

Change the PEI, DXE, and SMM service table revisions to 1.6.

Cc: Liming Gao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdePkg/Include/Pi/PiDxeCis.h | 6 +++---
 MdePkg/Include/Pi/PiPeiCis.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Include/Pi/PiDxeCis.h b/MdePkg/Include/Pi/PiDxeCis.h
index 079dd3eab1..c081d047b3 100644
--- a/MdePkg/Include/Pi/PiDxeCis.h
+++ b/MdePkg/Include/Pi/PiDxeCis.h
@@ -1,7 +1,7 @@
 /** @file
   Include file matches things in PI.
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under 
 the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
@@ -11,7 +11,7 @@ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS 
IS" BASIS,
 WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.  
   
 
   @par Revision Reference:
-  PI Version 1.4
+  PI Version 1.6
 
 **/
 
@@ -696,7 +696,7 @@ EFI_STATUS
 //
 #define DXE_SERVICES_SIGNATURE0x565245535f455844ULL
 #define DXE_SPECIFICATION_MAJOR_REVISION  1
-#define DXE_SPECIFICATION_MINOR_REVISION  40
+#define DXE_SPECIFICATION_MINOR_REVISION  60
 #define DXE_SERVICES_REVISION 
((DXE_SPECIFICATION_MAJOR_REVISION<<16) | (DXE_SPECIFICATION_MINOR_REVISION))
 
 typedef struct {
diff --git a/MdePkg/Include/Pi/PiPeiCis.h b/MdePkg/Include/Pi/PiPeiCis.h
index aebe3eacf4..eace494ecf 100644
--- a/MdePkg/Include/Pi/PiPeiCis.h
+++ b/MdePkg/Include/Pi/PiPeiCis.h
@@ -12,7 +12,7 @@ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS 
IS" BASIS,
 WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.  
   
 
   @par Revision Reference:
-  PI Version 1.4a.
+  PI Version 1.6.
 
 **/
 
@@ -859,7 +859,7 @@ EFI_STATUS
 // PEI Specification Revision information
 //
 #define PEI_SPECIFICATION_MAJOR_REVISION  1
-#define PEI_SPECIFICATION_MINOR_REVISION  40
+#define PEI_SPECIFICATION_MINOR_REVISION  60
 ///
 /// Specification inconsistency here: 
 /// In the PI1.0 spec, PEI_SERVICES_SIGNATURE is defined as 
0x5652455320494550. But 
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 0/2] MdePkg/Include: Reflect latest spec revision

[Hao Wu]

The series updates the header files within:
  MdePkg/Include/Pi/
  MdePkg/Include/Uefi/

to reflect the latest revision of the PI & UEFI spec respectively.

Cc: Liming Gao 
Cc: Michael D Kinney 

Hao Wu (2):
  MdePkg/UefiSpec.h: Update the UEFI version to reflect new revision
  MdePkg/Include/Pi: Modify specification number encoding

 MdePkg/Include/Pi/PiDxeCis.h   | 6 +++---
 MdePkg/Include/Pi/PiPeiCis.h   | 4 ++--
 MdePkg/Include/Uefi/UefiSpec.h | 7 ---
 3 files changed, 9 insertions(+), 8 deletions(-)

-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Documentation on "dh" shell command

[Carsey, Jaben]

Udit,

There are some examples in the shell spec just before the table you found, but 
no exact output requirement for dh. What handle information are you looking 
for?  Maybe we can add that to the dynamic help for the command (the stuff 
users can see from "dh =?").

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Udit Kumar
> Sent: Wednesday, December 13, 2017 4:48 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Documentation on "dh" shell command
> Importance: High
> 
> Hi,
> Could you help me, where I can get documentation on output of dh shell
> command.
> UEFI_Shell_Spec_2_2.pdf , table 18 is giving details of sfo option only for
> each column.
> 
> Thanks
> Udit
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] 答复: [PATCH V2] UefiCpuPkg: Check invalid RegisterCpuFeature parameter

[Fan Jeff]

I agree to add one _MAX #define in library instance implementation instead of 
in class header file.



Jeff




From: edk2-devel  on behalf of Ni, Ruiyu 

Sent: Wednesday, December 13, 2017 4:49:01 PM
To: Laszlo Ersek; Song, BinX; edk2-devel@lists.01.org
Cc: Dong, Eric
Subject: Re: [edk2] [PATCH V2] UefiCpuPkg: Check invalid RegisterCpuFeature 
parameter

On 12/13/2017 4:44 PM, Laszlo Ersek wrote:
> On 12/13/17 03:35, Song, BinX wrote:
>> V2:
>> Update function name, add more detail description.
>> V1:
>> Check and assert invalid RegisterCpuFeature function parameter
>>
>> Cc: Eric Dong 
>> Cc: Laszlo Ersek 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Bell Song 
>> ---
>>   .../Include/Library/RegisterCpuFeaturesLib.h   |  5 
>>   .../RegisterCpuFeaturesLib.c   | 29 
>> ++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h 
>> b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> index 9331e49..fc3ccda 100644
>> --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> @@ -71,6 +71,11 @@
>>   #define CPU_FEATURE_APIC_TPR_UPDATE_MESSAGE (32+9)
>>   #define CPU_FEATURE_ENERGY_PERFORMANCE_BIAS (32+10)
>>   #define CPU_FEATURE_PPIN(32+11)
>> +//
>> +// Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
>> +// If you define a feature bigger than it, please also replace it
>> +// in RegisterCpuFeatureLibIsFeatureValid function.
>> +//
>>   #define CPU_FEATURE_PROC_TRACE  (32+12)
>>
>>   #define CPU_FEATURE_BEFORE_ALL  BIT27
>> diff --git 
>> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
>> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> index dd6a82b..6ec26e1 100644
>> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> @@ -81,6 +81,34 @@ DumpCpuFeature (
>>   }
>>
>>   /**
>> +  Determines if the CPU feature is valid.
>> +
>> +  @param[in]  FeaturePointer to CPU feature
>> +
>> +  @retval TRUE  The CPU feature is valid.
>> +  @retval FALSE The CPU feature is invalid.
>> +**/
>> +BOOLEAN
>> +RegisterCpuFeatureLibIsFeatureValid (
>> +  IN UINT32Feature
>> +  )
>> +{
>> +  UINT32  Data;
>> +
>> +  Data = Feature;
>> +  Data &= ~(CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER | CPU_FEATURE_BEFORE_ALL 
>> | CPU_FEATURE_AFTER_ALL);
>> +  //
>> +  // Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
>> +  // If you define a feature bigger than it, please replace it at below.
>> +  //
>> +  if (Data > CPU_FEATURE_PROC_TRACE) {
>> +DEBUG ((DEBUG_ERROR, "Invalid CPU feature: 0x%x ", Feature));
>> +return FALSE;
>> +  }
>> +  return TRUE;
>> +}
>> +
>> +/**
>> Determines if the feature bit mask is in dependent CPU feature bit mask 
>> buffer.
>>
>> @param[in]  FeatureMaskPointer to CPU feature bit mask
>> @@ -444,6 +472,7 @@ RegisterCpuFeature (
>>
>> VA_START (Marker, InitializeFunc);
>> Feature = VA_ARG (Marker, UINT32);
>> +  ASSERT (RegisterCpuFeatureLibIsFeatureValid(Feature));
>> while (Feature != CPU_FEATURE_END) {
>>   ASSERT ((Feature & (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER))
>>   != (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER));
>>
>
> The consensus thus far seems to be that we should not add a separate
> _MAX macro for this purpose. I don't understand why -- in my opinion it
> would be easier to update the macro in one place only.
>
> Now, I realize we have a library class header file here, and a library
> instance. Those things are separate; it is conceivable that another
> library instance is developed independently, and thus we should not tie
> the MAX feature of *all* library instances to the same central class header.
>
> However, this separation is already being violated in this patch: the
> RegisterCpuFeatureLibIsFeatureValid() function is an implementation
> detail of the (currently only one) library instance. Thus, the lib class
> header should not refer to it, even in a comment.
>
> So, I don't understand why we can't just add a _MAX macro. The central
> library instance could use _MAX; all other (out of tree) instances would
> not use _MAX.
>

I do not understand either:)
But if the change doesn't expose more interfaces (_MAX in this case), I
feel safe because we can change much freely in future.

> Anyway, this doesn't mean the patch is not correct.
>
> Acked-by: Laszlo Ersek 
>
> Thanks
> Laszlo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> 

Re: [edk2] [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds

[Pete Batard]

Hi Ard. Thanks for reviewing this.

On 2017.12.12 19:59, Ard Biesheuvel wrote:

+  EXPORT _fltused
+  EXPORT __brkdiv0
+


Why are these being exported? Are these part of the CRT ABI?


Good point. I exported them because I was encountering undefined symbols 
for those when I started working on adding ARM support. But I now 
suspect they may have come from an export statement from VS headers 
(which we no longer include), as re-test shows that the exports seem to 
be superfluous.


I will test further to confirm that this is the case, and remove the 
unneeded exports.



+__brkdiv0
+udf #249
+


This will cause a crash when invoked, in a way that gives very little
to go on to figure out what happens.


Yes. I mostly copied what ReactOS was doing here, since it should be 
similar to what Microsoft does internally.



Perhaps branch to a C function
here that calls ASSERT() and CpuDeadLoop ?


I don't mind giving it a try, if we keep the separate .asm files for MSVC.

On the other hand, if I am to try to reuse existing assembly, as you 
suggest below, then I don't think I want to introduce anything that will 
differ from how divisions by zero are currently handled there. As far as 
I can tell (for RVCT, which is the only assembly that might be suitable 
for reuse with MSFT) the current way of handling a zero divisor is to 
set result and remainder to 0 and return (as per uldiv.asm). For gcc, we 
also have the following comment in div.S:


  "@ What to do about division by zero?  For now, just return."

So, while I agree that calling ASSERT and CpuDeadLoop is probably the 
better approach, I'd rather have the introduction of this behaviour be 
the subject of a separate ARM harmonization patch, that applies to all 
the toolchains, rather than something that's specific to Visual Studio 
usage.



There are many different division implementations already in the same
directory. Do we really need a new one?


I'll look into that. My goal with this series was to make sure that the 
changes being introduced would not break existing toolchains, which is 
why I saw it preferable to keep the MSVC intrinsics separate.


Now, gcc assembly is not something that can be reused easily with the MS 
toolchain, so the only possibility are the RVCT .asm files.


There's a fair bit of work involved into trying to figure out if and how 
we can reuse that code, but I'll see what I can do. It'll probably be a 
few weeks before I get back to this list on that however.


Regards,

/Pete
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Documentation on "dh" shell command

[Udit Kumar]

Hi, 
Could you help me, where I can get documentation on output of dh shell command.
UEFI_Shell_Spec_2_2.pdf , table 18 is giving details of sfo option only for 
each column.

Thanks 
Udit
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/3] Use central definitions for EFI_VARIABLE_*

[Ard Biesheuvel]

On 13 December 2017 at 12:26, Leif Lindholm  wrote:
> The set of variable attribute definitions in  is
> used by C code, but VfrCompile has no way of dealing with structs or
> typedefs, and the VFRPP rules generate (and depend on) preprocessing with
> C rules.
>
> There may be neater ways of dealing with this, but a simple solution is
> to break the #defines into a separate header and include this both in
> UefiMultiPhase.h and directly in .vfr source.
>
> Leif Lindholm (3):
>   MdePkg: break #defines out of Uefi/UefiMultiPhase.h
>   MdeModulePkg: use central variable definitions in DriverSampleDxe
>   EmbeddedPkg: use central variable definitions in .vfr files
>

For the series

Reviewed-by: Ard Biesheuvel 

>  EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefHii.vfr |  9 +
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr   |  9 +
>  MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr|  9 +
>  MdePkg/Include/Uefi/UefiMultiPhase.h  | 23 +---
>  MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h   | 39 
> 
>  5 files changed, 44 insertions(+), 45 deletions(-)
>  create mode 100644 MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
>
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Ard Biesheuvel 
> Cc: Star Zeng 
> Cc: Eric Dong 
>
> --
> 2.11.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 0/3] Use central definitions for EFI_VARIABLE_*

[Leif Lindholm]

The set of variable attribute definitions in  is
used by C code, but VfrCompile has no way of dealing with structs or
typedefs, and the VFRPP rules generate (and depend on) preprocessing with
C rules.

There may be neater ways of dealing with this, but a simple solution is
to break the #defines into a separate header and include this both in
UefiMultiPhase.h and directly in .vfr source.

Leif Lindholm (3):
  MdePkg: break #defines out of Uefi/UefiMultiPhase.h
  MdeModulePkg: use central variable definitions in DriverSampleDxe
  EmbeddedPkg: use central variable definitions in .vfr files

 EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefHii.vfr |  9 +
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr   |  9 +
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr|  9 +
 MdePkg/Include/Uefi/UefiMultiPhase.h  | 23 +---
 MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h   | 39 
 5 files changed, 44 insertions(+), 45 deletions(-)
 create mode 100644 MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 

-- 
2.11.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 3/3] EmbeddedPkg: use central variable definitions in .vfr files

[Leif Lindholm]

Use UefiMultiPhaseDefinitions.h in Vfr.vfr instead of duplicating
the definitions.

Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
 EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefHii.vfr | 9 +
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr   | 9 +
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefHii.vfr 
b/EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefHii.vfr
index a1e603abf0..19371b3157 100644
--- a/EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefHii.vfr
+++ b/EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefHii.vfr
@@ -12,16 +12,9 @@
 *
 **/
 
+#include 
 #include "ConsolePrefDxe.h"
 
-//
-// EFI Variable attributes
-//
-#define EFI_VARIABLE_NON_VOLATILE   0x0001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002
-#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
-#define EFI_VARIABLE_READ_ONLY  0x0008
-
 formset
   guid  = CONSOLE_PREF_FORMSET_GUID,
   title = STRING_TOKEN(STR_FORM_SET_TITLE),
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr 
b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
index 3516746c4d..8e5d34dadb 100644
--- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformHii.vfr
@@ -12,16 +12,9 @@
 *
 **/
 
+#include 
 #include "DtPlatformDxe.h"
 
-//
-// EFI Variable attributes
-//
-#define EFI_VARIABLE_NON_VOLATILE   0x0001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002
-#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
-#define EFI_VARIABLE_READ_ONLY  0x0008
-
 formset
   guid  = DT_PLATFORM_FORMSET_GUID,
   title = STRING_TOKEN(STR_FORM_SET_TITLE),
-- 
2.11.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 1/3] MdePkg: break #defines out of Uefi/UefiMultiPhase.h

[Leif Lindholm]

Turns out all .vfr files in the tree interacting with DynamicPcds
manually copy the same set of EFI_VARIABLE_* definitions, since the rest
of UefiMultiPhase.h is incompatible with VfrCompile.

Split these out into a separate header file UefiMultiPhaseDefinitions.h
in order to make it possible to include just that portion into .vfr
files. Then include that from UefiMultiPhase.h.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
 MdePkg/Include/Uefi/UefiMultiPhase.h| 23 +---
 MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h | 39 
 2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/MdePkg/Include/Uefi/UefiMultiPhase.h 
b/MdePkg/Include/Uefi/UefiMultiPhase.h
index 0dcbb1b9ee..b360c9513b 100644
--- a/MdePkg/Include/Uefi/UefiMultiPhase.h
+++ b/MdePkg/Include/Uefi/UefiMultiPhase.h
@@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef __UEFI_MULTIPHASE_H__
 #define __UEFI_MULTIPHASE_H__
 
+#include "UefiMultiPhaseDefinitions.h"
+
 #include 
 ///
 /// Enumeration of memory types introduced in UEFI.
@@ -156,27 +158,6 @@ typedef struct {
 } EFI_TABLE_HEADER;
 
 ///
-/// Attributes of variable.
-///
-#define EFI_VARIABLE_NON_VOLATILE0x0001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS  0x0002
-#define EFI_VARIABLE_RUNTIME_ACCESS  0x0004
-///
-/// This attribute is identified by the mnemonic 'HR'
-/// elsewhere in this specification.
-///
-#define EFI_VARIABLE_HARDWARE_ERROR_RECORD   0x0008
-///
-/// Attributes of Authenticated Variable
-///
-#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS   0x0020
-#define EFI_VARIABLE_APPEND_WRITE0x0040
-///
-/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be 
considered reserved.
-///
-#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS  0x0010
-
-///
 /// AuthInfo is a WIN_CERTIFICATE using the wCertificateType
 /// WIN_CERTIFICATE_UEFI_GUID and the CertType
 /// EFI_CERT_TYPE_RSA2048_SHA256_GUID. If the attribute specifies
diff --git a/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h 
b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
new file mode 100644
index 00..df55a92dfa
--- /dev/null
+++ b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
@@ -0,0 +1,39 @@
+/** @file
+  This includes some definitions introduced in UEFI that will be used in both 
PEI and DXE phases.
+
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under
+the terms and conditions of the BSD License that accompanies this distribution.
+The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php.
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __UEFI_MULTIPHASE_DEFS_H__
+#define __UEFI_MULTIPHASE_DEFS_H__
+
+///
+/// Attributes of variable.
+///
+#define EFI_VARIABLE_NON_VOLATILE0x0001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS  0x0002
+#define EFI_VARIABLE_RUNTIME_ACCESS  0x0004
+///
+/// This attribute is identified by the mnemonic 'HR'
+/// elsewhere in this specification.
+///
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD   0x0008
+///
+/// Attributes of Authenticated Variable
+///
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS   0x0020
+#define EFI_VARIABLE_APPEND_WRITE0x0040
+///
+/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be 
considered reserved.
+///
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS  0x0010
+
+#endif
-- 
2.11.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/3] MdeModulePkg: use central variable definitions in DriverSampleDxe

[Leif Lindholm]

Use UefiMultiPhaseDefinitions.h in Vfr.vfr instead of duplicating
the definitions.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr 
b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
index c1682913fa..551d78c15f 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
+++ b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
@@ -14,6 +14,7 @@
 //**/
 
 
+#include 
 #include "NVDataStruc.h"
 
 //
@@ -35,14 +36,6 @@
 #define EFI_FRONT_PAGE_SUBCLASS   0x02
 #define EFI_SINGLE_USE_SUBCLASS   0x03
 
-//
-// EFI Variable attributes
-//
-#define EFI_VARIABLE_NON_VOLATILE   0x0001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002
-#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
-#define EFI_VARIABLE_READ_ONLY  0x0008
-
 #define EFI_USER_INFO_ACCESS_SETUP_ADMIN_GUID \
   { 0x85b75607, 0xf7ce, 0x471e, { 0xb7, 0xe4, 0x2a, 0xea, 0x5f, 0x72, 0x32, 
0xee } }
 
-- 
2.11.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [edk2 PATCH v3 2/2] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

[Meenakshi Aggarwal]

Hi Supreeth,

Few comments inline.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Supreeth Venkatesh
> Sent: Friday, November 17, 2017 10:18 PM
> To: edk2-devel@lists.01.org
> Cc: leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [edk2 PATCH v3 2/2] ArmPkg/Drivers: Add
> EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
> 
> PI v1.5 Specification Volume 4 defines Management Mode Core Interface
> and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides
> a
> means of communicating between drivers outside of MM and MMI
> handlers inside of MM.
> 
> This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE
> runtime
> driver for AARCH64 platforms. It uses SMCs allocated from the standard
> SMC range defined in
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Finfo
> center.arm.com%2Fhelp%2Ftopic%2Fcom.arm.doc.den0060a%2FDEN0060A_
> ARM_MM_Interface_Specification.pdf=02%7C01%7Cmeenakshi.aggar
> wal%40nxp.com%7C27ae12fc41414e501f8208d52ddb09f3%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C1%7C636465341155830632=pYKl2uUUF
> VCS4M87y%2BRK8TE852QqcusN0Fm208IkjtU%3D=0
> to communicate with the standalone MM environment in the secure world.
> 
> This patch also adds the MM Communication driver (.inf) file to define entry
> point for this driver and other compile related information the driver
> needs.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 339
> +
>  .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 +++
>  2 files changed, 389 insertions(+)
>  create mode 100644
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
>  create mode 100644
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> 
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> new file mode 100644
> index 00..e801c1c601
> --- /dev/null
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -0,0 +1,339 @@
> +/** @file
> +
> +  Copyright (c) 2016-2017, ARM Limited. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD
> License
> +  which accompanies this distribution.  The full text of the license may be
> found at
> +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C27ae12
> fc41414e501f8208d52ddb09f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636465341155830632=GPd9o7ovyTU10etIxP%2BBYNsYUKq
> m37tPcc%2BQDKtext4%3D=0
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include 
> +
> +//
> +// Address, Length of the pre-allocated buffer for communication with the
> secure
> +// world.
> +//
> +STATIC ARM_MEMORY_REGION_DESCRIPTOR  mNsCommBuffMemRegion;
> +
> +// Notification event when virtual address map is set.
> +STATIC EFI_EVENT  mSetVirtualAddressMapEvent;
> +
> +//
> +// Handle to install the MM Communication Protocol
> +//
> +STATIC EFI_HANDLE  mMmCommunicateHandle;
> +
> +/**
> +  Communicates with a registered handler.
> +
> +  This function provides an interface to send and receive messages to the
> +  Standalone MM environment on behalf of UEFI services.  This function is
> part
> +  of the MM Communication Protocol that may be called in physical mode
> prior to
> +  SetVirtualAddressMap() and in virtual mode after SetVirtualAddressMap().
> +
> +  @param[in]  ThisThe EFI_MM_COMMUNICATION_PROTOCOL
> +  instance.
> +  @param[in, out] CommBuffer  A pointer to the buffer to convey
> +  into MMRAM.
> +  @param[in, out] CommSizeThe size of the data buffer being
> +  passed in. This is optional.
> +
> +  @retval EFI_SUCCESS The message was successfully posted.
> +  @retval EFI_INVALID_PARAMETER   The CommBuffer was NULL.
> +  @retval EFI_BAD_BUFFER_SIZE The buffer size is incorrect for the MM
> +  implementation. If this error is
> +  returned, the MessageLength field in
> +  the CommBuffer header or the integer
> +  pointed by CommSize are updated to 
> reflect
> +  the maximum payload size the
> +  

Re: [edk2] [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit

[Laszlo Ersek]

On 12/13/17 10:29, Paolo Bonzini wrote:
> On 13/12/2017 09:35, Laszlo Ersek wrote:
>> I consider the lack of a "VIRTIO_SCSI_F_MQ" feature bit an issue with
>> the virtio specification (and consequently with vhost-scsi), not with
>> the guest driver(s).
>
> VIRTIO_SCSI_F_MQ does not exist because virtio-scsi has _always_
> supported multiqueue and has always had a "num_queues" field in the
> configuration space.  For virtio-net, VIRTIO_NET_F_MQ does not say
> "the device or driver knows about multiqueue", it says "the device or
> driver wants to read max_virtqueue_pairs" from configuration space.
> It's perfectly fine for a device to propose VIRTIO_NET_F_MQ and set
> max_virtqueue_pairs=1, or for a driver to negotiate VIRTIO_NET_F_MQ
> and then skip initialization of some virtqueues.
>
> This also means that Maxime's patch to DPDK is also not enough. :)
> Virtio-net actually does have a configuration mechanism for
> multiqueue, namely the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command; the
> driver sends that command specifying the number of the transmit and
> receive queues to use.  However, in my understanding, that command is
> only needed for the device to configure receive flow steering, so
> virtio-scsi doesn't need that either.
>
>> Perhaps you can update vhost-scsi similarly to the last patch of
>> Maxime's v4 series, even without "VIRTIO_SCSI_F_MQ" -- in the
>> SET_FEATURES request handler, just destroy the unused virtqueues that
>> have not been configured by the guest driver until that time?
>
> Yes, this is the right solution.  We can assume that if the descriptor
> address is equal to zero, the queue is not in use.  This is not in the
> spec as far as I can see, but it is QEMU's assumption.  I will send a
> patch to the virtio specification.

Hmmm, I'm not so sure about this, on a second thought. Reviewing the
OVMF code, I see that I added a comment (to all of the virtio drivers
actually):

>   //
>   // In virtio-1.0, feature negotiation is expected to complete before queue
>   // discovery, and the device can also reject the selected set of features.
>   //

I added this because of the following sections in the 1.0 spec:
- 3.1.1 Driver Requirements: Device Initialization
- 3.1.2 Legacy Interface: Device Initialization

In particular 3.1.2 writes, "The result was [...] steps 4, 7 and 8 were
conflated.".

(When I added virtio-1.0 support to OVMF, I paid attention to conform to
the new ordering for modern transports, and to keep the ordering
unchanged otherwise.)

I think this is a problem then; if a 1.0 driver is required to finish
feature negotiation (steps 4-6) before configuring the queues (step 7),
then the host side cannot derive any clues from the state of the queues
when the guest completes step 5 (= set FEATURES_OK).

Am I wrong?

... On the other hand, when the driver sets DRIVER_OK (step 8), then the
host *can* derive clues from the state of the queues; I think.

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit

[Laszlo Ersek]

On 12/13/17 10:29, Paolo Bonzini wrote:
> On 13/12/2017 09:35, Laszlo Ersek wrote:
>> I consider the lack of a "VIRTIO_SCSI_F_MQ" feature bit an issue with
>> the virtio specification (and consequently with vhost-scsi), not with
>> the guest driver(s).
> 
> VIRTIO_SCSI_F_MQ does not exist because virtio-scsi has _always_
> supported multiqueue and has always had a "num_queues" field in the
> configuration space.  For virtio-net, VIRTIO_NET_F_MQ does not say "the
> device or driver knows about multiqueue", it says "the device or driver
> wants to read max_virtqueue_pairs" from configuration space.  It's
> perfectly fine for a device to propose VIRTIO_NET_F_MQ and set
> max_virtqueue_pairs=1, or for a driver to negotiate VIRTIO_NET_F_MQ and
> then skip initialization of some virtqueues.
> 
> This also means that Maxime's patch to DPDK is also not enough. :)
> Virtio-net actually does have a configuration mechanism for multiqueue,
> namely the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command; the driver sends
> that command specifying the number of the transmit and receive queues to
> use.  However, in my understanding, that command is only needed for the
> device to configure receive flow steering, so virtio-scsi doesn't need
> that either.
> 
>> Perhaps you can update vhost-scsi similarly to the last patch of
>> Maxime's v4 series, even without "VIRTIO_SCSI_F_MQ" -- in the
>> SET_FEATURES request handler, just destroy the unused virtqueues that
>> have not been configured by the guest driver until that time?
> 
> Yes, this is the right solution.  We can assume that if the descriptor
> address is equal to zero, the queue is not in use.  This is not in the
> spec as far as I can see, but it is QEMU's assumption.  I will send a
> patch to the virtio specification.

Thank you Paolo!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH edk2-platforms] Platform/DeveloperBox: add configurable eMMC support

[Ard Biesheuvel]

Add a HII page to the PlatformDxe driver that allows eMMC support to
be enabled, and wire it up for both DeveloperBox and EVB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc   
 |   9 ++
 Platform/Socionext/DeveloperBox/DeveloperBox.fdf   
 |   7 ++
 Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc   
 |   2 +
 Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi  
 |   1 -
 Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts  
 |   4 -
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c  
 | 117 +++-
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h  
 |   9 ++
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
 |  11 ++
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.uni 
 |  27 +
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxeHii.vfr 
 |  53 +
 Silicon/Socionext/SynQuacer/Include/Guid/SynQuacerPlatformFormSet.h
 |  23 
 Silicon/Socionext/SynQuacer/Include/Platform/VarStore.h
 |  27 +
 
Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
   |  23 ++--
 
Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
 |   1 +
 Silicon/Socionext/SynQuacer/SynQuacer.dec  
 |   5 +
 15 files changed, 302 insertions(+), 17 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 83cb6f9942d4..1e39c29d7910 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -408,6 +408,8 @@ [PcdsDynamicExDefault.common.DEFAULT]
 [PcdsDynamicHii]
   
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|30
 
+  
gSynQuacerTokenSpaceGuid.PcdPlatformSettings|L"SynQuacerPlatformSettings"|gSynQuacerPlatformFormSetGuid|0x0|0x0|NV,BS
+
 [PcdsDynamicDefault]
   gArmTokenSpaceGuid.PcdSystemMemoryBase|0x
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x
@@ -550,6 +552,13 @@ [Components.common]
   
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
   #
+  # eMMC support
+  #
+  
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+
+  #
   # AHCI Support
   #
   MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf 
b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
index 39d17a24d5ce..c2bc5aa85739 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
@@ -148,6 +148,13 @@ [FV.FvMain]
   INF 
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
   #
+  # eMMC support
+  #
+  INF 
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+  INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+  INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+
+  #
   # AHCI Support
   #
   INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc 
b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
index 630727cd19b3..70ec7d7baeec 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
+++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
@@ -398,6 +398,8 @@ [PcdsDynamicExDefault.common.DEFAULT]
 [PcdsDynamicHii]
   
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|30
 
+  
gSynQuacerTokenSpaceGuid.PcdPlatformSettings|L"SynQuacerPlatformSettings"|gSynQuacerPlatformFormSetGuid|0x0|0x0|NV,BS
+
 [PcdsDynamicDefault]
   gArmTokenSpaceGuid.PcdSystemMemoryBase|0x
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi 
b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 37a3981f0360..0bbbf26b7d70 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -568,6 +568,5 @@
 clocks = <_alw_c_0 _alw_b_0>;
 clock-names = "core", "iface";
 dma-coherent;
-status = "disabled";
 };
 };
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts 
b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
index 132fd370a71b..7de7db182b27 

Re: [edk2] [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit

[Paolo Bonzini]

On 13/12/2017 09:35, Laszlo Ersek wrote:
> I consider the lack of a "VIRTIO_SCSI_F_MQ" feature bit an issue with
> the virtio specification (and consequently with vhost-scsi), not with
> the guest driver(s).

VIRTIO_SCSI_F_MQ does not exist because virtio-scsi has _always_
supported multiqueue and has always had a "num_queues" field in the
configuration space.  For virtio-net, VIRTIO_NET_F_MQ does not say "the
device or driver knows about multiqueue", it says "the device or driver
wants to read max_virtqueue_pairs" from configuration space.  It's
perfectly fine for a device to propose VIRTIO_NET_F_MQ and set
max_virtqueue_pairs=1, or for a driver to negotiate VIRTIO_NET_F_MQ and
then skip initialization of some virtqueues.

This also means that Maxime's patch to DPDK is also not enough. :)
Virtio-net actually does have a configuration mechanism for multiqueue,
namely the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command; the driver sends
that command specifying the number of the transmit and receive queues to
use.  However, in my understanding, that command is only needed for the
device to configure receive flow steering, so virtio-scsi doesn't need
that either.

> Perhaps you can update vhost-scsi similarly to the last patch of
> Maxime's v4 series, even without "VIRTIO_SCSI_F_MQ" -- in the
> SET_FEATURES request handler, just destroy the unused virtqueues that
> have not been configured by the guest driver until that time?

Yes, this is the right solution.  We can assume that if the descriptor
address is equal to zero, the queue is not in use.  This is not in the
spec as far as I can see, but it is QEMU's assumption.  I will send a
patch to the virtio specification.

Paolo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Enable HD audio.

[zwei4]

Enable HD audio on Intel reference board. 
(1) Enable HdAudioDspUaaCompliance.
(2) Move audio verb table to board specifc folder.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: zwei4 
---
 .../Board/LeafHill/BoardInitPostMem/BoardInit.c|  7 
 .../LeafHill/BoardInitPostMem/BoardInitPostMem.inf |  5 ++-
 .../LeafHill/BoardInitPostMem}/HdaVerbTables.c | 38 ++---
 .../LeafHill/BoardInitPostMem/HdaVerbTables.h  | 38 +
 .../PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf|  2 +
 .../PeiFspPolicyInitLib/PeiFspScPolicyInitLib.c| 11 ++---
 .../Library/PeiPolicyUpdateLib/HdaVerbTables.h | 30 --
 .../PeiPolicyUpdateLib/PeiPolicyUpdateLib.inf  |  1 -
 .../Library/PeiPolicyUpdateLib/PeiScPolicyUpdate.c | 48 --
 Platform/BroxtonPlatformPkg/PlatformPkg.dec|  5 +++
 10 files changed, 64 insertions(+), 121 deletions(-)
 rename Platform/BroxtonPlatformPkg/{Common/Library/PeiPolicyUpdateLib => 
Board/LeafHill/BoardInitPostMem}/HdaVerbTables.c (66%)
 create mode 100644 
Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/HdaVerbTables.h
 delete mode 100644 
Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/HdaVerbTables.h

diff --git 
a/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInit.c 
b/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInit.c
index 4bd93d167..ca49dfe0f 100644
--- a/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInit.c
+++ b/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInit.c
@@ -22,6 +22,7 @@
 #include 
 #include "BoardInit.h"
 #include "BoardInitMiscs.h"
+#include "HdaVerbTables.h"
 
 EFI_STATUS
 EFIAPI
@@ -135,6 +136,12 @@ LeafHillPostMemInitCallback (
   //
   PcdSet8 (PcdeMMCHostMaxSpeed, (UINT8) 
(SystemConfiguration.ScceMMCHostMaxSpeed));
 
+  //
+  // HDA audio verb table
+  //
+  PcdSet64 (PcdHdaVerbTablePtr, (UINT64) (UINTN) );
+  PcdSet8(HdaVerbTableEntryNum, 1);
+  
   //
   // Add init steps here
   //
diff --git 
a/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInitPostMem.inf
 
b/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInitPostMem.inf
index 0f11b1c11..5154235f8 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInitPostMem.inf
+++ 
b/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/BoardInitPostMem.inf
@@ -31,6 +31,7 @@
   PlatformInfoHob.c
   BoardGpios.c
   BoardGpios.h
+  HdaVerbTables.c
 
 [LibraryClasses]
   PeiServicesLib
@@ -64,7 +65,9 @@
   gPlatformModuleTokenSpaceGuid.PcdSueCreek
   gPlatformModuleTokenSpaceGuid.PcdMaxPkgCState
   gPlatformModuleTokenSpaceGuid.PcdeMMCHostMaxSpeed
-
+  gPlatformModuleTokenSpaceGuid.PcdHdaVerbTablePtr
+  gPlatformModuleTokenSpaceGuid.HdaVerbTableEntryNum
+  
 [Guids]
   gEfiPlatformInfoGuid
   gEfiAuthenticatedVariableGuid
diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/HdaVerbTables.c 
b/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/HdaVerbTables.c
similarity index 66%
rename from 
Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/HdaVerbTables.c
rename to 
Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/HdaVerbTables.c
index 6cd068e41..9a911fe2c 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/HdaVerbTables.c
+++ 
b/Platform/BroxtonPlatformPkg/Board/LeafHill/BoardInitPostMem/HdaVerbTables.c
@@ -1,5 +1,7 @@
 /** @file
-  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+  HD Audio Verb Table.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -14,13 +16,6 @@
 #include "HdaVerbTables.h"
 
 HDAUDIO_VERB_TABLE HdaVerbTableAlc662 = {
-  //
-  // VerbTable: (Realtek ALC662)
-  // Revision ID = 0xff
-  // Codec Verb Table for IOTG CRB boards
-  // Codec Address: CAd value (0/1/2)
-  // Codec Vendor: 0x10EC0662
-  //
   {
 0x10EC0662, // Vendor ID / Device ID
 0xFF,   // Revision ID
@@ -28,31 +23,7 @@ HDAUDIO_VERB_TABLE HdaVerbTableAlc662 = {
 15 * 4  // Number of data DWORDs following the header.
   },
   {
-//
-// Realtek Semiconductor Corp.
-//
-// Realtek High Definition Audio Configuration - Version : 5.0.2.6
-// Realtek HD Audio Codec : ALC662-VD
-// PCI PnP ID : PCI\VEN_8086_2668_72708086
-// HDA Codec PnP ID : HDAUDIO\FUNC_01_10EC_0662_8086
-// The number of verb command block : 15
-//
-// NID 0x12 : 0x4013
-// NID 0x14 : 0x01014010
-// NID 0x15 : 0x01011012
-// NID 0x16 : 0x01016011
-// NID 0x18 : 0x01A19030
-// NID 0x19 : 0x02A19040
-// NID 0x1A : 0x0181303F
-// NID 0x1B : 0x0221401F
-// NID 0x1C : 0x41F0
-// NID 0x1D : 

Re: [edk2] [PATCH V2] UefiCpuPkg: Check invalid RegisterCpuFeature parameter

[Ni, Ruiyu]

On 12/13/2017 4:44 PM, Laszlo Ersek wrote:

On 12/13/17 03:35, Song, BinX wrote:

V2:
Update function name, add more detail description.
V1:
Check and assert invalid RegisterCpuFeature function parameter

Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bell Song 
---
  .../Include/Library/RegisterCpuFeaturesLib.h   |  5 
  .../RegisterCpuFeaturesLib.c   | 29 ++
  2 files changed, 34 insertions(+)

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h 
b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index 9331e49..fc3ccda 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -71,6 +71,11 @@
  #define CPU_FEATURE_APIC_TPR_UPDATE_MESSAGE (32+9)
  #define CPU_FEATURE_ENERGY_PERFORMANCE_BIAS (32+10)
  #define CPU_FEATURE_PPIN(32+11)
+//
+// Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
+// If you define a feature bigger than it, please also replace it
+// in RegisterCpuFeatureLibIsFeatureValid function.
+//
  #define CPU_FEATURE_PROC_TRACE  (32+12)
  
  #define CPU_FEATURE_BEFORE_ALL  BIT27

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index dd6a82b..6ec26e1 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -81,6 +81,34 @@ DumpCpuFeature (
  }
  
  /**

+  Determines if the CPU feature is valid.
+
+  @param[in]  FeaturePointer to CPU feature
+
+  @retval TRUE  The CPU feature is valid.
+  @retval FALSE The CPU feature is invalid.
+**/
+BOOLEAN
+RegisterCpuFeatureLibIsFeatureValid (
+  IN UINT32Feature
+  )
+{
+  UINT32  Data;
+
+  Data = Feature;
+  Data &= ~(CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER | CPU_FEATURE_BEFORE_ALL | 
CPU_FEATURE_AFTER_ALL);
+  //
+  // Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
+  // If you define a feature bigger than it, please replace it at below.
+  //
+  if (Data > CPU_FEATURE_PROC_TRACE) {
+DEBUG ((DEBUG_ERROR, "Invalid CPU feature: 0x%x ", Feature));
+return FALSE;
+  }
+  return TRUE;
+}
+
+/**
Determines if the feature bit mask is in dependent CPU feature bit mask 
buffer.
  
@param[in]  FeatureMaskPointer to CPU feature bit mask

@@ -444,6 +472,7 @@ RegisterCpuFeature (
  
VA_START (Marker, InitializeFunc);

Feature = VA_ARG (Marker, UINT32);
+  ASSERT (RegisterCpuFeatureLibIsFeatureValid(Feature));
while (Feature != CPU_FEATURE_END) {
  ASSERT ((Feature & (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER))
  != (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER));



The consensus thus far seems to be that we should not add a separate
_MAX macro for this purpose. I don't understand why -- in my opinion it
would be easier to update the macro in one place only.

Now, I realize we have a library class header file here, and a library
instance. Those things are separate; it is conceivable that another
library instance is developed independently, and thus we should not tie
the MAX feature of *all* library instances to the same central class header.

However, this separation is already being violated in this patch: the
RegisterCpuFeatureLibIsFeatureValid() function is an implementation
detail of the (currently only one) library instance. Thus, the lib class
header should not refer to it, even in a comment.

So, I don't understand why we can't just add a _MAX macro. The central
library instance could use _MAX; all other (out of tree) instances would
not use _MAX.



I do not understand either:)
But if the change doesn't expose more interfaces (_MAX in this case), I
feel safe because we can change much freely in future.


Anyway, this doesn't mean the patch is not correct.

Acked-by: Laszlo Ersek 

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel




--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V2] UefiCpuPkg: Check invalid RegisterCpuFeature parameter

[Laszlo Ersek]

On 12/13/17 03:35, Song, BinX wrote:
> V2:
> Update function name, add more detail description.
> V1:
> Check and assert invalid RegisterCpuFeature function parameter
> 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bell Song 
> ---
>  .../Include/Library/RegisterCpuFeaturesLib.h   |  5 
>  .../RegisterCpuFeaturesLib.c   | 29 
> ++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h 
> b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> index 9331e49..fc3ccda 100644
> --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> @@ -71,6 +71,11 @@
>  #define CPU_FEATURE_APIC_TPR_UPDATE_MESSAGE (32+9)
>  #define CPU_FEATURE_ENERGY_PERFORMANCE_BIAS (32+10)
>  #define CPU_FEATURE_PPIN(32+11)
> +//
> +// Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
> +// If you define a feature bigger than it, please also replace it
> +// in RegisterCpuFeatureLibIsFeatureValid function.
> +//
>  #define CPU_FEATURE_PROC_TRACE  (32+12)
>  
>  #define CPU_FEATURE_BEFORE_ALL  BIT27
> diff --git 
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> index dd6a82b..6ec26e1 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -81,6 +81,34 @@ DumpCpuFeature (
>  }
>  
>  /**
> +  Determines if the CPU feature is valid.
> +
> +  @param[in]  FeaturePointer to CPU feature
> +
> +  @retval TRUE  The CPU feature is valid.
> +  @retval FALSE The CPU feature is invalid.
> +**/
> +BOOLEAN
> +RegisterCpuFeatureLibIsFeatureValid (
> +  IN UINT32Feature
> +  )
> +{
> +  UINT32  Data;
> +
> +  Data = Feature;
> +  Data &= ~(CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER | CPU_FEATURE_BEFORE_ALL 
> | CPU_FEATURE_AFTER_ALL);
> +  //
> +  // Currently, CPU_FEATURE_PROC_TRACE is the MAX feature we support.
> +  // If you define a feature bigger than it, please replace it at below.
> +  //
> +  if (Data > CPU_FEATURE_PROC_TRACE) {
> +DEBUG ((DEBUG_ERROR, "Invalid CPU feature: 0x%x ", Feature));
> +return FALSE;
> +  }
> +  return TRUE;
> +}
> +
> +/**
>Determines if the feature bit mask is in dependent CPU feature bit mask 
> buffer.
>  
>@param[in]  FeatureMaskPointer to CPU feature bit mask
> @@ -444,6 +472,7 @@ RegisterCpuFeature (
>  
>VA_START (Marker, InitializeFunc);
>Feature = VA_ARG (Marker, UINT32);
> +  ASSERT (RegisterCpuFeatureLibIsFeatureValid(Feature));
>while (Feature != CPU_FEATURE_END) {
>  ASSERT ((Feature & (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER))
>  != (CPU_FEATURE_BEFORE | CPU_FEATURE_AFTER));
> 

The consensus thus far seems to be that we should not add a separate
_MAX macro for this purpose. I don't understand why -- in my opinion it
would be easier to update the macro in one place only.

Now, I realize we have a library class header file here, and a library
instance. Those things are separate; it is conceivable that another
library instance is developed independently, and thus we should not tie
the MAX feature of *all* library instances to the same central class header.

However, this separation is already being violated in this patch: the
RegisterCpuFeatureLibIsFeatureValid() function is an implementation
detail of the (currently only one) library instance. Thus, the lib class
header should not refer to it, even in a comment.

So, I don't understand why we can't just add a _MAX macro. The central
library instance could use _MAX; all other (out of tree) instances would
not use _MAX.

Anyway, this doesn't mean the patch is not correct.

Acked-by: Laszlo Ersek 

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit

[Laszlo Ersek]

Hello Zheng Xiang,

I'm adding Maxime for DPDK / vhost-scsi expertise, and Paolo for general
virtio-scsi expertise:

On 12/13/17 04:16, Zheng Xiang wrote:
> VirtioScsiInit() allocate only one request queue which causes the
> address of the other vrings to be 0. In AARCH64 virtual machines,
> the address of system memory starts at 0x4000 and the address
> of rom starts at 0. This causes an error when QEMU translates the
> guest physical memory of vhost-scsi vrings to host virtual memory.
> 
> So this patch fixes this issue by allocating all the required Vrings.

I would have preferred if you had contacted us first, before putting
quite a bit of work into this patch (judged from the diffstat below);
perhaps via the TianoCore Bugzilla at .

Because, the approach described in the commit message is wrong. (In
other words, the concrete patch might be implementing the idea
correctly, but the idea itself is incorrect.)

The symptom you are seeing is a problem in vhost-scsi / DPDK -- and,
actually, now I'm thinking it is a bug in the virtio spec even.
Recently, Maxime has fixed a very similar bug in vhost-net:

  https://bugzilla.redhat.com/show_bug.cgi?id=1518884
  http://dpdk.org/ml/archives/dev/2017-December/083501.html

  [dpdk-dev] [PATCH v4 0/4] Vhost: fix mq=on but VIRTIO_NET_F_MQ not
negotiated

The vhost-net issue was that vhost-net required the guest driver to set
up *all* of the possible queues, even if the guest driver did not
negotiate VIRTIO_NET_F_MQ.

The same thing applies to virtio-scsi. The guest driver should not be
*required* to set up all of the possible queues -- it makes no sense to
*force* a guest driver to use multi-queue. (And indeed, QEMU does not
present this (invalid) requirement.)

The difference with virtio-net is that the virtio specification [*] does
not currently define an explicit multiqueue feature bit for virtio-scsi
-- VIRTIO_NET_F_MQ is virtio-net only -- so the guest driver has no
means to actively *clear* that bit as part of the negotiation.

[*] http://docs.oasis-open.org/virtio/virtio/v1.0/virtio-v1.0.html
Committee Specification 04; 03 March 2016

I consider the lack of a "VIRTIO_SCSI_F_MQ" feature bit an issue with
the virtio specification (and consequently with vhost-scsi), not with
the guest driver(s).

Firmware should not be forced to use advanced virtio features. For
example, SeaBIOS does not use multi-queue with virtio-scsi either:

Please refer to the *sole* vp_find_vq() call in init_virtio_scsi(), in
file "src/hw/virtio-scsi.c": only queue #2 is set up, i.e. the first
request queue. Queues #0 and #1 (controlq and eventq, respectively), and
all further request queues (>= #3) are ignored.


Perhaps you can update vhost-scsi similarly to the last patch of
Maxime's v4 series, even without "VIRTIO_SCSI_F_MQ" -- in the
SET_FEATURES request handler, just destroy the unused virtqueues that
have not been configured by the guest driver until that time?

Alternatively: if, for compatibility reasons, a new virtio-scsi feature
flag could only be introduced in the *negative* sense, such as
"VIRTIO_SCSI_F_NO_MQ", then we can add a (very small) patch to OVMF that
negotiates this bit. (I'm unsure why a negative sense flag would be
necessary; just mentioning it for completeness.)

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] MdeModulePkg/IpIoLib: Check the input parameters before use them.

[Fu Siyuan]

This patch updates the DxeIpIoLib to check the input parameters before using.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: Wang Fan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Include/Library/IpIoLib.h   | 20 
 MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 73 ++--
 2 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Include/Library/IpIoLib.h 
b/MdeModulePkg/Include/Library/IpIoLib.h
index aab0c68059..a57bc582d6 100644
--- a/MdeModulePkg/Include/Library/IpIoLib.h
+++ b/MdeModulePkg/Include/Library/IpIoLib.h
@@ -2,7 +2,7 @@
   This library is only intended to be used by UEFI network stack modules.
   It provides the combined IpIo layer on the EFI IP4 Protocol and EFI IP6 
protocol.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under 
 the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
@@ -359,8 +359,9 @@ IpIoDestroy (
 
   @param[in, out]  IpIoThe pointer to the IP_IO instance that 
needs to stop.
 
-  @retval  EFI_SUCCESS The IP_IO instance stopped successfully.
-  @retval  Others  Anrror condition occurred.
+  @retval  EFI_SUCCESSThe IP_IO instance stopped 
successfully.
+  @retval  EFI_INVALID_PARAMETER  Invalid input parameter.
+  @retval  Others Error condition occurred.
 
 **/
 EFI_STATUS
@@ -381,11 +382,12 @@ IpIoStop (
   @param[in]   OpenData   The configuration data and callbacks for
   the IP_IO instance.
 
-  @retval  EFI_SUCCESSThe IP_IO instance opened with OpenData
-  successfully.
-  @retval  EFI_ACCESS_DENIED  The IP_IO instance is configured; avoid  
-  reopening it.
-  @retval  Others An error condition occurred.
+  @retval  EFI_SUCCESSThe IP_IO instance opened with 
OpenData
+  successfully.
+  @retval  EFI_ACCESS_DENIED  The IP_IO instance is configured, 
avoid to 
+  reopen it.
+  @retval  EFI_INVALID_PARAMETER  Invalid input parameter.
+  @retval  Others Error condition occurred.
 
 **/
 EFI_STATUS
@@ -518,7 +520,7 @@ IpIoRemoveIp (
   @param[in]   Src   The local IP address.
 
   @return The pointer to the IP protocol can be used for sending purpose and 
its local
-  address is the same with Src.
+  address is the same with Src. NULL if failed.
 
 **/
 IP_IO_IP_INFO *
diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c 
b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
index abc07fb0ff..33e2863419 100644
--- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
+++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
@@ -2,7 +2,7 @@
   IpIo Library.
 
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -280,15 +280,22 @@ IpIoIcmpv4Handler (
   UINT8Type;
   UINT8Code;
   UINT32   TrimBytes;
-
+  
+  ASSERT (IpIo != NULL);
+  ASSERT (Pkt != NULL);
+  ASSERT (Session != NULL);
   ASSERT (IpIo->IpVersion == IP_VERSION_4);
-
-  IcmpHdr = NET_PROTO_HDR (Pkt, IP4_ICMP_ERROR_HEAD);
-  IpHdr   = (EFI_IP4_HEADER *) (>IpHead);
-
+  
   //
   // Check the ICMP packet length.
   //
+  if (Pkt->TotalSize < sizeof (IP4_ICMP_ERROR_HEAD)) {
+return EFI_ABORTED;
+  }
+  
+  IcmpHdr = NET_PROTO_HDR (Pkt, IP4_ICMP_ERROR_HEAD);
+  IpHdr   = (EFI_IP4_HEADER *) (>IpHead);
+
   if (Pkt->TotalSize < ICMP_ERRLEN (IpHdr)) {
 
 return EFI_ABORTED;
@@ -412,6 +419,9 @@ IpIoIcmpv6Handler (
   UINT32   TrimBytes;
   BOOLEAN  Flag;
 
+  ASSERT (IpIo != NULL);
+  ASSERT (Pkt != NULL);
+  ASSERT (Session != NULL);
   ASSERT (IpIo->IpVersion == IP_VERSION_6);
 
   //
@@ -1028,6 +1038,7 @@ IpIoListenHandlerDpc (
   }
 
   if (IpIo->IpVersion == IP_VERSION_4) {
+ASSERT (RxData->Ip4RxData.Header != NULL);
 if (IP4_IS_LOCAL_BROADCAST (EFI_IP4 
(RxData->Ip4RxData.Header->SourceAddress))) {
   //
   // The source address is a broadcast address, discard it.
@@ -1052,6 +1063,11 @@ IpIoListenHandlerDpc (
 }
 
 //
+// The