On Wed, Apr 09, 2025 at 03:50:13PM +0200, Jan Beulich wrote: > 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.
I was thinking about using the segment and offset parameters of the mmio_ro_emulated_write() call. > > 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. I see... Yet for AMD that address is not uniformly available as part of the vmexit information? As I understand the checks done in mmio_ro_emulated_write() are to ensure correctness, but carrying the access even when the %cr2 check fail wouldn't cause issues to Xen itself? Thanks, Roger.
