Messages in this thread | | | Date | Wed, 5 Apr 2023 15:10:09 -0400 | Subject | Re: [PATCH] x86/amd_nb: Check for invalid SMN reads | From | Yazen Ghannam <> |
| |
On 4/5/23 14:32, Yazen Ghannam wrote: > On 4/5/23 14:06, Borislav Petkov wrote: >> On Mon, Apr 03, 2023 at 05:36:40PM -0400, Yazen Ghannam wrote: >>> Yes, that's fine. Should I send another revision? >> >> On a second thought, I think we should do what you said in the write >> function too. Because the write can fail too. So if it can, we need to >> handle that potential error too. >> >> Care to send a new version which does this check in the read and in the >> write function? Basically what you had initially but with the write side >> check added too to amd_smn_write. >> > > Sure thing. I don't have a real test for the write path. But I'll test by > faking it. >
So I thought about it for a bit and quickly realized the "write and read back" method isn't robust when done here.
Possible issues: 1) Bits that are "Write-1-to-clear". In this case, we *don't* want the read to match the write. 2) Bits that are "Read-as-Zero"/"Writes-Ignored". We can't know this information here. 3) Bits that are "Reserved / Set to 1". Ditto above.
I think all these issues should be handled by the callers of amd_smn_write(). They should do the "write and read back" check themselves, if needed.
For #1, they can see if their target bits got cleared.
For #2 and #3, they can check if their target bits got set as intended.
This matches what we do for rdmsr/wrmsr. As long as there's no #GP, then we're good, and the caller does their own checking.
The "PCI Error Response" for the SMN read is the only check that would apply to *any* SMN read. So I think that makes sense to do here instead of at each call site.
What do you think?
Thanks, Yazen
| |