[edk2-devel] [PATCH] On branch PCIBus dulePkg/PciBusDxe: PciTestSupportedAttribute logic should be changed.

[Ni, Ray]

Shengfeng,
I guess your patch is to avoid some other bits (other than EFI_PCI_COMMAND_IO_SPACE, EFI_PCI_COMMAND_MEMORY_SPACE,
EFI_PCI_COMMAND_BUS_MASTER, EFI_PCI_COMMAND_VGA_PALETTE_SNOOP) be cleared by the first PCI_SET_COMMAND_REGISTER().

Can you refine your commit message to describe it clearly?
The current commit message looks very confusing to me.

And don't do inline assignment please.

The flow could be:
  UINT16  CommandValue;

  PCI_READ_COMMAND_REGISTER (PciIoDevice, OldCommand);

  //
  // Raise TPL to high level to disable timer interrupt while the BAR is probed
  //
  OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);

  CommandValue = *Command | OldCommand;
  PCI_SET_COMMAND_REGISTER (PciIoDevice, CommandValue);
  PCI_READ_COMMAND_REGISTER (PciIoDevice, &CommandValue);
  *Command = *Command & CommandValue;

  //
  // Write back the original value
  //
  PCI_SET_COMMAND_REGISTER (PciIoDevice, *OldCommand);



-----Original Message-----
From: xueshengfeng <xueshengfeng at byosoft.com.cn> 
Sent: Wednesday, September 22, 2021 11:18 AM
To: devel at edk2.groups.io; gaoliming at byosoft.com.cn; Wu, Hao A <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>
Cc: Xue, ShengfengX <shengfengx.xue at intel.com>; Liang, PanlingX <panlingx.liang at intel.com>
Subject: [PATCH] On branch PCIBus dulePkg/PciBusDxe: PciTestSupportedAttribute logic should be changed.

 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3635

 In file Edk2\MdeModulePkg\Bus\Pci\PciBusDxe\PciEnumeratorSupport.c
 Function PciTestSupportedAttribute, This function is called when doing  the PCI enumerate, it tries to test whether the device can support  given attributes by follow steps.
 1), read the original register value
 2), set to the input register value
 3), read back the register value, return this value as output  4), restore the original value This will cause the enabled bits in this
   register be disabled during this sequence.

   Below are the new suggested flow:
   1) , read the original register value.
   2), set to input register value OR(|) the original register value.
   3), read back the register value, return the value AND(&) the input
     command value as output.
     4), restore the original value

     Above sequence will not change the enabled bits in the register,
     and can the new supported attributes by the device.

     Signed-off-by: shengfengx.xue at intel.com
     Reviewed-by: gaoliming at byosoft.com.cn
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index db1b35f8ef..2462f58833 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -933,6 +933,7 @@ PciTestSupportedAttribute (
   )
 {
   EFI_TPL OldTpl;
+  UINT16                             CommandTemp = 0;
 
   //
   // Preserve the original value
@@ -944,9 +945,10 @@ PciTestSupportedAttribute (
   //
   OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
 
-  PCI_SET_COMMAND_REGISTER (PciIoDevice, *Command);
-  PCI_READ_COMMAND_REGISTER (PciIoDevice, Command);
+  PCI_SET_COMMAND_REGISTER (PciIoDevice, (*Command | *OldCommand));  
+ PCI_READ_COMMAND_REGISTER (PciIoDevice, &CommandTemp);
 
+  *Command = (*Command) & CommandTemp;
   //
   // Write back the original value
   //
--
2.31.1.windows.1




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80947): https://edk2.groups.io/g/devel/message/80947
Mute This Topic: https://groups.io/mt/85783989/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-