[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/AMD: correct certain Fam17 checks
On 19/03/2019 09:16, Jan Beulich wrote: > Commit 3157bb4e13 ("Add MSR support for various feature AMD processor > families") converted certain checks for Fam11 to include families all > the way up to Fam17. The commit having no description, it is hard to > tell whether this was a mechanical dec->hex conversion mistake, or > indeed intended. In any event the NB_CFG handling needs to be restricted > to Fam16 and below: Fam17 doesn't have such an MSR anymore. > > A non-MMCFG extended config space access mechanism still appears to > exist, but code to deal with it will need to be written down the road, > when it can actually be tested. > > Reported-by: Pu Wen <puwen@xxxxxxxx> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> Having looked through various spec documents, this is a chronic mess. First, to NB_CTL MSR itself. It certainly is documented in Fam15, and is absence in the documentation of Fam17. In Fam15, it is explicitly documented as an alias of 00:18.3[NB_CFG_LOW/HIGH] which are registers at offset 0x88 and 0x8c in config space. Fam17 documents that the extended cfc/cf8 mechanism does still exist, and the new controls for that found in 00:18.4[CoreMasterAccessCtrl] with a different bit layout. Experimentation on a Rome system indicates that NB_CTL is fully read0/write discard, so this patch is probably an improvement. Therefore, in principle, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> However, the actual code touched is completely insane. HVM guests have it automatically read0/write discard, even for Intel (citing cross vendor migration). PV guest handling is complicated. For CPUs without the MSR, it is read #GP, write discard. For CPUs which do have the MSR, it is read0/write discard for domU or nonpinned dom0, which is 100% of usecases. The PV vs HVM differences cause an asymetry for the hardware domain. A pinned PV dom0 is permitted to change just the IO_ECS bit, eliciting a warning but no #GP for modifying other bits. As NB_CTL is a per-node control (not a per-core control), unless dom0 vcpus == host pcpus, this creates an asymmetry across the system as to whether IO_ECS is enabled or not. The HVM IOREQ path, and PV cfg_ok() path, when seeing an IO_ECS access from the guest, reads the MSR every time, which is results in behaviour which doesn't match the settings a guest can see, and comes with a massive perf hit. It also means the behaviour of the guest IO depends on which node it is currently scheduled on. Moving on to Linux... Linux tries to enable IO_ECS on all AMD and HYGON CPUs >= Fam10. One path tries to enable things using PCI config space, and the PCI device list includes Fam17h and HYGON northbridges, but the code only operates on the Fam15h information, meaning that it clobbers what appears to be a reserved register on Fam17/Hygon hardware. Linux also uses NB_CTL in all circumstances, but given hardware's read0/write discard behaviour, Linux fails to notice that it doesn't enable IO_ECS at all. Nothing ever reads back settings to check whether IO_ECS has been correctly enabled, which means that it really isn't enabled on Fam17h/Hygon, and really isn't when running under Xen, but Linux is under the expectation that it is enabled. In practice, this means that the use of raw_pci_ext_ops between being set up in pci_direct_init() and possibly overridden in pci_mmcfg_init(). Back to Xen... The IO_ECS setting should be chosen once at boot, made consistent across the entire system, and never touched at runtime. In all cases for guests, we can offer MMCFG even on a system which doesn't have IO_ECS, and they will prefer that. The behaviour of the extra 4 bits is reserved, so we could have IO_ECS working in practice with no signal. However, we could equally drop IO_ECS entirely. Guests can't find its setting to begin with, so can't reliably use it, and also wouldn't in the presence of MMCFG anyway. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |