Re: [PATCH] x86/AMD: expose SYSCFG, TOM, and TOM2 to Dom0

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2021 15:23, Roger Pau Monné wrote:
> On Thu, May 27, 2021 at 12:41:51PM +0200, Jan Beulich wrote:
>> On 27.05.2021 10:33, Roger Pau Monné wrote:
>>> On Wed, May 26, 2021 at 02:59:00PM +0200, Jan Beulich wrote:
>>>> Sufficiently old Linux (3.12-ish) accesses these MSRs in an unguarded
>>>> manner. Furthermore these MSRs, at least on Fam11 and older CPUs, are
>>>> also consulted by modern Linux, and their (bogus) built-in zapping of
>>>> #GP faults from MSR accesses leads to it effectively reading zero
>>>> instead of the intended values, which are relevant for PCI BAR placement
>>>> (which ought to all live in MMIO-type space, not in DRAM-type one).
>>>>
>>>> For SYSCFG, only certain bits get exposed. In fact, whether to expose
>>>> MtrrVarDramEn is debatable: It controls use of not just TOM, but also
>>>> the IORRs. Introduce (consistently named) constants for the bits we're
>>>> interested in and use them in pre-existing code as well.
>>>
>>> I think we should also allow access to the IORRs MSRs for coherency
>>> (c001001{6,9}) for the hardware domain.
>>
>> Hmm, originally I was under the impression that these could conceivably
>> be written by OSes, and hence would want dealing with separately. But
>> upon re-reading I see that they are supposed to be set by the BIOS alone.
>> So yes, let me add them for read access, taking care of the limitation
>> that I had to spell out.
>>
>> This raises the question then though whether to also include SMMAddr
>> and SMMMask in the set - the former does get accessed by Linux as well,
>> and was one of the reasons for needing 6eef0a99262c ("x86/PV:
>> conditionally avoid raising #GP for early guest MSR reads").
> 
> That seems fine, we might also want SMM_BASE?

That's pretty unrelated to the topic here - there's no memory type
or DRAM vs MMIO decision associated with that register. I'm also
having trouble seeing what an OS would want to use SMM's CS value
for.

>> Especially for SMMAddr, and maybe also for IORR_BASE, returning zero
>> for DomU-s might be acceptable. The respective masks, however, can
>> imo not sensibly be returned as zero. Hence even there I'd leave DomU
>> side handling (see below) for a later time.
> 
> Sure. I think for consistency we should however enable reading the
> hardware IORR MSRs for the hardware domain, or else returning
> MtrrVarDramEn set is likely to cause trouble as the guest could assume
> IORRs to be unconditionally present.

Well, yes, I've added IORRs already, as I was under the impression
that we were agreeing already that we want to expose them to Dom0.

>>>> As a welcome side effect, verbosity on/of debug builds gets (perhaps
>>>> significantly) reduced.
>>>>
>>>> Note that at least as far as those MSR accesses by Linux are concerned,
>>>> there's no similar issue for DomU-s, as the accesses sit behind PCI
>>>> device matching logic. The checked for devices would never be exposed to
>>>> DomU-s in the first place. Nevertheless I think that at least for HVM we
>>>> should return sensible values, not 0 (as svm_msr_read_intercept() does
>>>> right now). The intended values may, however, need to be determined by
>>>> hvmloader, and then get made known to Xen.
>>>
>>> Could we maybe come up with a fixed memory layout that hvmloader had
>>> to respect?
>>>
>>> Ie: DRAM from 0 to 3G, MMIO from 3G to 4G, and then the remaining
>>> DRAM from 4G in a contiguous single block?
>>>
>>> hvmloader would have to place BARs that don't fit in the 3G-4G hole at
>>> the end of DRAM (ie: after TOM2).
>>
>> Such a fixed scheme may be too limiting, I'm afraid.
> 
> Maybe, I guess a possible broken scenario would be for a guest to be
> setup with a set of 32bit BARs that cannot possibly fit in the 3-4G
> hole, but I think that's unlikely.

Can't one (almost) arbitrarily size the amount of VRAM of the emulated
VGA? I wouldn't be surprised if this can't be placed above 4Gb.

Jan