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

[Jan Beulich <jbeulich@xxxxxxxx>]

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

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

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5187,7 +5187,12 @@ int cf_check mmio_ro_emulated_write(
>  
>      /* Only allow naturally-aligned stores at the original %cr2 address. */
>      if ( ((bytes | offset) & (bytes - 1)) || !bytes ||
> -         offset != mmio_ro_ctxt->cr2 )
> +         /*
> +          * HVM domains might not have a valid fault GLA in the context, as 
> AMD
> +          * NPT faults don't report the faulting GLA.  It's also possible for
> +          * the fault to happen in non-paging modes.
> +          */
> +         (is_pv_domain(current->domain) && offset != mmio_ro_ctxt->cr2) )
>      {
>          gdprintk(XENLOG_WARNING, "bad access (cr2=%lx, addr=%lx, 
> bytes=%u)\n",
>                  mmio_ro_ctxt->cr2, offset, bytes);

Is logging the supposed CR2 value useful then for cases where the GLA
isn't valid? I fear it might be more confusing than helpful.

Jan