x86/AMD: adjust SYSCFG, TOM, etc exposure to deal with running nested

Commit Message

Jan Beulich July 13, 2021, 7:22 a.m. UTC

In the original change I neglected to consider the case of us running as
L1 under another Xen. In this case we're not Dom0, so the underlying Xen
wouldn't permit us access to these MSRs. As an immediate workaround use
rdmsr_safe(); I don't view this as the final solution though, as the
original problem the earlier change tried to address also applies when
running nested. Yet it is then unclear to me how to properly address the
issue: We shouldn't generally expose the MSR values, but handing back
zero (or effectively any other static value) doesn't look appropriate
either.

Fixes: bfcdaae9c210 ("x86/AMD: expose SYSCFG, TOM, TOM2, and IORRs to Dom0")
Signed-off-by: Jan Beulich <[e-mail]>

Comments

Julien Grall July 19, 2021, 9:18 a.m. UTC | #1

Hi Jan,

On 13/07/2021 08:22, Jan Beulich wrote:
> In the original change I neglected to consider the case of us running as
> L1 under another Xen. In this case we're not Dom0, so the underlying Xen
> wouldn't permit us access to these MSRs. As an immediate workaround use
> rdmsr_safe(); I don't view this as the final solution though, as the
> original problem the earlier change tried to address also applies when
> running nested. Yet it is then unclear to me how to properly address the
> issue: We shouldn't generally expose the MSR values, but handing back
> zero (or effectively any other static value) doesn't look appropriate
> either.

IIUC, the unsolved problem is a Linux 3.12 dom0 running on top of the L1 
Xen. The kernel is quite old (and looks to be unsupported), so are we 
expecting anyone to build a new stack with a newer Xen and such dom0?

If the answer is unlikely, then I think it would be fair to keep the 
limitation until someone comes up with such setup.

> 
> Fixes: bfcdaae9c210 ("x86/AMD: expose SYSCFG, TOM, TOM2, and IORRs to Dom0")
> Signed-off-by: Jan Beulich <[e-mail]>

Reviewed-by: Julien Grall <[e-mail]>

Cheers,

Jan Beulich July 19, 2021, 9:30 a.m. UTC | #2

On 19.07.2021 11:18, Julien Grall wrote:
> On 13/07/2021 08:22, Jan Beulich wrote:
>> In the original change I neglected to consider the case of us running as
>> L1 under another Xen. In this case we're not Dom0, so the underlying Xen
>> wouldn't permit us access to these MSRs. As an immediate workaround use
>> rdmsr_safe(); I don't view this as the final solution though, as the
>> original problem the earlier change tried to address also applies when
>> running nested. Yet it is then unclear to me how to properly address the
>> issue: We shouldn't generally expose the MSR values, but handing back
>> zero (or effectively any other static value) doesn't look appropriate
>> either.
> 
> IIUC, the unsolved problem is a Linux 3.12 dom0 running on top of the L1 
> Xen. The kernel is quite old (and looks to be unsupported), so are we 
> expecting anyone to build a new stack with a newer Xen and such dom0?
> 
> If the answer is unlikely, then I think it would be fair to keep the 
> limitation until someone comes up with such setup.

I might want to put it differently: If you want to run nested, you
shouldn't be using this old a kernel for your Dom0. You saying "looks
to be unsupported" is, aiui, a statement from upstream perspective,
which distros may have a different view on.

>> Fixes: bfcdaae9c210 ("x86/AMD: expose SYSCFG, TOM, TOM2, and IORRs to Dom0")
>> Signed-off-by: Jan Beulich <[e-mail]>
> 
> Reviewed-by: Julien Grall <[e-mail]>

Thanks.

Jan

Patch

--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -367,7 +367,8 @@  int guest_rdmsr(struct vcpu *v, uint32_t
             goto gp_fault;
         if ( !is_hardware_domain(d) )
             return X86EMUL_UNHANDLEABLE;
-        rdmsrl(msr, *val);
+        if ( rdmsr_safe(msr, *val) )
+            goto gp_fault;
         if ( msr == MSR_K8_SYSCFG )
             *val &= (SYSCFG_TOM2_FORCE_WB | SYSCFG_MTRR_TOM2_EN |
                      SYSCFG_MTRR_VAR_DRAM_EN | SYSCFG_MTRR_FIX_DRAM_EN);