On Thu, Apr 04, 2024, Kai Huang wrote: > On 4/04/2024 7:57 am, Sean Christopherson wrote: > > On Wed, Mar 27, 2024, Kai Huang wrote: > > > IIUC, the purpose of this patch is for the kernel to use X86_MEMTYPE_xx, which > > > are architectural values, where applicable? > > > > Maybe? Probably? > > > > > Yeah we need to keep MTRR_TYPE_xx in the uapi header, but in the kernel, should > > > we change all places that use MTRR_TYPE_xx to X86_MEMTYPE_xx? The > > > static_assert()s above have guaranteed the two are the same, so there's nothing > > > wrong for the kernel to use X86_MEMTYPE_xx instead. > > > > > > Both PAT_xx and VMX_BASIC_MEM_TYPE_xx to X86_MEMTYPE_xx, it seems a little bit > > > odd if we don't switch for MTRR_TYPE_xx. > > > > > > However by simple search MEM_TYPE_xx are intensively used in many files, so... > > > > Yeah, I definitely don't want to do it in this series due to the amount of churn > > that would be required. > > > > $ git grep MTRR_TYPE_ | wc -l > > 100 > > > > I'm not even entirely convinced that it would be a net positive. Much of the KVM > > usage that's being cleaned up is flat out wrong, e.g. using "MTRR" enums in places > > that having nothing to do with MTRRs. But the majority of the remaining usage is > > in MTRR code, i.e. isn't wrong, and is arguably better off using the MTRR specific > > #defines. > > Yeah understood. > > But the patch title says we also "add common defines for ... MTRRs", so to > me looks we should get rid of MTRR_TPYE_xx and use the common ones instead. > And it also looks a little bit inconsistent if we remove the PAT_xx but keep > the MTRR_TYPE_xx. > > Perhaps we can keep PAT_xx but add macros? > > #define PAT_UC X86_MEMTYPE_UC > ... > > But looks not nice either because the only purpose is to keep the PAT_xx.. Ya, keeping PAT_* is the only option I strongly object to. I have no problem replacing MTRR_TPYE_* usage, I would simply prefer not to do it in this series. And I obviously have no problem leaving the MTRR stuff as-is.
