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

intpm: use MMIO for AMD EFCH CPUs #1437

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Conversation

brian90013
Copy link
Contributor

I observed an issue when attempting to use SMBus devices on an AMD Zen3 7443P CPU where running smbmsg -p found no devices. Previously I used a Zen2 7702P CPU in the same chassis/motherboard and had no issue finding or accessing SMBus devices. Debugging the intsmb driver (source file intpm.c) showed all the PMIO reads were returning 0xff on Zen3.

A comment in the Linux i2c-piix4.c source pointed me to the root cause:

cd6h/cd7h port I/O accesses can be disabled on AMD processors
w/ SMBus PCI revision ID 0x51 or greater. MMIO is supported on
the same processors and is the recommended access method.

Indeed, the revision ID of both the Zen2 and Zen3 CPUs is 0x61. I added a test for this condition and when true, set and allocate resources using MMIO instead of PMIO. I now see valid results from smbmsg -p and am able to use SMBus devices on both the Zen2 and Zen3 CPUs using MMIO.

if (devid == AMDCZ_SMBUS_DEVID && revid >= AMDCZ51_SMBUS_REVID) {
sc->type = SYS_RES_MEMORY;
addr = AMDFCH41_MMIO_ADDR + AMDFCH41_MMIO_PM_OFF;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would set sc_type and addr in an else clause here as I think that will be clearer to future readers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I guess I see why sc->type has to be set where it is, I would still move the addr into an else here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. While there I included sc->type = SYS_RES_IOPORT in the else for clarity even though it is set by default to work in non sb8xx cases.

@brian90013
Copy link
Contributor Author

Hello, I made the requested change back on October 7th and barring other feedback believe this MR is ready.

Copy link
Member

@bsdimp bsdimp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to my eye

Recent AMD CPUs (SMBus PCI revision ID >= 0x51) can disable port-mapped
IO and only support memory-mapped IO. In practice this was observed on a
Zen 3 CPU where PMIO reads all returned 0xff. Update the driver to use
MMIO for these processors while continuing to use PMIO by default.

Reviewed by: imp
Pull Request: #1437
@freebsd-git freebsd-git merged commit b2a49e8 into freebsd:main Jan 24, 2025
7 of 10 checks passed
@brian90013 brian90013 deleted the intpm-mmio branch January 24, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants