On 09.04.2025 15:33, Roger Pau Monné wrote: > On Wed, Apr 09, 2025 at 02:59:45PM +0200, Jan Beulich wrote: >> On 09.04.2025 12:39, Roger Pau Monné wrote: >>> On Wed, Apr 09, 2025 at 12:00:16PM +0200, Jan Beulich wrote: >>>> On 09.04.2025 11:07, Roger Pau Monné wrote: >>>>> On Tue, Apr 08, 2025 at 03:57:17PM +0200, Jan Beulich wrote: >>>>>> On 08.04.2025 11:31, Roger Pau Monne wrote: >>>>>>> When running on AMD hardware in HVM mode the guest linear address (GLA) >>>>>>> will not be provided to hvm_emulate_one_mmio(), and instead is >>>>>>> unconditionally set of ~0. As a consequence mmio_ro_emulated_write() >>>>>>> will >>>>>>> always report an error, as the fault GLA generated by the emulation of >>>>>>> the >>>>>>> access won't be ~0. >>>>>> >>>>>> Which means subpage_mmio_write_accept() is flawed, too, on AMD (or more >>>>>> generally whenever .gla_valid isn't set). >>>>> >>>>> Oh, yes, good catch. I didn't notice that one. We should move all >>>>> those checks to use a paddr rather than a gla. >>>> >>>> Really that function could just be passed the offset into the page. >>>> >>>>>>> Fix this by only checking for the fault GLA in mmio_ro_emulated_write() >>>>>>> when the guest is PV. >>>>>> >>>>>> This narrows checking too much, imo. For VT-x we could continue to do so, >>>>>> provided we pass e.g. npfec down into hvm_emulate_one_mmio(), i.e. make >>>>>> the gla_valid flag visible there. >>>>> >>>>> I don't think we should rely on the gla at all in >>>>> mmio_ro_emulated_write(), and instead just use the physical address. >>>> >>>> But you can't validate a physical address against a CR2 value. And I view >>>> this validation as meaningful, to guard (best effort, but still) against >>>> e.g. insn re-writing under our feet. >>> >>> But we have the mfn in mmio_ro_ctxt, and could possibly use that to >>> validate? I could expand the context to include the offset also, so >>> that we could fully validate it. >> >> How would you use the MFN to validate against the VA in CR2? > > I would use hvmemul_virtual_to_linear()
If you mean to use the CR2 as input, you wouldn't need this. I said VA in my earlier reply, yes, but strictly speaking that's a linear address. > and hvm_translate_get_page() > to get the underlying mfn of the linear address. But maybe there's a > part of this that I'm missing, I've certainly haven't tried to > implement any of it. Hmm, I see. I didn't think of doing it this way round. There's certainly at least one caveat with this approach: Multiple linear addresses can all map to the same GFN and hence MFN. Checking against the original linear address (when available) doesn't have such an issue. Jan
