| Message ID | 20231207150804.3425468-1-james.morse@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: arm64: Hide unsupported MPAM from the guest | expand |
Hi James, Thank you very much for posting these fixes. On Thu, Dec 07, 2023 at 03:08:00PM +0000, James Morse wrote: > 'lo > > This series fixes up a long standing bug where MPAM was accidentally exposed > to a guest, but the feature was not otherwise trapped or context switched. > This could result in KVM warning about unexpected traps, and injecting an > undef into the guest contradicting the ID registers. > This would prevent an MPAM aware kernel from booting - fortunately, there > aren't any of those. > > Ideally, we'd take the MPAM feature away from the ID registers, but that > would leave existing guests unable to migrate to a newer kernel. Instead, > use the writable ID registers to allow MPAM to be re-enabled - but emulate > it as RAZ/WI for the system registers that are trapped. This is certainly a reasonable approach, but TBH I'm not too terribly concerned about the completeness of the workaround plumbing that we need here. Undoubtedly what you propose is the most complete solution to the problem, but it is somewhat involved. So long as we don't break migration at the userspace ioctl level I'm not too worried. Maybe something like: - Ensure the reset value of ID_AA64PFR0_EL1.MPAM is 0 - Allow userspace to write ID_AA64PFR0_EL1.MPAM as 1, *but* - KVM reinterprets this as '0' behind the scenes for the final register value - Add the MPAM registers to the sysreg table with trap_raz_wi() as the handler to avoid the unsupported sysreg printk, since it is possible that older VMs may've already seen the MPAM feature. We've already done something similar to hide our mistakes with IMP DEF PMU versions in commit f90f9360c3d7 ("KVM: arm64: Rewrite IMPDEF PMU version as NI"), and I think MPAM may be a good candidate for something similar.
Hi Oliver, On 13/12/2023 20:24, Oliver Upton wrote: > On Thu, Dec 07, 2023 at 03:08:00PM +0000, James Morse wrote: >> This series fixes up a long standing bug where MPAM was accidentally exposed >> to a guest, but the feature was not otherwise trapped or context switched. >> This could result in KVM warning about unexpected traps, and injecting an >> undef into the guest contradicting the ID registers. >> This would prevent an MPAM aware kernel from booting - fortunately, there >> aren't any of those. >> >> Ideally, we'd take the MPAM feature away from the ID registers, but that >> would leave existing guests unable to migrate to a newer kernel. Instead, >> use the writable ID registers to allow MPAM to be re-enabled - but emulate >> it as RAZ/WI for the system registers that are trapped. > This is certainly a reasonable approach, but TBH I'm not too terribly > concerned about the completeness of the workaround plumbing that we need > here. Undoubtedly what you propose is the most complete solution to the > problem, but it is somewhat involved. Yup, I figured having a new upper limit than the default for id registers would be needed at some point, but maybe not today. > So long as we don't break migration at the userspace ioctl level I'm not > too worried. Maybe something like: > > - Ensure the reset value of ID_AA64PFR0_EL1.MPAM is 0 > > - Allow userspace to write ID_AA64PFR0_EL1.MPAM as 1, *but* > - KVM reinterprets this as '0' behind the scenes for the final register > value > - Add the MPAM registers to the sysreg table with trap_raz_wi() as the > handler to avoid the unsupported sysreg printk, since it is possible > that older VMs may've already seen the MPAM feature. > We've already done something similar to hide our mistakes with IMP DEF > PMU versions in commit f90f9360c3d7 ("KVM: arm64: Rewrite IMPDEF PMU > version as NI"), and I think MPAM may be a good candidate for something > similar. As there is precedent, I feel less dirty doing that! This also solves the problem of the VMM re-writing the broken value after the vCPU has started running, and getting a surprise error. A weird side effect of doing this would be you can write MPAM=1 on A53 and KVM will ignore it, I don't want user-space to start relying on that! I'll add a final-cap check so this can only be ignored on hardware that actually has MPAM, and could have been exposed to the bug. Thanks, James
On Fri, Jan 19, 2024 at 05:15:59PM +0000, James Morse wrote: [...] > > We've already done something similar to hide our mistakes with IMP DEF > > PMU versions in commit f90f9360c3d7 ("KVM: arm64: Rewrite IMPDEF PMU > > version as NI"), and I think MPAM may be a good candidate for something > > similar. > > As there is precedent, I feel less dirty doing that! > > This also solves the problem of the VMM re-writing the broken value after the vCPU has > started running, and getting a surprise error. A weird side effect of doing this would be > you can write MPAM=1 on A53 and KVM will ignore it, I don't want user-space to start > relying on that! I'll add a final-cap check so this can only be ignored on hardware that > actually has MPAM, and could have been exposed to the bug. Ah, good idea. I probably should've done something similar in the PMU case.
Hi James, Will you have a new version for this patch series? Jing On Tue, Jan 23, 2024 at 11:06 AM Oliver Upton <oliver.upton@linux.dev> wrote: > > On Fri, Jan 19, 2024 at 05:15:59PM +0000, James Morse wrote: > > [...] > > > > We've already done something similar to hide our mistakes with IMP DEF > > > PMU versions in commit f90f9360c3d7 ("KVM: arm64: Rewrite IMPDEF PMU > > > version as NI"), and I think MPAM may be a good candidate for something > > > similar. > > > > As there is precedent, I feel less dirty doing that! > > > > This also solves the problem of the VMM re-writing the broken value after the vCPU has > > started running, and getting a surprise error. A weird side effect of doing this would be > > you can write MPAM=1 on A53 and KVM will ignore it, I don't want user-space to start > > relying on that! I'll add a final-cap check so this can only be ignored on hardware that > > actually has MPAM, and could have been exposed to the bug. > > Ah, good idea. I probably should've done something similar in the PMU > case. > > -- > Thanks, > Oliver >
Hi Jing,
On 08/03/2024 17:48, Jing Zhang wrote:
> Will you have a new version for this patch series?
Yes - sorry, I got bogged down in the x86 end of the MPAM tree, so wasn't ready to post
this until ~rc5 ... I figured the arm64 folk wouldn't be too pleased with head.S changes
that late.
I'm trying to get this series posted today, as I'm away for a few weeks.
Thanks for the nudge!
James