Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

acs_pcie.c needs reads added to preserve memory ordering #422

Open
cdwilde opened this issue Jan 17, 2025 · 5 comments
Open

acs_pcie.c needs reads added to preserve memory ordering #422

cdwilde opened this issue Jan 17, 2025 · 5 comments

Comments

@cdwilde
Copy link

cdwilde commented Jan 17, 2025

acs_pcie.c has many PCIe writes. During testing, we ran into an issue where a disable and enable got out of order. This is because writes in PCIe can go out of order. To ensure that this doesn't happen, we can add a read after each write to ensure ordering.

/* Set SERR# Enable bit in the Command Register to enable reporting

  • upstream of Non-fatal and Fatal errors detected by the Function.
    */
    val_pcie_read_cfg(bdf, TYPE01_CR, &reg_value);
    dis_mask = (1 << CR_SERRE_SHIFT);
    val_pcie_write_cfg(bdf, TYPE01_CR, reg_value | dis_mask);
    val_pcie_read_cfg(bdf, TYPE01_CR, &reg_value);
@Sujana-M
Copy link
Contributor

Hi @cdwilde ,

Thank you raising this. We are looking into this. Can you provide us more information on which enable and disable of which test got out of order. This will help us for better understanding the issue. The particular enable mentioned here is more generic and is used by multiple tests. If the enable hadn't occurred, the tests would not have proceeded. We would like to understand if this occurred for all tests or only for one. Request you to please share the information on the tests run and the list of PCIe devices in the system.

Thanks,
ACS Team

@cdwilde
Copy link
Author

cdwilde commented Feb 6, 2025

Hi, yes. There is one test that fails: 830 : Check Cmd Reg memory space enable.

Without the reads, we get:
830 : Check Cmd Reg memory space enable
[14404147363 ps] STARTException handler installing for ExceptionType 0
[14406459204 ps] ESR install result = 0x0
[14407469978 ps] Exception handler installing for ExceptionType 0
[14409191394 ps] ESR install result = 0x0
[14410192195 ps] NOTICE: ExceptionType 3 is changed to ExceptionType 47
[14412093662 ps] Exception handler installing for ExceptionType 47
[14413835022 ps] ESR install result = 0x0
[14414825851 ps]
[14414855906 ps] tbl_index 0 BDF 0 pal_mmio_read A=0x4000000034 D=0x40
[14417958306 ps] pal_mmio_read A=0x4000000040 D=0xda035001
[14419819884 ps] pal_mmio_read A=0x4000000050 D=0x2807005
[14421651270 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14423452739 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14425264181 ps] pal_mmio_read A=0x400000000c D=0x10000
[14427035596 ps] pal_mmio_read A=0x4000000018 D=0x10100
[14428957145 ps] pal_mmio_read A=0x400010000c D=0x0
[14430758614 ps] pal_mmio_read A=0x400010000c D=0x0
[14432339869 ps] bus= 1
[14432780160 ps] dev= 0
[14433220590 ps] func= 0
[14433690936 ps] offset= 10
[14434251307 ps] index= 0
[14434781762 ps] pal pci_cfg_read Ecam Base = 0x4000000000
[14436543205 ps] pal_mmio_read A=0x4000100010 D=0xc
[14438124460 ps] pcie bar[31:0] = 0xc (bottom 4 bits will be masked)
[14439955984 ps] pal pci_cfg_read Ecam Base = 0x4000000000
[14441717288 ps] pal_mmio_read A=0x4000100014 D=0x100
[14443328597 ps] pcie bar[63:32] = 0x100
[14444289372 ps] pcie bar[63:0] = 0x10000000000
[14445480333 ps] Bar Base 40000 pal_mmio_read A=0x4000000004 D=0x100406
[14448122498 ps] pal_mmio_write A=0x4000000004 D=0x100406
[14450033936 ps] pal_mmio_read A=0x4000000034 D=0x40
[14451725298 ps] pal_mmio_read A=0x4000000040 D=0xda035001
[14453586877 ps] pal_mmio_read A=0x4000000050 D=0x2807005
[14455418262 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14457219732 ps] pal_mmio_read A=0x4000000078 D=0x103170
[14458921066 ps] pal_mmio_write A=0x4000000078 D=0x103170
[14460842615 ps] pal_mmio_read A=0x4000000034 D=0x40
[14462524005 ps] pal_mmio_read A=0x4000000040 D=0xda035001
[14464385445 ps] pal_mmio_read A=0x4000000050 D=0x2807005
[14466216969 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14468018300 ps] pal_mmio_read A=0x4000000078 D=0x103170
[14469719772 ps] pal_mmio_write A=0x4000000078 D=0x83170
[14471601156 ps] pal_mmio_read A=0x4000000004 D=0x100406
[14473302629 ps] pal_mmio_write A=0x4000000004 D=0x100404
[14475304231 ps] bar_data 0
[14476034818 ps] BDF 0 MSE functionality failure pal_mmio_read A=0x4000000004 D=0x100404

