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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, May 27, 2021 at 04:57:04PM +0200, Jan Beulich wrote:
> 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.

Right, I think I read the text too fast. I don't think we do need it.

> >> 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.

Indeed, I think that's fine.

> >>>> 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.

I would say it's fine to request such big region that resides in a BAR
to be placed in a 64bit BAR and then be put above the 4G boundary, but
anyway, we don't need to discuss this now.

Also I'm not sure how big VRAM regions does QEMU really allow.

Thanks, Roger.