On Thu Jun 27, 2024 at 10:42 AM BST, Jan Beulich wrote: > On 26.06.2024 18:28, Alejandro Vallejo wrote: > > This greatly simplifies a later patch that makes use of HVM contexts to > > upload > > LAPIC data. The idea is to reuse MTRR setting procedure to avoid code > > duplication. It's currently only used for PVH, but there's no real reason to > > overcomplicate the toolstack preventing them being set for HVM too when > > hvmloader will override them anyway. > > Yet then - why set them when hvmloader will do so again?
To keep the toolstack complexity tractable, essentially. This way I can send N hypercalls (for N vCPUs) rather than 2*N and have a single hvmcontext struct rather than several. In truth though, I could simply write back the old MTRRs taken from bsp_ctx on HVM. > Is it even guaranteed > to be no change in (guest) behavior to do so? hvmloader overrides those values, so there is no change by the time BIOS or OVMF start running. As I mentioned before though, I can actually upload back the old values in the HVM case. > > Plus what about a guest which was configured to have the CPUID bit for MTRRs > clear? > I think we ought to document this as not supported for PVH (we may By "this" do you mean PVH _must_ have MTRR support? I would agree. > actually choose to refuse building such a guest), but in principle the MTRR > save/load operations should simply fail for a HVM guest in said configuration. What use cases does that cover? With the adjustment I mention at the top that should be sorted. I'm wondering why we allow !mtrr at all. > Making such a change in Xen now would, afaict, be benign to the tool stack. > After this adjustment it would result in a perceived regression, when there > shouldn't be any. Fair point. > > Thinking about it, even for PVH it may make sense to allow CPUID.MTRR=0, as > long as CPUID.PAT=1, thus forcing it into PAT-only mode. I think we did even > discuss this possible configuration before. > > Jan Is PAT-only an existing real HW configuration? Can't say I've seen any. Cheers, Alejandro