With the reads, we get:
[14641176817 ps] 830 : Check Cmd Reg memory space enable
[14643018313 ps] STARTException handler installing for ExceptionType 0
[14645340127 ps] ESR install result = 0x0
[14646350900 ps] Exception handler installing for ExceptionType 0
[14648062344 ps] ESR install result = 0x0
[14649063145 ps] NOTICE: ExceptionType 3 is changed to ExceptionType 47
[14650964612 ps] Exception handler installing for ExceptionType 47
[14652705972 ps] ESR install result = 0x0
[14653706773 ps]
[14653736828 ps] tbl_index 0 BDF 0 pal_mmio_read A=0x4000000034 D=0x40
[14656829256 ps] pal_mmio_read A=0x4000000040 D=0xda035001
[14658690834 ps] pal_mmio_read A=0x4000000050 D=0x2807005
[14660532192 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14662333661 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14664145103 ps] pal_mmio_read A=0x400000000c D=0x10000
[14665926490 ps] pal_mmio_read A=0x4000000018 D=0x10100
[14667878093 ps] pal_mmio_read A=0x400010000c D=0x0
[14669679563 ps] pal_mmio_read A=0x400010000c D=0x0
[14671270789 ps] bus= 1
[14671711219 ps] dev= 0
[14672151511 ps] func= 0
[14672621857 ps] offset= 10
[14673182366 ps] index= 0
[14673712821 ps] pal pci_cfg_read Ecam Base = 0x4000000000
[14675474126 ps] pal_mmio_read A=0x4000100010 D=0xc
[14677035436 ps] pcie bar[31:0] = 0xc (bottom 4 bits will be masked)
[14678876932 ps] pal pci_cfg_read Ecam Base = 0x4000000000
[14680638375 ps] pal_mmio_read A=0x4000100014 D=0x100
[14682249546 ps] pcie bar[63:32] = 0x100
[14683210320 ps] pcie bar[63:0] = 0x10000000000
[14684391310 ps] Bar Base 40000 pal_mmio_read A=0x4000000004 D=0x100406
[14687033474 ps] pal_mmio_write A=0x4000000004 D=0x100406
[14688955023 ps] pal_mmio_read A=0x4000000004 D=0x100406
[14690766465 ps] pal_mmio_read A=0x4000000034 D=0x40
[14692447716 ps] pal_mmio_read A=0x4000000040 D=0xda035001
[14694309295 ps] pal_mmio_read A=0x4000000050 D=0x2807005
[14696150652 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14697952122 ps] pal_mmio_read A=0x4000000078 D=0x103170
[14699653455 ps] pal_mmio_write A=0x4000000078 D=0x103170
[14701575005 ps] pal_mmio_read A=0x4000000078 D=0x103170
[14703386446 ps] pal_mmio_read A=0x4000000034 D=0x40
[14705067836 ps] pal_mmio_read A=0x4000000040 D=0xda035001
[14706929275 ps] pal_mmio_read A=0x4000000050 D=0x2807005
[14708760800 ps] pal_mmio_read A=0x4000000070 D=0x420010
[14710562270 ps] pal_mmio_read A=0x4000000078 D=0x103170
[14712263604 ps] pal_mmio_write A=0x4000000078 D=0x83170
[14714145126 ps] pal_mmio_read A=0x4000000004 D=0x100406
[14715846460 ps] pal_mmio_write A=0x4000000004 D=0x100404
[14717768009 ps] pal_mmio_read A=0x4000000004 D=0x100404
[14719609505 ps] bar_data ffffffff pal_mmio_read A=0x4000000004 D=0x100404
[14722321612 ps] pal_mmio_write A=0x4000000004 D=0x100406
[14724243161 ps] pal_mmio_read A=0x4000000004 D=0x100406

@cdwilde
Copy link
Author

cdwilde commented Feb 7, 2025

Checking the runs without the change requested, the same issue exists on SBSA test 830. They both have the same title: "Check Cmd Reg memory space enable" I am going to target a local fix for these tests, and we can just change them if you are amenable. Testing now.

@Sujana-M
Copy link
Contributor

Sujana-M commented Feb 7, 2025

Hi @cdwilde,

We are working on the changes to fix this. The same will be reflected for SBSA as well.

Thanks,
ACS Team

@cdwilde
Copy link
Author

cdwilde commented Feb 7, 2025

Yes, I have a local fix that is working by just modifying the 830 tests on both BSA and SBSA.
test_pool/pcie/operating_system/test_os_p030.c
val_pcie_disable_msa(bdf);
val_pcie_read_cfg(bdf, TYPE01_CR, &reg_value);

and

  val_pcie_enable_msa(bdf);
  val_pcie_read_cfg(bdf, TYPE01_CR, &reg_value);

(just adding a read following the enable and disable)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants