Re: [Xen-devel] [PATCH] x86/AMD: correct certain Fam17 checks

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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