Re: [PATCH 2/2] x86/hvm: fix write emulation of RO ranges

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

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

Thanks, Roger.