MdeModulePkg: PciHostBridgeDxe Map MMIO NX

[apop5]

Description

Update PciHostBridgeDxe to map MMIO as NX, in addition to existing UC attribute.

• Breaking change?
• Impacts security?
• Includes tests?

How This Was Tested

Tested on physical systems when Mmio Nx is expected.
Performed page audit test to verify MMIO regions are nx protected

Integration Instructions

No integration necessary

[apop5]

PR #12878 needs to complete. Ovmf was adding memory space without XP capabilities.

[os-d]

PR #12878 needs to complete. Ovmf was adding memory space without XP capabilities.

I think this can be handled more robustly by adding the capability here before setting it. I thought that was already in the downstream commit, but perhaps not.

[apop5]

PR #12878 needs to complete. Ovmf was adding memory space without XP capabilities.

I think this can be handled more robustly by adding the capability here before setting it. I thought that was already in the downstream commit, but perhaps not.

If you are suggesting checking the capabilities, adding the capabilities and then setting the attributes, I think that is bleeding responsibility from the mmio producer into this driver.

In my ideal world, the entity that is adding mmio would correctly report the capabilities in the first place. Attempting to patch it after the fact moves the responsibility too far from the mmio owner, and it sets a precedent that any driver can go modify capabilities.

Additionally there were multiple drivers in ovmf that were correctly reporting XP. This was the only place that was not reporting the capability.

[os-d]

PR #12878 needs to complete. Ovmf was adding memory space without XP capabilities.

I think this can be handled more robustly by adding the capability here before setting it. I thought that was already in the downstream commit, but perhaps not.

If you are suggesting checking the capabilities, adding the capabilities and then setting the attributes, I think that is bleeding responsibility from the mmio producer into this driver.

In my ideal world, the entity that is adding mmio would correctly report the capabilities in the first place. Attempting to patch it after the fact moves the responsibility too far from the mmio owner, and it sets a precedent that any driver can go modify capabilities.

Additionally there were multiple drivers in ovmf that were correctly reporting XP. This was the only place that was not reporting the capability.

Yes, but I don't think MMIO producers are doing this correctly, like in OVMF. I think the right path is for the resc desc HOB to be correct. But, when they are not, this will produce the same failure. This driver already adds the capabilities when it adds the memory space. This then is the case where it doesn't add the memory space because it already exists. I think it is reasonable for it to either add the capability or don't set attributes for this range and expect the driver that added the space to have set it up correctly.

[apop5]

PR #12878 needs to complete. Ovmf was adding memory space without XP capabilities.

I think this can be handled more robustly by adding the capability here before setting it. I thought that was already in the downstream commit, but perhaps not.

If you are suggesting checking the capabilities, adding the capabilities and then setting the attributes, I think that is bleeding responsibility from the mmio producer into this driver.
In my ideal world, the entity that is adding mmio would correctly report the capabilities in the first place. Attempting to patch it after the fact moves the responsibility too far from the mmio owner, and it sets a precedent that any driver can go modify capabilities.
Additionally there were multiple drivers in ovmf that were correctly reporting XP. This was the only place that was not reporting the capability.

Yes, but I don't think MMIO producers are doing this correctly, like in OVMF. I think the right path is for the resc desc HOB to be correct. But, when they are not, this will produce the same failure. This driver already adds the capabilities when it adds the memory space. This then is the case where it doesn't add the memory space because it already exists. I think it is reasonable for it to either add the capability or don't set attributes for this range and expect the driver that added the space to have set it up correctly.

Updated to add the capabilities, per your suggestion