On 17.03.2025 19:40, Andrew Cooper wrote:
> Petr reports:
>
> (XEN) MMIO emulation failed (1): d12v1 64bit @ 0010:fffff8057ba7dfbf -> 45
> 0f 20 c2 ...
>
> during introspection.
>
> This is MOV %cr8, which is wired up for hvm_mov_{to,from}_cr(); the VMExit
> fastpaths, but not for the full emulation slowpaths. Wire %cr8 up in
> hvmemul_{read,write}_cr() too.
>
> Reported-by: Petr Beneš <w1be...@gmail.com>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Like in a few other cases I guess there's no good Fixes: tag to use, as the
omission must have been there basically forever.
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -2285,6 +2285,11 @@ static int cf_check hvmemul_read_cr(
> *val = current->arch.hvm.guest_cr[reg];
> TRACE(TRC_HVM_CR_READ64, reg, *val, *val >> 32);
> return X86EMUL_OKAY;
> +
> + case 8:
> + *val = (vlapic_get_reg(vcpu_vlapic(current), APIC_TASKPRI) & 0xf0)
> >> 4;
I think it would be nice to add a #define to apicdef.h that could be utilized to
use MASK_EXTR() here (and MASK_INSR() below). Otherwise even without such a
#define I'd like to ask that we use the two macros here.
> + return X86EMUL_OKAY;
> +
> default:
> break;
> }
> @@ -2325,6 +2330,11 @@ static int cf_check hvmemul_write_cr(
> rc = hvm_set_cr4(val, true);
> break;
>
> + case 8:
> + vlapic_set_reg(vcpu_vlapic(current), APIC_TASKPRI, ((val & 0x0f) <<
> 4));
> + rc = X86EMUL_OKAY;
> + break;
Both SDM and PM say #GP upon writes to reserved bits, without there being a
place
where it is said which bits are reserved. Don't we therefore need to treat bits
4
and up as must-be-zero, implying that all bits which have no meaning are
reserved?
Jan
